Skip to content

Commit 556ecd0

Browse files
committed
fix: address PR feedback for user API improvements
- Apply detached entity re-fetch pattern to updateUserAccount for consistency with updatePassword endpoint - Change hardcoded "User not found" error to use i18n getMessage() with fallback for consistency with other error messages - Update test mocks to use 4-argument getMessage() signature that includes default message parameter - Add findUserByEmail mocks to updateUser and updatePassword tests
1 parent 50e0a03 commit 556ecd0

2 files changed

Lines changed: 17 additions & 8 deletions

File tree

src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,12 @@ public ResponseEntity<JSONResponse> updateUserAccount(@AuthenticationPrincipal D
155155
@Valid @RequestBody UserProfileUpdateDto profileUpdateDto,
156156
HttpServletRequest request, Locale locale) {
157157
validateAuthenticatedUser(userDetails);
158-
User user = userDetails.getUser();
158+
// Re-fetch user from database to ensure we have an attached entity
159+
User user = userService.findUserByEmail(userDetails.getUser().getEmail());
160+
if (user == null) {
161+
log.error("User not found in database: {}", userDetails.getUser().getEmail());
162+
return buildErrorResponse(messages.getMessage("message.user.not-found", null, "User not found", locale), 1, HttpStatus.BAD_REQUEST);
163+
}
159164
user.setFirstName(profileUpdateDto.getFirstName());
160165
user.setLastName(profileUpdateDto.getLastName());
161166
userService.saveRegisteredUser(user);
@@ -276,7 +281,7 @@ public ResponseEntity<JSONResponse> updatePassword(@AuthenticationPrincipal DSUs
276281
User user = userService.findUserByEmail(userDetails.getUser().getEmail());
277282
if (user == null) {
278283
log.error("User not found in database: {}", userDetails.getUser().getEmail());
279-
return buildErrorResponse("User not found", 1, HttpStatus.BAD_REQUEST);
284+
return buildErrorResponse(messages.getMessage("message.user.not-found", null, "User not found", locale), 1, HttpStatus.BAD_REQUEST);
280285
}
281286

282287
try {

src/test/java/com/digitalsanctuary/spring/user/api/UserAPIUnitTest.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
400400
.setControllerAdvice(new TestExceptionHandler())
401401
.build();
402402

403-
when(messageSource.getMessage(eq("message.update-password.success"), any(), any(Locale.class)))
403+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
404+
when(messageSource.getMessage(eq("message.update-password.success"), any(), any(), any(Locale.class)))
404405
.thenReturn("Password updated successfully");
405406
when(userService.checkIfValidOldPassword(any(User.class), eq("oldPassword"))).thenReturn(true);
406407

@@ -443,7 +444,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
443444
.setControllerAdvice(new TestExceptionHandler())
444445
.build();
445446

446-
when(messageSource.getMessage(eq("message.update-password.invalid-old"), any(), any(Locale.class)))
447+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
448+
when(messageSource.getMessage(eq("message.update-password.invalid-old"), any(), any(), any(Locale.class)))
447449
.thenReturn("Invalid old password");
448450
when(userService.checkIfValidOldPassword(any(User.class), eq("wrongPassword"))).thenReturn(false);
449451

@@ -509,7 +511,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
509511
.setControllerAdvice(new TestExceptionHandler())
510512
.build();
511513

512-
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(Locale.class)))
514+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
515+
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(), any(Locale.class)))
513516
.thenReturn("Profile updated successfully");
514517
when(userService.saveRegisteredUser(any(User.class))).thenReturn(testUser);
515518

@@ -522,8 +525,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
522525
.andExpect(jsonPath("$.success").value(true))
523526
.andExpect(jsonPath("$.messages[0]").value("Profile updated successfully"));
524527

525-
verify(userService).saveRegisteredUser(argThat(user ->
526-
user.getFirstName().equals("UpdatedFirst") &&
528+
verify(userService).saveRegisteredUser(argThat(user ->
529+
user.getFirstName().equals("UpdatedFirst") &&
527530
user.getLastName().equals("UpdatedLast")
528531
));
529532
}
@@ -741,7 +744,8 @@ public Object resolveArgument(org.springframework.core.MethodParameter parameter
741744
.setControllerAdvice(new TestExceptionHandler())
742745
.build();
743746

744-
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(Locale.class)))
747+
when(userService.findUserByEmail(testUser.getEmail())).thenReturn(testUser);
748+
when(messageSource.getMessage(eq("message.update-user.success"), any(), any(), any(Locale.class)))
745749
.thenReturn("Profile updated successfully");
746750
when(userService.saveRegisteredUser(any(User.class))).thenReturn(testUser);
747751

0 commit comments

Comments
 (0)