[SSF-102] Add endpoints to update user info by bhuvanh66 · Pull Request #79 · Code-4-Community/ssf

bhuvanh66 · 2025-12-04T02:07:35Z

ℹ️ 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

Tested having no parameters, 2 parameters and 3 parameters and throwing the proper exceptions when having faulty data.

dburkhart07

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?

apps/backend/src/users/dtos/updateUserInfo.dto.ts

apps/backend/src/users/users.controller.ts

dburkhart07 · 2026-02-16T01:09:03Z

apps/backend/src/users/users.controller.ts

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


@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.

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.

dburkhart07 · 2026-02-16T20:08:22Z

apps/backend/src/users/users.controller.ts

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


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.

dburkhart07 · 2026-02-21T06:59:52Z

apps/backend/src/users/users.controller.spec.ts

-  describe('PUT /:id/role', () => {
-    it('should update user role with valid role', async () => {
-      const updatedUser = { ...mockUser1, role: Role.ADMIN };
+  describe('PUT :id/info', () => {


nit: can we rename this to patch?

dburkhart07 · 2026-02-21T07:31:45Z

apps/backend/src/users/users.service.spec.ts

+      mockUserRepository.save.mockResolvedValue(updatedUser as User);
+
+      const result = await service.update(1, dto);
+


we should check here that the updatedUser's firstName is now 'OnlyFirst'

sam-schu · 2026-02-21T22:13:01Z

apps/backend/src/users/users.service.spec.ts

    });
  });
+
+  describe('update', () => {


Please correct me if I'm wrong, but it looks like you added these new service tests this week? If so, can we write them according to the new style where we use the real testing database rather than mocking TypeORM? (You can use order.service.spec.ts as an example, but also see the change Dalton made here)

Can we combine the update tests earlier in the file into these?

sam-schu · 2026-02-21T22:19:07Z

apps/backend/src/users/dtos/updateUserInfo.dto.ts

@@ -0,0 +1,24 @@
+import { IsString, IsPhoneNumber, IsOptional, IsNotEmpty, MaxLength } from 'class-validator';
+
+export class updateUserInfo {


Can we make this more consistent with our other DTOs by naming the file update-user-info.dto.ts and the class UpdateUserInfoDto?

sam-schu · 2026-02-21T22:26:49Z

apps/backend/src/users/users.controller.ts


-  @Put('/:id/role')
-  async updateRole(
+  @Patch(':id/info')


We don't need the /info here since we are patching the user object itself

sam-schu · 2026-02-21T22:33:00Z

apps/backend/src/users/users.service.ts

-    const user = await this.findOne(id);
+    const { firstName, lastName, phone } = dto;

-    if (!user) {


Why was this check removed?

sam-schu · 2026-02-21T22:43:30Z

apps/backend/src/users/users.controller.spec.ts

-        BadRequestException,
-      );
-      expect(mockUserService.update).not.toHaveBeenCalled();
+    it('should update user info with defaults', async () => {


If we're adding a test where we try to call the controller method with an empty DTO, shouldn't the expected behavior be that we throw an exception?

sam-schu · 2026-02-21T22:43:54Z

apps/backend/src/users/users.controller.spec.ts

+    it('should update user info with defaults', async () => {
+      mockUserService.update.mockResolvedValue(mockUser1 as User);
+
+      const updateUserSchema: Partial<updateUserInfo> = {};


We shouldn't need the Partial since all the DTO fields are optional anyway

added update user info endpoint and tests

b968ae0

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

sam-schu added the high priority label Dec 5, 2025

dburkhart07 requested changes Dec 6, 2025

View reviewed changes

apps/backend/src/users/dtos/updateUserInfo.dto.ts Show resolved Hide resolved

apps/backend/src/users/dtos/updateUserInfo.dto.ts Show resolved Hide resolved

apps/backend/src/users/users.controller.ts Outdated Show resolved Hide resolved

sam-schu added blocked and removed blocked labels Jan 1, 2026

sam-schu assigned swarkewalia Jan 24, 2026

swarkewalia added 3 commits February 1, 2026 14:44

merge

5830938

merge fixes

bd3f846

some fixes

061c4a8

dburkhart07 reviewed Feb 16, 2026

View reviewed changes

dburkhart07 requested changes Feb 16, 2026

View reviewed changes

swarkewalia added 7 commits February 16, 2026 18:35

Merge branch 'main' into BH/SSF-102-Update-User-Info

8a8420c

Merge branch 'main' into BH/SSF-102-Update-User-Info

6411d03

Merge branch 'main' into BH/SSF-102-Update-User-Info

dc91943

update endpoints

3f6119c

update user entity

9df9b00

update service tests

9a7f68f

fix git tests

50ffbec

swarkewalia requested a review from dburkhart07 February 21, 2026 00:25

switch put to patch

4d9140d

dburkhart07 approved these changes Feb 21, 2026

View reviewed changes

nits

f9ae01b

sam-schu self-requested a review February 21, 2026 18:34

sam-schu requested changes Feb 21, 2026

View reviewed changes

		mockUserRepository.save.mockResolvedValue(updatedUser as User);

		const result = await service.update(1, dto);

		@@ -0,0 +1,24 @@
		import { IsString, IsPhoneNumber, IsOptional, IsNotEmpty, MaxLength } from 'class-validator';

		export class updateUserInfo {

Comments

Conversation

bhuvanh66 commented Dec 4, 2025

ℹ️ Issue

📝 Description

✔️ Verification

Uh oh!

dburkhart07 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dburkhart07 Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dburkhart07 Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dburkhart07 Feb 16, 2026 •

edited

Loading

dburkhart07 Feb 21, 2026 •

edited

Loading