fix: fix bugs in the endpoint for updating endorsements (#208)

* fix: ensure empty endorsements msg not allowed

* fix: add feature-flag in endorsement controller

* fix: update service impl to ensure no unauthorized endorsement update, user-not-found bug

* fix: enable disabled test cases

* fix: disable endorsements update when not in dev mode

* fix: run all tests in dev mode, add test case to validate updation is no allowed in non-dev mode

* fix: update name of test case

* fix: import only used exceptions

* fix: update control flow

Author: Shyam-Vishwakarma

skill-tree/src/main/java/com/RDS/skilltree/apis/EndorsementsApi.java

@@ -22,7 +22,9 @@ public class EndorsementsApi {
 
     @PatchMapping("/{id}")
     public ResponseEntity<EndorsementViewModel> update(
-            @PathVariable Integer id, @Valid @RequestBody UpdateEndorsementViewModel body) {
-        return new ResponseEntity<>(endorsementService.update(id, body), HttpStatus.OK);
+            @PathVariable Integer id,
+            @Valid @RequestBody UpdateEndorsementViewModel body,
+            @RequestParam(name = "dev", required = false, defaultValue = "false") boolean isDev) {
+        return new ResponseEntity<>(endorsementService.update(id, body, isDev), HttpStatus.OK);
     }
 }

skill-tree/src/main/java/com/RDS/skilltree/exceptions/GlobalExceptionHandler.java

@@ -140,4 +140,11 @@ public ResponseEntity<?> handleEndorsementAlreadyExistsException(
         return new ResponseEntity<>(
                 new GenericResponse<>(ex.getMessage()), HttpStatus.METHOD_NOT_ALLOWED);
     }
+
+    @ExceptionHandler(IllegalStateException.class)
+    public ResponseEntity<?> handleIllegalStateException(IllegalStateException ex) {
+        log.error("IllegalStateException - Error : {}", ex.getMessage());
+        return new ResponseEntity<>(
+                new GenericResponse<>(ex.getMessage()), HttpStatus.METHOD_NOT_ALLOWED);
+    }
 }

skill-tree/src/main/java/com/RDS/skilltree/services/EndorsementService.java

@@ -10,5 +10,6 @@ public interface EndorsementService {
 
     EndorsementViewModel create(CreateEndorsementViewModel endorsement);
 
-    EndorsementViewModel update(Integer endorsementId, UpdateEndorsementViewModel endorsement);
+    EndorsementViewModel update(
+            Integer endorsementId, UpdateEndorsementViewModel endorsement, boolean isDev);
 }

skill-tree/src/main/java/com/RDS/skilltree/services/EndorsementServiceImplementation.java

@@ -3,6 +3,7 @@
 import com.RDS.skilltree.dtos.RdsGetUserDetailsResDto;
 import com.RDS.skilltree.exceptions.EndorsementAlreadyExistsException;
 import com.RDS.skilltree.exceptions.EndorsementNotFoundException;
+import com.RDS.skilltree.exceptions.ForbiddenException;
 import com.RDS.skilltree.exceptions.SelfEndorsementNotAllowedException;
 import com.RDS.skilltree.exceptions.SkillNotFoundException;
 import com.RDS.skilltree.models.Endorsement;
@@ -130,30 +131,39 @@ public EndorsementViewModel create(CreateEndorsementViewModel endorsementViewMod
     }
 
     @Override
-    public EndorsementViewModel update(Integer endorsementId, UpdateEndorsementViewModel body) {
-        Optional<Endorsement> exitingEndorsement = endorsementRepository.findById(endorsementId);
-
-        if (exitingEndorsement.isEmpty()) {
-            log.info(String.format("Endorsement with id: %s not found", endorsementId));
-            throw new EndorsementNotFoundException(ExceptionMessages.ENDORSEMENT_NOT_FOUND);
+    public EndorsementViewModel update(
+            Integer endorsementId, UpdateEndorsementViewModel body, boolean isDev) {
+        if (isDev) {
+            Optional<Endorsement> existingEndorsement = endorsementRepository.findById(endorsementId);
+
+            if (existingEndorsement.isEmpty()) {
+                log.info("Endorsement with id: {} not found", endorsementId);
+                throw new EndorsementNotFoundException(ExceptionMessages.ENDORSEMENT_NOT_FOUND);
+            }
+
+            Endorsement endorsement = existingEndorsement.get();
+
+            JwtUser jwtDetails =
+                    (JwtUser) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
+            String userId = jwtDetails.getRdsUserId();
+
+            if (endorsement.getEndorserId().equals(userId)) {
+                RdsGetUserDetailsResDto endorseDetails =
+                        rdsService.getUserDetails(endorsement.getEndorseId());
+                RdsGetUserDetailsResDto endorserDetails = rdsService.getUserDetails(userId);
+
+                endorsement.setMessage(body.getMessage());
+                Endorsement savedEndorsementDetails = endorsementRepository.save(endorsement);
+
+                return EndorsementViewModel.toViewModel(
+                        savedEndorsementDetails,
+                        UserViewModel.toViewModel(endorseDetails.getUser()),
+                        UserViewModel.toViewModel(endorserDetails.getUser()));
+            } else {
+                log.warn("User: {} is not authorized to update endorsement: {}", userId, endorsementId);
+                throw new ForbiddenException(ExceptionMessages.UNAUTHORIZED_ENDORSEMENT_UPDATE);
+            }
         }
-
-        Endorsement endorsement = exitingEndorsement.get();
-        String updatedMessage = body.getMessage();
-
-        if (updatedMessage != null) {
-            endorsement.setMessage(updatedMessage);
-        }
-
-        Endorsement savedEndorsementDetails = endorsementRepository.save(endorsement);
-        RdsGetUserDetailsResDto endorseDetails =
-                rdsService.getUserDetails(savedEndorsementDetails.getEndorseId());
-        RdsGetUserDetailsResDto endorserDetails =
-                rdsService.getUserDetails(savedEndorsementDetails.getEndorserId());
-
-        return EndorsementViewModel.toViewModel(
-                savedEndorsementDetails,
-                UserViewModel.toViewModel(endorseDetails.getUser()),
-                UserViewModel.toViewModel(endorserDetails.getUser()));
+        throw new IllegalStateException(ExceptionMessages.UPDATE_DISABLED_IN_NON_DEV_MODE);
     }
 }

skill-tree/src/main/java/com/RDS/skilltree/utils/Constants.java

@@ -15,5 +15,7 @@ public static final class ExceptionMessages {
         public static final String INVALID_ACCESS_TOKEN =
                 "The access token provided is expired, revoked, malformed, or invalid for other reasons.";
         public static final String ACCESS_DENIED = "Access Denied";
+        public static final String UPDATE_DISABLED_IN_NON_DEV_MODE =
+                "Update is not allowed outside of development mode";
     }
 }

skill-tree/src/main/java/com/RDS/skilltree/viewmodels/UpdateEndorsementViewModel.java

@@ -1,13 +1,13 @@
 package com.RDS.skilltree.viewmodels;
 
 import com.RDS.skilltree.utils.Constants.ExceptionMessages;
-import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.NotBlank;
 import lombok.Getter;
 import lombok.Setter;
 
 @Getter
 @Setter
 public class UpdateEndorsementViewModel {
-    @NotNull(message = ExceptionMessages.ENDORSEMENT_MESSAGE_EMPTY)
+    @NotBlank(message = ExceptionMessages.ENDORSEMENT_MESSAGE_EMPTY)
     private String message;
 }

skill-tree/src/test/java/com/RDS/skilltree/integration/skills/UpdateEndorsementsIntegrationTest.java

@@ -108,7 +108,8 @@ private MvcResult performPatchRequest(String url, String requestBody) throws Exc
     }
 
     private String createUrl(Integer endorsementId) {
-        return String.format("/v1/endorsements/%d", endorsementId);
+        String isDev = "?dev=true";
+        return String.format("/v1/endorsements/%d" + isDev, endorsementId);
     }
 
     private UpdateEndorsementViewModel createRequestModel(String newMessage) {
@@ -192,7 +193,6 @@ public void updateEndorsement_whenEndorsementIdDoesNotExist_shouldReturnNotFound
     }
 
     @Test
-    @Disabled("Fails due to authorization bug tracked in #206 – re-enable once fixed")
     @DisplayName("when user is not the endorser, should not update endorsement")
     @WithCustomMockUser(
             username = userId1,
@@ -217,7 +217,6 @@ public void updateEndorsement_othersEndorsement_shouldNotUpdateEndorsement() thr
     }
 
     @Test
-    @Disabled("Fails due to validation bug tracked in #206 – re-enable once fixed")
     @DisplayName("Message is empty string, request is not valid")
     @WithCustomMockUser(
             username = userId1,
@@ -238,7 +237,6 @@ public void updateEndorsement_whenMessageIsValidAndEmpty_shouldReturnBadRequest(
     }
 
     @Test
-    @Disabled("Fails due to bug tracked in #206 – re-enable once fixed")
     @DisplayName("RdsService fails to get 'endorser' details, should return 404")
     @WithCustomMockUser(
             username = "non-existent-endorser-id",
@@ -267,7 +265,6 @@ public void updateEndorsement_whenRdsServiceFailsForEndorserDetails_shouldReturn
     }
 
     @Test
-    @Disabled("Fails due to bug tracked in #206 – re-enable once fixed")
     @DisplayName("RdsService fails to get 'endorse' details, should return 404")
     @WithCustomMockUser(
             username = userId1,
@@ -352,4 +349,25 @@ public void updateEndorsement_whenUserIsUnauthenticated_shouldReturn401() throws
         assertThat(result.getResponse().getContentAsString())
                 .contains(ExceptionMessages.INVALID_ACCESS_TOKEN);
     }
+
+    @Test
+    @DisplayName("Endorsement update not allowed in non-dev mode, should return 405")
+    @WithCustomMockUser(
+            username = userId1,
+            authorities = {"USER"})
+    public void updateEndorsement_nonDevMode_shouldReturn405() throws Exception {
+        Skill skill = createAndSaveSkill(SKILL_NAME);
+        Endorsement existingEndorsement =
+                createAndSaveEndorsement(skill, userId2, userId1, INITIAL_MESSAGE);
+
+        UpdateEndorsementViewModel updateEndorsementViewModel = createRequestModel(NEW_MESSAGE);
+        String updateBody = objectMapper.writeValueAsString(updateEndorsementViewModel);
+
+        String url = String.format("/v1/endorsements/%d", existingEndorsement.getId());
+        MvcResult result = performPatchRequest(url, updateBody);
+
+        assertThat(result.getResponse().getStatus()).isEqualTo(405);
+        assertThat(result.getResponse().getContentAsString())
+                .contains(ExceptionMessages.UPDATE_DISABLED_IN_NON_DEV_MODE);
+    }
 }