GAP-2461 | implement delete question version check, fix and add tests

cabinetoffice · Mar 11, 2024 · 1308210 · 1308210
1 parent fa28e8a
commit 1308210
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 17 deletions.
diff --git a/...va/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormQuestionsController.java b/...va/gov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormQuestionsController.java
@@ -112,14 +112,14 @@ public ResponseEntity<GenericPostResponseDTO> postNewQuestion(HttpServletRequest
                     content = @Content(mediaType = "application/json")) })
     @CheckSchemeOwnership
     public ResponseEntity<Void> deleteQuestion(HttpServletRequest request, @PathVariable @NotNull Integer applicationId,
-            @PathVariable @NotBlank String sectionId, @PathVariable @NotBlank String questionId) {
+            @PathVariable @NotBlank String sectionId, @PathVariable @NotBlank String questionId, @RequestParam Integer version) {
         try {
             // don't allow admins to delete questions from mandatory sections
             if (Objects.equals(sectionId, "ELIGIBILITY") || Objects.equals(sectionId, "ESSENTIAL")) {
                 return new ResponseEntity(new GenericErrorDTO("You cannot delete mandatory sections"),
                         HttpStatus.BAD_REQUEST);
             }
-            this.applicationFormService.deleteQuestionFromSection(applicationId, sectionId, questionId);
+            this.applicationFormService.deleteQuestionFromSection(applicationId, sectionId, questionId, version);
 
             logApplicationUpdatedEvent(request.getRequestedSessionId(), applicationId);
 

diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java
@@ -288,11 +288,13 @@ private QuestionAbstractPostDTO validatePostQuestion(ApplicationFormQuestionDTO
 
     }
 
-    public void deleteQuestionFromSection(Integer applicationId, String sectionId, String questionId) {
+    public void deleteQuestionFromSection(Integer applicationId, String sectionId, String questionId, Integer version) {
 
         ApplicationFormEntity applicationForm = this.applicationFormRepository.findById(applicationId)
                 .orElseThrow(() -> new NotFoundException("Application with id " + applicationId + " does not exist"));
 
+        ApplicationFormUtils.verifyApplicationFormVersion(version, applicationForm);
+
         boolean questionDeleted = applicationForm.getDefinition().getSectionById(sectionId).getQuestions()
                 .removeIf(question -> Objects.equals(question.getQuestionId(), questionId));
 

diff --git a/...ov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormQuestionsControllerTest.java b/...ov/cabinetoffice/gap/adminbackend/controllers/ApplicationFormQuestionsControllerTest.java
@@ -211,10 +211,12 @@ void addQuestionWithOptionsHappyPathTest() throws Exception {
     void deleteQuestionHappyPathTest() throws Exception {
 
         doNothing().when(this.applicationFormService).deleteQuestionFromSection(SAMPLE_APPLICATION_ID,
-                SAMPLE_SECTION_ID, SAMPLE_QUESTION_ID);
+                SAMPLE_SECTION_ID, SAMPLE_QUESTION_ID, SAMPLE_VERSION);
 
-        this.mockMvc.perform(delete("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + SAMPLE_SECTION_ID
-                + "/questions/" + SAMPLE_QUESTION_ID)).andExpect(status().isOk());
+        this.mockMvc
+                .perform(delete("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + SAMPLE_SECTION_ID
+                        + "/questions/" + SAMPLE_QUESTION_ID + "?version=" + SAMPLE_VERSION))
+                .andExpect(status().isOk());
 
         verify(eventLogService).logApplicationUpdatedEvent(any(), anyString(), anyLong(),
                 eq(SAMPLE_APPLICATION_ID.toString()));
@@ -223,8 +225,9 @@ void deleteQuestionHappyPathTest() throws Exception {
     @Test
     void deleteQuestionDeleteFromMandatorySectionTest() throws Exception {
 
-        this.mockMvc.perform(delete(
-                "/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/ESSENTIAL/questions/" + SAMPLE_QUESTION_ID))
+        this.mockMvc
+                .perform(delete("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/ESSENTIAL/questions/"
+                        + SAMPLE_QUESTION_ID + "?version=" + SAMPLE_VERSION))
                 .andExpect(status().isBadRequest());
 
         verifyNoInteractions(eventLogService);
@@ -234,11 +237,11 @@ void deleteQuestionDeleteFromMandatorySectionTest() throws Exception {
     void deleteQuestionOrSectionDoesntExistTest() throws Exception {
 
         doThrow(new NotFoundException("Error message")).when(this.applicationFormService)
-                .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, "incorrectId");
+                .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, "incorrectId", SAMPLE_VERSION);
 
         this.mockMvc
                 .perform(delete("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + SAMPLE_SECTION_ID
-                        + "/questions/incorrectId"))
+                        + "/questions/incorrectId" + "?version=" + SAMPLE_VERSION))
                 .andExpect(status().isNotFound())
                 .andExpect(content().json(HelperUtils.asJsonString(new GenericErrorDTO("Error message"))));
 
@@ -249,10 +252,13 @@ void deleteQuestionOrSectionDoesntExistTest() throws Exception {
     void deleteQuestion_AccessDeniedTest() throws Exception {
 
         doThrow(new AccessDeniedException("Error message")).when(this.applicationFormService)
-                .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, "incorrectId");
+                .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, "incorrectId", SAMPLE_VERSION);
 
-        this.mockMvc.perform(delete("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + SAMPLE_SECTION_ID
-                + "/questions/incorrectId")).andExpect(status().isForbidden()).andExpect(content().string(""));
+        this.mockMvc
+                .perform(delete("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + SAMPLE_SECTION_ID
+                        + "/questions/incorrectId" + "?version=" + SAMPLE_VERSION))
+                .andExpect(status().isForbidden())
+                .andExpect(content().string(""));
 
         verifyNoInteractions(eventLogService);
     }

diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java
@@ -8,6 +8,7 @@
 import gov.cabinetoffice.gap.adminbackend.entities.ApplicationFormEntity;
 import gov.cabinetoffice.gap.adminbackend.enums.ApplicationStatusEnum;
 import gov.cabinetoffice.gap.adminbackend.exceptions.ApplicationFormException;
+import gov.cabinetoffice.gap.adminbackend.exceptions.ConflictException;
 import gov.cabinetoffice.gap.adminbackend.exceptions.FieldViolationException;
 import gov.cabinetoffice.gap.adminbackend.exceptions.NotFoundException;
 import gov.cabinetoffice.gap.adminbackend.mappers.ApplicationFormMapper;
@@ -519,7 +520,7 @@ void deleteQuestionHappyPathTest() {
                     .when(applicationFormService).save(any());
 
             applicationFormService.deleteQuestionFromSection(applicationId, sectionId,
-                    questionId);
+                    questionId, SAMPLE_VERSION);
 
             verify(applicationFormService).save(argument.capture());
             List<ApplicationFormQuestionDTO> questions = argument.getValue().getDefinition().getSectionById(sectionId)
@@ -541,7 +542,7 @@ void deleteQuestionApplicationNotFoundTest() {
                     .thenReturn(Optional.empty());
 
             assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService
-                    .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, SAMPLE_QUESTION_ID))
+                    .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, SAMPLE_QUESTION_ID, SAMPLE_VERSION))
                             .isInstanceOf(NotFoundException.class)
                             .hasMessage("Application with id " + SAMPLE_APPLICATION_ID + " does not exist");
 
@@ -555,7 +556,7 @@ void deleteQuestionSectionNotFound() {
                     .thenReturn(Optional.of(SAMPLE_APPLICATION_FORM_ENTITY_DELETE_SECTION));
 
             assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService
-                    .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, incorrectId, SAMPLE_QUESTION_ID))
+                    .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, incorrectId, SAMPLE_QUESTION_ID, SAMPLE_VERSION))
                             .isInstanceOf(NotFoundException.class)
                             .hasMessage("Section with id " + incorrectId + " does not exist");
 
@@ -569,12 +570,26 @@ void deleteQuestionQuestionNotFound() {
                     .thenReturn(Optional.of(SAMPLE_APPLICATION_FORM_ENTITY_DELETE_SECTION));
 
             assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService
-                    .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, incorrectId))
+                    .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, incorrectId, SAMPLE_VERSION))
                             .isInstanceOf(NotFoundException.class)
                             .hasMessage("Question with id " + incorrectId + " does not exist");
 
         }
 
+        @Test
+        void deleteQuestionQuestionVersionConflict() {
+            String incorrectId = "incorrectId";
+
+            Mockito.when(ApplicationFormServiceTest.this.applicationFormRepository.findById(SAMPLE_APPLICATION_ID))
+                    .thenReturn(Optional.of(SAMPLE_APPLICATION_FORM_ENTITY_DELETE_SECTION));
+
+            assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService
+                    .deleteQuestionFromSection(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, SAMPLE_QUESTION_ID, 2))
+                            .isInstanceOf(ConflictException.class)
+                            .hasMessage("MULTIPLE_EDITORS");
+
+        }
+
     }
 
     @Nested