Skip to content

Conversation

@bhuvanh66
Copy link

ℹ️ Issue

Closes 102

📝 Description

Added an endpoint to update a user’s first name, last name, or phone number and tests for the controller to verify that it works.

✔️ Verification

Screenshot 2025-12-03 at 9 03 08 PM Screenshot 2025-12-03 at 9 03 24 PM Screenshot 2025-12-03 at 9 04 06 PM

Tested having no parameters, 2 parameters and 3 parameters and throwing the proper exceptions when having faulty data.
Screenshot 2025-12-03 at 9 06 13 PM

Screenshot 2025-12-03 at 9 06 32 PM

@bhuvanh66 bhuvanh66 changed the title [SSF-102] [SSF-102] Add endpoints to update user info Dec 4, 2025
Copy link

@dburkhart07 dburkhart07 left a comment

Choose a reason for hiding this comment

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

Few small changes. Additionally, can we use this PR to update the user entity? If you notice, all the other fieldsin our entity have a name, type, and sometimes a few other things. Can we apply that to firstName, lastName, and email?


export class updateUserInfo {
@IsOptional()
@IsString()

Choose a reason for hiding this comment

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

Can we add @IsNotEmpty and @MaxLength(255) to this and the last name?

lastName?: string;

@IsOptional()
@IsString()

Choose a reason for hiding this comment

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

Can we also add @IsNotEmpty here too?

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.

Can we give this a Promise<User> return type?


describe('PUT :id/info', () => {
it('should update user info with valid information', async () => {
const updatedUser = {

Choose a reason for hiding this comment

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

Can we type this? I would suggest moving the updateUserSchema to lin 122, and then making the value for each in the updatedUser `updateUserSchema.{firstName, lastName, phone}


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

Choose a reason for hiding this comment

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

Can we add one more test here where we just update with the first name and last name, to test that an actual Partial<updateUserInfo> works? I only ask this since our logic for creating the body we pass into the service is in the controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants