Skip to content

Commit

Permalink
GAP-2454 | add helper function to check version without renaming vers…
Browse files Browse the repository at this point in the history
…ion to revision
  • Loading branch information
ConorFayleAND committed Mar 8, 2024
1 parent 56df1aa commit 19ce87d
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import gov.cabinetoffice.gap.adminbackend.dtos.GenericPostResponseDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.application.ApplicationFormSectionDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.application.ApplicationSectionOrderPatchDto;
import gov.cabinetoffice.gap.adminbackend.dtos.application.PatchSectionDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.application.PostSectionDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.errors.GenericErrorDTO;
import gov.cabinetoffice.gap.adminbackend.enums.SectionStatusEnum;
Expand Down Expand Up @@ -176,12 +177,12 @@ public ResponseEntity updateSectionStatus(final HttpServletRequest request,
@CheckSchemeOwnership
public ResponseEntity<Void> updateSectionTitle(final HttpServletRequest request,
final @PathVariable Integer applicationId, final @PathVariable String sectionId,
final @RequestBody @Validated PostSectionDTO sectionDTO) {
final @RequestBody @Validated PatchSectionDTO sectionDTO) {
if (Objects.equals(sectionId, "ELIGIBILITY") || Objects.equals(sectionId, "ESSENTIAL")) {
return new ResponseEntity(new GenericErrorDTO("You cannot update the title of a non-custom section"),
HttpStatus.BAD_REQUEST);
}
this.applicationFormSectionService.updateSectionTitle(applicationId, sectionId, sectionDTO.getSectionTitle());
this.applicationFormSectionService.updateSectionTitle(applicationId, sectionId, sectionDTO.getSectionTitle(), sectionDTO.getVersion());
logApplicationUpdatedEvent(request.getSession().getId(), applicationId);

return ResponseEntity.ok().build();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package gov.cabinetoffice.gap.adminbackend.dtos.application;

import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.NoArgsConstructor;

import javax.validation.constraints.NotBlank;
import javax.validation.constraints.Pattern;
import javax.validation.constraints.Size;

@Data
@AllArgsConstructor
@NoArgsConstructor
@Builder
public class PatchSectionDTO {

// following regex allows empty string, letters, spaces, apostrophes, commas, and
// hyphens.
// the reason it allows empty string is so that only the @NotBlank error message will
// return on empty string
@Pattern(regexp = "^(?![\\s\\S])|^[a-zA-Z\\s',-]+$",
message = "Section name must only use letters a to z, and special characters such as hyphens, spaces and apostrophes")
@NotBlank(message = "Enter a section name")
@Size(max = 250, message = "Your Section name must be 250 characters or less.")
private String sectionTitle;

private Integer version;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package gov.cabinetoffice.gap.adminbackend.exceptions;

import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ResponseStatus;

@ResponseStatus(HttpStatus.CONFLICT)
public class ConflictException extends RuntimeException {

public ConflictException() {
}

public ConflictException(String message) {
super(message);
}

public ConflictException(String message, Throwable cause) {
super(message, cause);
}

public ConflictException(Throwable cause) {
super(cause);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import gov.cabinetoffice.gap.adminbackend.enums.SectionStatusEnum;
import gov.cabinetoffice.gap.adminbackend.exceptions.FieldViolationException;
import gov.cabinetoffice.gap.adminbackend.exceptions.NotFoundException;
import gov.cabinetoffice.gap.adminbackend.exceptions.ConflictException;
import gov.cabinetoffice.gap.adminbackend.models.AdminSession;
import gov.cabinetoffice.gap.adminbackend.repositories.ApplicationFormRepository;
import gov.cabinetoffice.gap.adminbackend.utils.ApplicationFormUtils;
Expand Down Expand Up @@ -90,7 +91,7 @@ public void updateSectionStatus(final Integer applicationId, final String sectio
this.applicationFormRepository.save(applicationForm);
}

public void updateSectionTitle(final Integer applicationId, final String sectionId, final String title) {
public void updateSectionTitle(final Integer applicationId, final String sectionId, final String title, final Integer version) {

ApplicationFormEntity applicationForm = this.applicationFormRepository.findById(applicationId)
.orElseThrow(() -> new NotFoundException("Application with id " + applicationId + " does not exist"));
Expand All @@ -100,6 +101,8 @@ public void updateSectionTitle(final Integer applicationId, final String section
verifyUniqueSectionName(applicationForm, title);

applicationDefinition.getSectionById(sectionId).setSectionTitle(title.replace("\"", ""));

ApplicationFormUtils.verifyApplicationFormVersion(version, applicationForm);
ApplicationFormUtils.updateAuditDetailsAfterFormChange(applicationForm, false);
this.applicationFormRepository.save(applicationForm);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package gov.cabinetoffice.gap.adminbackend.utils;

import gov.cabinetoffice.gap.adminbackend.entities.ApplicationFormEntity;
import gov.cabinetoffice.gap.adminbackend.exceptions.ConflictException;
import gov.cabinetoffice.gap.adminbackend.models.AdminSession;

import java.time.Instant;
import java.util.Objects;

public class ApplicationFormUtils {

Expand All @@ -16,4 +18,10 @@ public static void updateAuditDetailsAfterFormChange(ApplicationFormEntity appli
applicationFormEntity.setVersion(applicationFormEntity.getVersion() + 1);
}

public static void verifyApplicationFormVersion(Integer version, ApplicationFormEntity applicationFormEntity) {
if (!Objects.equals(version, applicationFormEntity.getVersion())) {
throw new ConflictException("MULTIPLE_EDITORS");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import gov.cabinetoffice.gap.adminbackend.annotations.WithAdminSession;
import gov.cabinetoffice.gap.adminbackend.dtos.application.ApplicationSectionOrderPatchDto;
import gov.cabinetoffice.gap.adminbackend.dtos.application.PatchSectionDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.application.PostSectionDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.errors.FieldErrorsDTO;
import gov.cabinetoffice.gap.adminbackend.dtos.errors.GenericErrorDTO;
Expand Down Expand Up @@ -280,22 +281,21 @@ void updateSectionStatus_AccessDeniedTest() throws Exception {

@Test
void updateSectionTitle__HappyPath() throws Exception {

String sectionTitle = "sectionTitle";
PatchSectionDTO patchSectionDTO = new PatchSectionDTO().builder().sectionTitle("sectionTitle").version(1).build();

doNothing().when(ApplicationFormSectionsControllerTest.this.applicationFormSectionService)
.updateSectionTitle(any(), any(), any());
.updateSectionTitle(any(), any(), any(), any());

ApplicationFormSectionsControllerTest.this.mockMvc.perform(
patch("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + "CUSTOM_SECTION" + "/title")
.contentType(MediaType.APPLICATION_JSON).content(HelperUtils.asJsonString(sectionTitle)))
.contentType(MediaType.APPLICATION_JSON).content(HelperUtils.asJsonString(patchSectionDTO)))
.andExpect(status().isOk());
}

@Test
void updateSectionTitle__BadRequest__NoPayload() throws Exception {
doNothing().when(ApplicationFormSectionsControllerTest.this.applicationFormSectionService)
.updateSectionTitle(any(), any(), any());
.updateSectionTitle(any(), any(), any(), any());

ApplicationFormSectionsControllerTest.this.mockMvc
.perform(patch(
Expand All @@ -307,7 +307,7 @@ void updateSectionTitle__BadRequest__NoPayload() throws Exception {
void updateSectionTitle__BadRequest__NotACustomSection() throws Exception {
String sectionTitle = "sectionTitle";
doNothing().when(ApplicationFormSectionsControllerTest.this.applicationFormSectionService)
.updateSectionTitle(any(), any(), any());
.updateSectionTitle(any(), any(), any(), any());

ApplicationFormSectionsControllerTest.this.mockMvc.perform(
patch("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + "ELIGIBILITY" + "/title")
Expand All @@ -317,27 +317,27 @@ void updateSectionTitle__BadRequest__NotACustomSection() throws Exception {

@Test
void updateSectionTitle__NotFound() throws Exception {
String sectionTitle = "sectionTitle";
PatchSectionDTO patchSectionDTO = new PatchSectionDTO().builder().sectionTitle("sectionTitle").version(1).build();
doThrow(NotFoundException.class)
.when(ApplicationFormSectionsControllerTest.this.applicationFormSectionService)
.updateSectionTitle(any(), any(), any());
.updateSectionTitle(any(), any(), any(), any());

ApplicationFormSectionsControllerTest.this.mockMvc.perform(
patch("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + "CUSTOM_SECTION" + "/title")
.contentType(MediaType.APPLICATION_JSON).content(HelperUtils.asJsonString(sectionTitle)))
.contentType(MediaType.APPLICATION_JSON).content(HelperUtils.asJsonString(patchSectionDTO)))
.andExpect(status().isNotFound());
}

@Test
void updateSectionTitle__AccessDenied() throws Exception {
String sectionTitle = "sectionTitle";
PatchSectionDTO patchSectionDTO = new PatchSectionDTO().builder().sectionTitle("sectionTitle").version(1).build();
doThrow(AccessDeniedException.class)
.when(ApplicationFormSectionsControllerTest.this.applicationFormSectionService)
.updateSectionTitle(any(), any(), any());
.updateSectionTitle(any(), any(), any(), any());

ApplicationFormSectionsControllerTest.this.mockMvc.perform(
patch("/application-forms/" + SAMPLE_APPLICATION_ID + "/sections/" + "CUSTOM_SECTION" + "/title")
.contentType(MediaType.APPLICATION_JSON).content(HelperUtils.asJsonString(sectionTitle)))
.contentType(MediaType.APPLICATION_JSON).content(HelperUtils.asJsonString(patchSectionDTO)))
.andExpect(status().isForbidden());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
import gov.cabinetoffice.gap.adminbackend.dtos.application.PostSectionDTO;
import gov.cabinetoffice.gap.adminbackend.entities.ApplicationFormEntity;
import gov.cabinetoffice.gap.adminbackend.enums.SectionStatusEnum;
import gov.cabinetoffice.gap.adminbackend.exceptions.ApplicationFormException;
import gov.cabinetoffice.gap.adminbackend.exceptions.FieldViolationException;
import gov.cabinetoffice.gap.adminbackend.exceptions.ForbiddenException;
import gov.cabinetoffice.gap.adminbackend.exceptions.NotFoundException;
import gov.cabinetoffice.gap.adminbackend.exceptions.*;
import gov.cabinetoffice.gap.adminbackend.mappers.ApplicationFormMapper;
import gov.cabinetoffice.gap.adminbackend.mappers.ApplicationFormMapperImpl;
import gov.cabinetoffice.gap.adminbackend.models.AdminSession;
Expand Down Expand Up @@ -441,7 +438,7 @@ void updateSectionTitle_HappyPath() {

ArgumentCaptor<ApplicationFormEntity> argument = ArgumentCaptor.forClass(ApplicationFormEntity.class);

applicationFormSectionService.updateSectionTitle(SAMPLE_APPLICATION_ID, "1", newTitle);
applicationFormSectionService.updateSectionTitle(SAMPLE_APPLICATION_ID, "1", newTitle, 1);

Mockito.verify(ApplicationFormSectionServiceTest.this.applicationFormRepository).save(argument.capture());
}
Expand All @@ -454,7 +451,7 @@ void updateSectionTitle_NotFound() {
.thenReturn(Optional.empty());

assertThatThrownBy(() -> ApplicationFormSectionServiceTest.this.applicationFormSectionService
.updateSectionTitle(SAMPLE_APPLICATION_ID, "1", newTitle)).isInstanceOf(NotFoundException.class)
.updateSectionTitle(SAMPLE_APPLICATION_ID, "1", newTitle, 1)).isInstanceOf(NotFoundException.class)
.hasMessage("Application with id 111 does not exist");
}

Expand All @@ -466,7 +463,20 @@ void updateSectionTitle_throwsFieldErrorWithAMatchingSectionTitle() {
.thenReturn(Optional.of(testApplicationForm));

Assertions.assertThrows(FieldViolationException.class, () -> applicationFormSectionService
.updateSectionTitle(SAMPLE_APPLICATION_ID, "1", "Section title"));
.updateSectionTitle(SAMPLE_APPLICATION_ID, "1", "Section title", 1));
}

@Test
void updateSectionTitle_VersionDoesNotMatch() {
String newTitle = "newTitle";
ApplicationFormEntity testApplicationForm = randomApplicationFormEntity().version(2).build();
Mockito.when(
ApplicationFormSectionServiceTest.this.applicationFormRepository.findById(SAMPLE_APPLICATION_ID))
.thenReturn(Optional.of(testApplicationForm));

assertThatThrownBy(() -> ApplicationFormSectionServiceTest.this.applicationFormSectionService
.updateSectionTitle(SAMPLE_APPLICATION_ID, "1", newTitle, 1)).isInstanceOf(ConflictException.class)
.hasMessage("MULTIPLE_EDITORS");
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import gov.cabinetoffice.gap.adminbackend.annotations.WithAdminSession;
import gov.cabinetoffice.gap.adminbackend.entities.ApplicationFormEntity;
import gov.cabinetoffice.gap.adminbackend.exceptions.ConflictException;
import gov.cabinetoffice.gap.adminbackend.models.AdminSession;
import gov.cabinetoffice.gap.adminbackend.testdata.generators.RandomApplicationFormGenerators;
import org.junit.Before;
Expand All @@ -14,6 +15,8 @@
import java.time.Instant;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mockStatic;
Expand Down Expand Up @@ -54,7 +57,7 @@ void updateAuditDetailsAfterFormChange_UpdatingExpectedAuditDetails() {
}

@Test
void doesntCallSetLastUpdateByWhenIsLambdaEqualsTrue() {
void updateAuditDetailsAfterFormChange_DoesNotCallSetLastUpdateByWhenIsLambdaEqualsTrue() {
Instant fiveSecondsAgo = Instant.now().minusSeconds(5);
Integer version = 1;
ApplicationFormEntity applicationForm = Mockito.spy(RandomApplicationFormGenerators
Expand All @@ -66,4 +69,31 @@ void doesntCallSetLastUpdateByWhenIsLambdaEqualsTrue() {
assertEquals(Integer.valueOf(2), applicationForm.getVersion());
}

@Test
void verifyApplicationFormVersion_DoesNotThrowErrorWhenVersionMatches() {
Integer version = 2;
ApplicationFormEntity applicationForm = RandomApplicationFormGenerators
.randomApplicationFormEntity()
.version(version)
.build();

assertDoesNotThrow(() ->
ApplicationFormUtils.verifyApplicationFormVersion(version, applicationForm)
);
}

@Test
void verifyApplicationFormVersion_ThrowsErrorWhenVersionDoesNotMatch() {
Integer version = 2;
ApplicationFormEntity applicationForm = RandomApplicationFormGenerators
.randomApplicationFormEntity()
.version(version)
.build();

assertThrows(
ConflictException.class,
() -> ApplicationFormUtils.verifyApplicationFormVersion(1, applicationForm)
);
}

}

0 comments on commit 19ce87d

Please sign in to comment.