fix: address review comments

Health-Education-England · Jan 20, 2025 · 6d7f575 · 6d7f575
1 parent ab30090
commit 6d7f575
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 33 deletions.
diff --git a/tcs-client/src/main/java/com/transformuk/hee/tis/tcs/client/service/impl/TcsServiceImpl.java b/tcs-client/src/main/java/com/transformuk/hee/tis/tcs/client/service/impl/TcsServiceImpl.java
@@ -82,7 +82,6 @@ public class TcsServiceImpl extends AbstractClientService {
   private static final String API_RIGHT_TO_WORKS = "/api/right-to-works/";
   private static final String API_PROGRAMME_MEMBERSHIPS = "/api/programme-memberships/";
   private static final String API_CURRICULUM_MEMBERSHIPS = "/api/curriculum-memberships";
-  private static final String API_CURRICULUM_MEMBERSHIP = "/api/curriculum-membership";
   private static final String API_TRAINEE_PLACEMENTS = "/api/people/%d/placements/new";
   private static final String API_SPECIALTIES = "/api/specialties/";
   private static final String API_CURRENT_SPECIALTIES_COLUMN_FILTERS =
@@ -514,11 +513,11 @@ public CurriculumMembershipDTO createCurriculumMembership(CurriculumMembershipDT
    * @param cmDto the Curriculum Membership dto to patch
    * @return the updated Curriculum Membership dto
    */
-  public CurriculumMembershipDTO updateCurriculumMembership(CurriculumMembershipDTO cmDto) {
+  public CurriculumMembershipDTO patchCurriculumMembership(CurriculumMembershipDTO cmDto) {
     HttpHeaders headers = new HttpHeaders();
     HttpEntity<CurriculumMembershipDTO> httpEntity = new HttpEntity<>(cmDto, headers);
     return tcsRestTemplate
-        .exchange(serviceUrl + API_CURRICULUM_MEMBERSHIP, HttpMethod.PATCH, httpEntity,
+        .exchange(serviceUrl + API_CURRICULUM_MEMBERSHIPS, HttpMethod.PATCH, httpEntity,
             new ParameterizedTypeReference<CurriculumMembershipDTO>() {
             })
         .getBody();

diff --git a/...ent/src/test/java/com/transformuk/hee/tis/tcs/client/service/impl/TcsServiceImplTest.java b/...ent/src/test/java/com/transformuk/hee/tis/tcs/client/service/impl/TcsServiceImplTest.java
@@ -404,10 +404,10 @@ public void getGmcDetailsByIdShouldThrowErrorWhenNotFound() {
   }
 
   @Test
-  public void updateCurriculumMembershipShouldReturnSavedDto() {
+  public void patchCurriculumMembershipShouldReturnSavedDto() {
     CurriculumMembershipDTO dto = new CurriculumMembershipDTO();
 
-    String url = "http://localhost:9999/tcs/api/curriculum-membership";
+    String url = "http://localhost:9999/tcs/api/curriculum-memberships";
 
     HttpHeaders headers = new HttpHeaders();
     HttpEntity<CurriculumMembershipDTO> httpEntity = new HttpEntity<>(dto, headers);
@@ -416,7 +416,7 @@ public void updateCurriculumMembershipShouldReturnSavedDto() {
         new ParameterizedTypeReference<CurriculumMembershipDTO>() {
         });
 
-    CurriculumMembershipDTO result = testObj.updateCurriculumMembership(dto);
+    CurriculumMembershipDTO result = testObj.patchCurriculumMembership(dto);
 
     assertThat("Unexpected result", result, is(dto));
     verify(restTemplate).exchange(url, HttpMethod.PATCH, httpEntity,

diff --git a/...e/src/main/java/com/transformuk/hee/tis/tcs/service/api/CurriculumMembershipResource.java b/...e/src/main/java/com/transformuk/hee/tis/tcs/service/api/CurriculumMembershipResource.java
@@ -2,6 +2,7 @@
 
 import com.transformuk.hee.tis.tcs.api.dto.CurriculumMembershipDTO;
 import com.transformuk.hee.tis.tcs.api.dto.validation.Create;
+import com.transformuk.hee.tis.tcs.api.dto.validation.Update;
 import com.transformuk.hee.tis.tcs.service.api.util.HeaderUtil;
 import com.transformuk.hee.tis.tcs.service.api.validation.CurriculumMembershipValidator;
 import com.transformuk.hee.tis.tcs.service.service.CurriculumMembershipService;
@@ -71,15 +72,22 @@ public ResponseEntity<CurriculumMembershipDTO> createCurriculumMembership(
    * @param curriculumMembershipDto the dto to patch
    * @return the ResponseEntity with status 200 (OK) and with body the patched dto
    */
-  @PatchMapping("/curriculum-membership")
+  @PatchMapping("/curriculum-memberships")
   @PreAuthorize("hasPermission('tis:people::person:', 'Update')")
   public ResponseEntity<CurriculumMembershipDTO> patchCurriculumMembership(
-      @RequestBody CurriculumMembershipDTO curriculumMembershipDto) {
-    log.debug("REST request to patch curriculum membership via bulk upload : {}",
-        curriculumMembershipDto);
+      @RequestBody @Validated(Update.class) CurriculumMembershipDTO curriculumMembershipDto)
+      throws MethodArgumentNotValidException, NoSuchMethodException {
+    log.debug("REST request to patch CurriculumMembership : {}", curriculumMembershipDto);
 
+    if (curriculumMembershipDto.getId() == null) {
+      return ResponseEntity.badRequest()
+          .headers(HeaderUtil.createFailureAlert(ENTITY_NAME, "id_missing",
+              "The ID is required for patching an existing curriculumMembership.")).body(null);
+    }
+    cmValidator.validate(curriculumMembershipDto);
     CurriculumMembershipDTO result = cmService.patch(curriculumMembershipDto);
-    return ResponseEntity.ok().headers(HeaderUtil.createEntityUpdateAlert(ENTITY_NAME,
-        curriculumMembershipDto.getId().toString())).body(result);
+    return ResponseEntity.ok()
+        .headers(HeaderUtil.createEntityUpdateAlert(ENTITY_NAME, curriculumMembershipDto.getId().toString()))
+        .body(result);
   }
 }
diff --git a/...rc/main/java/com/transformuk/hee/tis/tcs/service/service/CurriculumMembershipService.java b/...rc/main/java/com/transformuk/hee/tis/tcs/service/service/CurriculumMembershipService.java
@@ -17,5 +17,4 @@ public interface CurriculumMembershipService {
 
   CurriculumMembershipDTO patch(CurriculumMembershipDTO cmDto);
 
-  CurriculumMembershipDTO findOne(Long id);
 }
diff --git a/...ava/com/transformuk/hee/tis/tcs/service/service/impl/CurriculumMembershipServiceImpl.java b/...ava/com/transformuk/hee/tis/tcs/service/service/impl/CurriculumMembershipServiceImpl.java
@@ -39,15 +39,6 @@ public CurriculumMembershipServiceImpl(CurriculumMembershipRepository cmReposito
     this.pmRepository = pmRepository;
   }
 
-  @Override
-  @Transactional(readOnly = true)
-  public CurriculumMembershipDTO findOne(Long id) {
-    log.debug("Request to get CurriculumMembership : {}", id);
-    CurriculumMembership curriculumMembership = cmRepository.findById(id)
-        .orElse(null);
-    return cmMapper.curriculumMembershipToCurriculumMembershipDto(curriculumMembership);
-  }
-
   /**
    * Save a curriculumMembership.
    *
@@ -76,21 +67,26 @@ public CurriculumMembershipDTO save(CurriculumMembershipDTO cmDto) {
   @Transactional
   public CurriculumMembershipDTO patch(CurriculumMembershipDTO cmDto) {
     log.debug("Request to patch CurriculumMembership : {}", cmDto);
-    CurriculumMembershipDTO curriculumMembershipDtoFromDb = findOne(cmDto.getId());
-    if (curriculumMembershipDtoFromDb == null) {
+    CurriculumMembership curriculumMembershipDb = cmRepository.findById(cmDto.getId())
+        .orElse(null);
+    if (curriculumMembershipDb == null) {
       cmDto.addMessage("Curriculum membership id not found.");
       return cmDto;
     }
-    CurriculumMembership cm = cmMapper.toEntity(cmDto);
     ProgrammeMembership pm = pmRepository.findByUuid(cmDto.getProgrammeMembershipUuid())
         .orElse(null);
     if (pm == null) {
       cmDto.addMessage("Programme membership id not found.");
       return cmDto;
     }
-
-    cm.setProgrammeMembership(pm);
-    CurriculumMembership returnedCm = cmRepository.save(cm);
+    if(cmDto.getCurriculumStartDate() != null) {
+      curriculumMembershipDb.setCurriculumStartDate(cmDto.getCurriculumStartDate());
+    }
+    if(cmDto.getCurriculumEndDate() != null) {
+      curriculumMembershipDb.setCurriculumEndDate(cmDto.getCurriculumEndDate());
+    }
+    curriculumMembershipDb.setProgrammeMembership(pm);
+    CurriculumMembership returnedCm = cmRepository.save(curriculumMembershipDb);
     return cmMapper.curriculumMembershipToCurriculumMembershipDto(returnedCm);
   }
 }
diff --git a/...c/test/java/com/transformuk/hee/tis/tcs/service/api/CurriculumMembershipResourceTest.java b/...c/test/java/com/transformuk/hee/tis/tcs/service/api/CurriculumMembershipResourceTest.java
@@ -133,7 +133,7 @@ void shouldCreateCurriculumMembership() throws Exception {
 
   @Test
   @Transactional
-  void shouldUpdateCurriculumMembership() throws Exception {
+  void shouldPatchCurriculumMembership() throws Exception {
     personRepository.saveAndFlush(person);
     curriculum.setStatus(Status.CURRENT);
     cmRepository.saveAndFlush(curriculum);
@@ -159,7 +159,7 @@ void shouldUpdateCurriculumMembership() throws Exception {
     cmDto.setCurriculumEndDate(END_DATE_2);
     cmDto.setProgrammeMembershipUuid(programmeMembership.getUuid());
 
-    restCmMockMvc.perform(patch("/api/curriculum-membership")
+    restCmMockMvc.perform(patch("/api/curriculum-memberships")
             .contentType(MediaType.APPLICATION_JSON)
             .content(TestUtil.convertObjectToJsonBytes(cmDto)))
         .andDo(print())

diff --git a/...ava/com/transformuk/hee/tis/tcs/service/service/impl/CurriculumMembershipServiceTest.java b/...ava/com/transformuk/hee/tis/tcs/service/service/impl/CurriculumMembershipServiceTest.java
@@ -120,7 +120,6 @@ void shouldPatchCmDto() {
     cm.setCurriculumStartDate(START_DATE);
     cm.setCurriculumEndDate(END_DATE);
     cm.setCurriculumId(CURRICULUM_ID);
-    when(cmMapper.toEntity(dto)).thenReturn(cm);
 
     ProgrammeMembership pm = new ProgrammeMembership();
     pm.setUuid(PM_UUID);
@@ -151,7 +150,7 @@ void shouldPatchCmDto() {
   void shouldReturnMessageWhenCurriculumMembershipNotFound() {
     CurriculumMembershipDTO cmDto = new CurriculumMembershipDTO();
     cmDto.setId(CM_ID);
-    when(cmService.findOne(CM_ID)).thenReturn(null);
+    when(cmRepository.findById(CM_ID)).thenReturn(Optional.empty());
 
    CurriculumMembershipDTO result = cmService.patch(cmDto);
 
@@ -165,13 +164,84 @@ void shouldReturnMessageWhenProgrammeMembershipNotFound() {
     cmDto.setId(CM_ID);
     cmDto.setProgrammeMembershipUuid(PM_UUID);
 
+    CurriculumMembership returnedCm = new CurriculumMembership();
+    returnedCm.setId(CURRICULUM_ID);
+    returnedCm.setCurriculumStartDate(START_DATE);
+    returnedCm.setCurriculumEndDate(END_DATE);
+    returnedCm.setCurriculumId(CURRICULUM_ID);
+    returnedCm.setId(CM_ID);
+
     CurriculumMembershipDTO fromDb = new CurriculumMembershipDTO();
-    when(cmService.findOne(CM_ID)).thenReturn(fromDb);
+    when(cmRepository.findById(CM_ID)).thenReturn(Optional.of(returnedCm));
     when(pmRepository.findByUuid(PM_UUID)).thenReturn(Optional.empty());
 
     CurriculumMembershipDTO result = cmService.patch(cmDto);
 
     assertEquals("Programme membership id not found.", result.getMessageList().get(0));
     verify(cmRepository, never()).save(any());
   }
+
+  @Test
+  void shouldKeepDatabaseCurriculumMembershipStartAndEndDateWhenTheyAreEmptyInTemplate() {
+    CurriculumMembershipDTO cmDto = new CurriculumMembershipDTO();
+    cmDto.setId(CM_ID);
+    cmDto.setCurriculumStartDate(null);
+    cmDto.setCurriculumEndDate(null);
+    cmDto.setProgrammeMembershipUuid(PM_UUID);
+
+    CurriculumMembership returnedCm = new CurriculumMembership();
+    returnedCm.setId(CURRICULUM_ID);
+    returnedCm.setCurriculumStartDate(START_DATE);
+    returnedCm.setCurriculumEndDate(END_DATE);
+    returnedCm.setCurriculumId(CURRICULUM_ID);
+    returnedCm.setId(CM_ID);
+
+    ProgrammeMembership pm = new ProgrammeMembership();
+    pm.setUuid(PM_UUID);
+
+    when(cmRepository.save(any(CurriculumMembership.class))).thenReturn(returnedCm);
+    CurriculumMembershipDTO returnedCmDto = createDto(CURRICULUM_ID, PM_UUID, START_DATE, END_DATE);
+    returnedCmDto.setId(CM_ID);
+    when(cmMapper.curriculumMembershipToCurriculumMembershipDto(returnedCm)).thenReturn(
+        returnedCmDto);
+    when(cmRepository.findById(CM_ID)).thenReturn(Optional.of(returnedCm));
+    when(pmRepository.findByUuid(PM_UUID)).thenReturn(Optional.of(pm));
+
+    CurriculumMembershipDTO result = cmService.patch(cmDto);
+
+    assertEquals(returnedCm.getCurriculumEndDate(), result.getCurriculumEndDate());
+    assertEquals(returnedCm.getCurriculumStartDate(), result.getCurriculumStartDate());
+  }
+
+  @Test
+  void shouldKeepDatabaseCurriculumMembershipStartWhenTheyAreEmptyInTemplate() {
+    CurriculumMembershipDTO cmDto = new CurriculumMembershipDTO();
+    cmDto.setId(CM_ID);
+    cmDto.setCurriculumStartDate(null);
+    cmDto.setCurriculumEndDate(END_DATE);
+    cmDto.setProgrammeMembershipUuid(PM_UUID);
+
+    CurriculumMembership returnedCm = new CurriculumMembership();
+    returnedCm.setId(CURRICULUM_ID);
+    returnedCm.setCurriculumStartDate(START_DATE);
+    returnedCm.setCurriculumEndDate(END_DATE);
+    returnedCm.setCurriculumId(CURRICULUM_ID);
+    returnedCm.setId(CM_ID);
+
+    ProgrammeMembership pm = new ProgrammeMembership();
+    pm.setUuid(PM_UUID);
+
+    when(cmRepository.save(any(CurriculumMembership.class))).thenReturn(returnedCm);
+    CurriculumMembershipDTO returnedCmDto = createDto(CURRICULUM_ID, PM_UUID, START_DATE, END_DATE);
+    returnedCmDto.setId(CM_ID);
+    when(cmMapper.curriculumMembershipToCurriculumMembershipDto(returnedCm)).thenReturn(
+        returnedCmDto);
+    when(cmRepository.findById(CM_ID)).thenReturn(Optional.of(returnedCm));
+    when(pmRepository.findByUuid(PM_UUID)).thenReturn(Optional.of(pm));
+
+    CurriculumMembershipDTO result = cmService.patch(cmDto);
+
+    assertEquals(returnedCm.getCurriculumEndDate(), result.getCurriculumEndDate());
+    assertEquals(returnedCm.getCurriculumStartDate(), result.getCurriculumStartDate());
+  }
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -17,5 +17,4 @@ public interface CurriculumMembershipService {

		CurriculumMembershipDTO patch(CurriculumMembershipDTO cmDto);

		CurriculumMembershipDTO findOne(Long id);
		}