From ee1b0b08ba10e5c86281ef6986eeea516f8ab5ac Mon Sep 17 00:00:00 2001 From: john-tco <135222889+john-tco@users.noreply.github.com> Date: Wed, 27 Mar 2024 09:18:22 +0000 Subject: [PATCH] Multiple editors - handle editing recently deleted section (#265) * throw multiple editors exception on retrieve question --- .../services/ApplicationFormService.java | 17 ++++----- .../utils/ApplicationFormUtils.java | 12 +++++++ .../services/ApplicationFormServiceTest.java | 36 +++++++++++++++++-- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java index d8508288..d020f172 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormService.java @@ -12,7 +12,6 @@ import gov.cabinetoffice.gap.adminbackend.enums.ResponseTypeEnum; import gov.cabinetoffice.gap.adminbackend.enums.SessionObjectEnum; 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; @@ -238,13 +237,8 @@ public String addQuestionToApplicationForm(Integer applicationId, String section String questionId = UUID.randomUUID().toString(); this.applicationFormRepository.findById(applicationId).ifPresentOrElse(applicationForm -> { - ApplicationFormSectionDTO sectionDTO; - try { - sectionDTO = applicationForm.getDefinition().getSectionById(sectionId); - } catch (NotFoundException e) { - //If the section does not exist here it must have been recently deleted by another editor. - throw new ConflictException("MULTIPLE_EDITORS_SECTION_DELETED"); - } + ApplicationFormSectionDTO sectionDTO = + ApplicationFormUtils.verifyAndGetApplicationFormSection(applicationForm, sectionId); ApplicationFormQuestionDTO applicationFormQuestionDTO; QuestionAbstractPostDTO questionAbstractPostDTO = validatePostQuestion(question); @@ -328,7 +322,9 @@ public ApplicationFormQuestionDTO retrieveQuestion(Integer applicationId, String ApplicationFormEntity applicationForm = this.applicationFormRepository.findById(applicationId) .orElseThrow(() -> new NotFoundException("Application with id " + applicationId + " does not exist")); - return applicationForm.getDefinition().getSectionById(sectionId).getQuestionById(questionId); + ApplicationFormSectionDTO sectionDTO = ApplicationFormUtils.verifyAndGetApplicationFormSection(applicationForm, sectionId); + + return sectionDTO.getQuestionById(questionId); } @@ -374,10 +370,11 @@ public void updateQuestionOrder(final Integer applicationId, final String sectio .orElseThrow(() -> new NotFoundException( "Application with id " + applicationId + " does not exist or insufficient permissions")); + final ApplicationFormSectionDTO section = ApplicationFormUtils.verifyAndGetApplicationFormSection(applicationForm, sectionId); ApplicationFormUtils.verifyApplicationFormVersion(version, applicationForm); final List sections = applicationForm.getDefinition().getSections(); - final ApplicationFormSectionDTO section = applicationForm.getDefinition().getSectionById(sectionId); + final List questions = section.getQuestions(); final ApplicationFormQuestionDTO question = section.getQuestionById(questionId); diff --git a/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java b/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java index 93a65acf..18c110a1 100644 --- a/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java +++ b/src/main/java/gov/cabinetoffice/gap/adminbackend/utils/ApplicationFormUtils.java @@ -1,7 +1,9 @@ package gov.cabinetoffice.gap.adminbackend.utils; +import gov.cabinetoffice.gap.adminbackend.dtos.application.ApplicationFormSectionDTO; import gov.cabinetoffice.gap.adminbackend.entities.ApplicationFormEntity; import gov.cabinetoffice.gap.adminbackend.exceptions.ConflictException; +import gov.cabinetoffice.gap.adminbackend.exceptions.NotFoundException; import gov.cabinetoffice.gap.adminbackend.models.AdminSession; import java.time.Instant; @@ -23,4 +25,14 @@ public static void verifyApplicationFormVersion(Integer version, ApplicationForm } } + public static ApplicationFormSectionDTO verifyAndGetApplicationFormSection(ApplicationFormEntity applicationForm, String sectionId) { + ApplicationFormSectionDTO sectionDTO; + try { + sectionDTO = applicationForm.getDefinition().getSectionById(sectionId); + } catch (NotFoundException e) { + // If the section is not found it must have recently been deleted by another editor. + throw new ConflictException("MULTIPLE_EDITORS_SECTION_DELETED"); + } + return sectionDTO; + } } diff --git a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java index 338a5b2a..2383382b 100644 --- a/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/adminbackend/services/ApplicationFormServiceTest.java @@ -403,6 +403,9 @@ void addNewQuestionValuesGenericHappyPathTest() { .thenReturn(Optional.of(SAMPLE_APPLICATION_FORM_ENTITY)); doReturn(form).when(applicationFormService).save(any()); + utilMock.when(() -> ApplicationFormUtils.verifyAndGetApplicationFormSection(any(), any())) + .thenReturn(SAMPLE_APPLICATION_FORM_ENTITY.getDefinition().getSections().get(0)); + String questionId = applicationFormService.addQuestionToApplicationForm( SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, SAMPLE_QUESTION_GENERIC_POST_DTO, new MockHttpSession()); @@ -432,6 +435,9 @@ void addNewQuestionValuesOptionsHappyPathTest() { .thenReturn(Optional.of(SAMPLE_APPLICATION_FORM_ENTITY)); doReturn(form).when(applicationFormService).save(any()); + utilMock.when(() -> ApplicationFormUtils.verifyAndGetApplicationFormSection(any(), any())) + .thenReturn(SAMPLE_APPLICATION_FORM_ENTITY.getDefinition().getSections().get(0)); + String questionId = applicationFormService.addQuestionToApplicationForm( SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, SAMPLE_QUESTION_OPTIONS_POST_DTO, new MockHttpSession()); @@ -513,6 +519,9 @@ void addNewQuestionOptionValueConflictExceptionTest() { when(ApplicationFormServiceTest.this.applicationFormRepository.findById(SAMPLE_APPLICATION_ID)) .thenReturn(Optional.of(SAMPLE_EMPTY_APPLICATION_FORM_ENTITY)); + utilMock.when(() -> ApplicationFormUtils.verifyAndGetApplicationFormSection(any(), any())) + .thenThrow(new ConflictException("MULTIPLE_EDITORS_SECTION_DELETED")); + assertThatThrownBy(() -> applicationFormService .addQuestionToApplicationForm(SAMPLE_APPLICATION_ID, SAMPLE_SECTION_ID, SAMPLE_QUESTION_OPTIONS_CONTENT_INVALID_POST_DTO, session)) @@ -652,8 +661,8 @@ void getQuestionSectionNotFoundTest() { assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService .retrieveQuestion(SAMPLE_APPLICATION_ID, "differentId", SAMPLE_QUESTION_ID)) - .isInstanceOf(NotFoundException.class) - .hasMessage("Section with id differentId does not exist"); + .isInstanceOf(ConflictException.class) + .hasMessage("MULTIPLE_EDITORS_SECTION_DELETED"); } @@ -884,6 +893,7 @@ void updateQuestionOrderOutdatedVersion() { ArgumentCaptor argument = ArgumentCaptor.forClass(ApplicationFormEntity.class); ApplicationFormEntity testApplicationFormEntity = randomApplicationFormEntity().build(); + String sectionId = testApplicationFormEntity.getDefinition().getSections().get(0).getSectionId(); final Integer applicationId = testApplicationFormEntity.getGrantApplicationId(); final Integer increment = 1; @@ -894,11 +904,31 @@ void updateQuestionOrderOutdatedVersion() { doReturn(testApplicationFormEntity).when(applicationFormService).save(any()); assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService - .updateQuestionOrder(applicationId, SAMPLE_SECTION_ID, SAMPLE_QUESTION_ID, increment, SAMPLE_VERSION - 1)) + .updateQuestionOrder(applicationId,sectionId, SAMPLE_QUESTION_ID, increment, SAMPLE_VERSION - 1)) .isInstanceOf(ConflictException.class) .hasMessage("MULTIPLE_EDITORS"); } + @Test + void updateQuestionOrderSectionDeleted() { + ArgumentCaptor argument = ArgumentCaptor.forClass(ApplicationFormEntity.class); + + ApplicationFormEntity testApplicationFormEntity = randomApplicationFormEntity().build(); + + final Integer applicationId = testApplicationFormEntity.getGrantApplicationId(); + final Integer increment = 1; + + when(applicationFormRepository.findById(applicationId)) + .thenReturn(Optional.of(testApplicationFormEntity)); + + doReturn(testApplicationFormEntity).when(applicationFormService).save(any()); + + assertThatThrownBy(() -> ApplicationFormServiceTest.this.applicationFormService + .updateQuestionOrder(applicationId,SAMPLE_SECTION_ID, SAMPLE_QUESTION_ID, increment, SAMPLE_VERSION - 1)) + .isInstanceOf(ConflictException.class) + .hasMessage("MULTIPLE_EDITORS_SECTION_DELETED"); + } + } @Nested