Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions apps/backend/src/users/dtos/updateUserInfo.dto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { IsString, IsPhoneNumber, IsOptional, IsNotEmpty, MaxLength } from 'class-validator';

export class updateUserInfo {
@IsOptional()
@IsString()
@IsNotEmpty()
@MaxLength(255)
firstName?: string;

@IsOptional()
@IsString()
@IsNotEmpty()
@MaxLength(255)
lastName?: string;

@IsOptional()
@IsString()
@IsNotEmpty()
@IsPhoneNumber('US', {
message:
'phone must be a valid phone number (make sure all the digits are correct)',
})
phone?: string;
}
33 changes: 33 additions & 0 deletions apps/backend/src/users/users.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { userSchemaDto } from './dtos/userSchema.dto';

import { Test, TestingModule } from '@nestjs/testing';
import { mock } from 'jest-mock-extended';
import { updateUserInfo } from './dtos/updateUserInfo.dto';
import { Pantry } from '../pantries/pantries.entity';

const mockUserService = mock<UsersService>();
Expand Down Expand Up @@ -158,6 +159,38 @@ describe('UsersController', () => {
});
});

describe('PUT :id/info', () => {
it('should update user info with valid information', async () => {
const updatedUser = {
...mockUser1,
firstName: 'UpdatedFirstName',
lastName: 'UpdatedLastName',
phone: '777-777-7777',
};
mockUserService.update.mockResolvedValue(updatedUser);

const updateUserSchema: updateUserInfo = {
firstName: 'UpdatedFirstName',
lastName: 'UpdatedLastName',
phone: '777-777-7777',
};
const result = await controller.updateInfo(1, updateUserSchema);

expect(result).toEqual(updatedUser);
expect(mockUserService.update).toHaveBeenCalledWith(1, updateUserSchema);
});

it('should update user info with defaults', async () => {
mockUserService.update.mockResolvedValue(mockUser1);

const updateUserSchema: Partial<updateUserInfo> = {};
const result = await controller.updateInfo(1, updateUserSchema);

expect(result).toEqual(mockUser1);
expect(mockUserService.update).toHaveBeenCalledWith(1, updateUserSchema);
});
});

describe('POST /api/users', () => {
it('should create a new user with all required fields', async () => {
const createUserSchema: userSchemaDto = {
Expand Down
16 changes: 16 additions & 0 deletions apps/backend/src/users/users.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { UsersService } from './users.service';
import { User } from './user.entity';
import { Role } from './types';
import { userSchemaDto } from './dtos/userSchema.dto';
import { updateUserInfo } from './dtos/updateUserInfo.dto';
import { Pantry } from '../pantries/pantries.entity';

@Controller('users')
Expand Down Expand Up @@ -54,6 +55,21 @@ export class UsersController {
return this.usersService.update(id, { role: role as Role });
}

@Put(':id/info')
async updateInfo(
@Param('id', ParseIntPipe) id: number,
@Body() updateUserInfo: updateUserInfo,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sam-schu @Yurika-Kan

Thoughts on making this change here:

We already have an update user role, and are now maing an update info endpoint. Would we rather have two separate updates, or just abstract this updateUserInfo DTO to include the role as well. We could then make it so the update function just takes the DTO instead of a Partial (this seems weird, we dont do this anywhere else), and adjust both functions' logic.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we needed both endpoints I would suggest keeping them separate for the sake of letting any user type call this endpoint, but limiting the update role endpoint to admins. But now that we have combined standard and lead volunteers, there is no longer any use case for updating a user's role, so we should just remove that endpoint.

): Promise<User> {
const { firstName, lastName, phone } = updateUserInfo;

const updateData: Partial<User> = {};
Copy link

@dburkhart07 dburkhart07 Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to Sam's comment above, can we just make it so tha usersService.update now just takings in an updateUserInfo DTO, and do all the parsing/checking if valid values in there? While we are at it, let's use this as an opportunity to write unit tests for that service function. Use the format in orders.service.spec.ts as a reference for how we should go about it, as well as the dummy data in the testing db for what you should expect as results.

if (firstName !== undefined) updateData.firstName = firstName;
if (lastName !== undefined) updateData.lastName = lastName;
if (phone !== undefined) updateData.phone = phone;

return this.usersService.update(id, updateData);
}

@Post('/')
async createUser(@Body() createUserDto: userSchemaDto): Promise<User> {
const { email, firstName, lastName, phone, role } = createUserDto;
Expand Down
Loading