From 838cda03f8c1356760334a3e8ddab8151c307f50 Mon Sep 17 00:00:00 2001 From: ChrisS1512 <87066931+ChrisS1512@users.noreply.github.com> Date: Fri, 13 Dec 2024 12:56:56 +0000 Subject: [PATCH] Revert "PUB-2249 - Refactor MI Endpoints to use Models" --- build.gradle | 2 +- .../CustomAccountRetrievalTest.java | 25 ++++++------ .../AccountFilteringController.java | 21 +++++++--- .../management/database/UserRepository.java | 9 ++--- .../service/AccountFilteringService.java | 15 ++++--- .../service/AccountFilteringServiceTest.java | 39 ++++++++++++------- 6 files changed, 64 insertions(+), 47 deletions(-) diff --git a/build.gradle b/build.gradle index 1e6bc5821..fb9ba4b07 100644 --- a/build.gradle +++ b/build.gradle @@ -221,7 +221,7 @@ dependencies { implementation group: 'com.opencsv', name: 'opencsv', version: '5.9' implementation group: 'commons-validator', name: 'commons-validator', version: '1.9.0' - implementation group: 'com.github.hmcts', name: 'pip-data-models', version: '2.1.33', { + implementation group: 'com.github.hmcts', name: 'pip-data-models', version: '2.1.32', { exclude group: 'org.springframework.boot', module: 'spring-boot-starter-data-jpa' } implementation group: 'io.hypersistence', name: 'hypersistence-utils-hibernate-63', version: '3.8.3' diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/CustomAccountRetrievalTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/CustomAccountRetrievalTest.java index 3db09916b..519cb421c 100644 --- a/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/CustomAccountRetrievalTest.java +++ b/src/integrationTest/java/uk/gov/hmcts/reform/pip/account/management/controllers/CustomAccountRetrievalTest.java @@ -23,15 +23,12 @@ import uk.gov.hmcts.reform.pip.account.management.model.PiUser; import uk.gov.hmcts.reform.pip.model.account.Roles; import uk.gov.hmcts.reform.pip.model.account.UserProvenances; -import uk.gov.hmcts.reform.pip.model.report.AccountMiData; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.springframework.http.HttpStatus.FORBIDDEN; import static org.springframework.http.HttpStatus.NOT_FOUND; @@ -62,10 +59,11 @@ class CustomAccountRetrievalTest { private static final String NOT_FOUND_STATUS_CODE_MESSAGE = "Status code does not match not found"; private static final String USER_SHOULD_MATCH = "Users should match"; private static final String FORBIDDEN_STATUS_CODE = "Status code does not match forbidden"; - private static final String VALIDATION_MI_REPORT = "Should successfully retrieve MI data"; private static final String UNAUTHORIZED_ROLE = "APPROLE_unknown.authorized"; private static final String UNAUTHORIZED_USERNAME = "unauthorized_isAuthorized"; + private static final String EXPECTED_HEADERS = "user_id,provenance_user_id,user_provenance,roles," + + "created_date,last_signed_in_date"; private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); private static final PiUser VALID_USER = createUser(true, UUID.randomUUID().toString()); @@ -108,23 +106,22 @@ void testMiAccountDataRequestSuccess() throws Exception { } ); - String createdUserId = mappedResponse.get(CreationEnum.CREATED_ACCOUNTS).getFirst().toString(); + String createdUserId = mappedResponse.get(CreationEnum.CREATED_ACCOUNTS).get(0).toString(); MockHttpServletRequestBuilder request = MockMvcRequestBuilders .get(MI_REPORTING_ACCOUNT_DATA_URL); - MvcResult responseMiData = mockMvc.perform(request).andExpect(status().isOk()).andReturn(); - - assertNotNull(responseMiData.getResponse(), VALIDATION_MI_REPORT); + String responseMiData = mockMvc.perform(request).andExpect(status().isOk()).andReturn() + .getResponse().getContentAsString(); - List miData = Arrays.asList( - OBJECT_MAPPER.readValue( - responseMiData.getResponse().getContentAsString(), - AccountMiData[].class - ) + assertEquals(EXPECTED_HEADERS, responseMiData.split("\n")[0], + "Should successfully retrieve MI data headers" ); - assertEquals(createdUserId, miData.getLast().getUserId().toString(), VALIDATION_MI_REPORT); + assertTrue( + responseMiData.contains(createdUserId), + "Should successfully retrieve MI data" + ); } @Test diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountFilteringController.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountFilteringController.java index 9c55d7f60..faceb0c4a 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountFilteringController.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/controllers/AccountFilteringController.java @@ -1,6 +1,9 @@ package uk.gov.hmcts.reform.pip.account.management.controllers; import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.media.Content; +import io.swagger.v3.oas.annotations.media.ExampleObject; +import io.swagger.v3.oas.annotations.media.Schema; import io.swagger.v3.oas.annotations.responses.ApiResponse; import io.swagger.v3.oas.annotations.security.SecurityRequirement; import io.swagger.v3.oas.annotations.tags.Tag; @@ -9,6 +12,7 @@ import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.GetMapping; @@ -22,7 +26,6 @@ import uk.gov.hmcts.reform.pip.model.account.Roles; import uk.gov.hmcts.reform.pip.model.account.UserProvenances; import uk.gov.hmcts.reform.pip.model.authentication.roles.IsAdmin; -import uk.gov.hmcts.reform.pip.model.report.AccountMiData; import java.util.List; @@ -47,11 +50,19 @@ public AccountFilteringController(AccountFilteringService accountFilteringServic this.accountFilteringService = accountFilteringService; } - @ApiResponse(responseCode = OK_CODE, description = "A JSON model which contains the data. ") - @Operation(summary = "Returns (anonymized) account data for MI reporting") + @ApiResponse(responseCode = OK_CODE, description = "A CSV like structure which contains the data. " + + "See example for headers ", content = { + @Content(examples = {@ExampleObject("user_id,provenance_user_id,user_provenance,roles," + + "created_date,last_signed_in_date")}, + mediaType = MediaType.TEXT_PLAIN_VALUE, + schema = @Schema(implementation = String.class)) + } + ) + @Operation(summary = "Returns a list of (anonymized) account data for MI reporting. This endpoint will be " + + "deprecated in the future, in favour of returning a JSON model") @GetMapping("/mi-data") - public ResponseEntity> getMiData() { - return ResponseEntity.status(HttpStatus.OK).body(accountFilteringService.getAccountDataForMi()); + public ResponseEntity getMiData() { + return ResponseEntity.status(HttpStatus.OK).body(accountFilteringService.getAccManDataForMiReporting()); } @ApiResponse(responseCode = OK_CODE, description = "List of third party accounts") diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java index f78e79773..22e3705bd 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/database/UserRepository.java @@ -10,7 +10,6 @@ import uk.gov.hmcts.reform.pip.account.management.model.PiUser; import uk.gov.hmcts.reform.pip.model.account.Roles; import uk.gov.hmcts.reform.pip.model.account.UserProvenances; -import uk.gov.hmcts.reform.pip.model.report.AccountMiData; import java.util.List; import java.util.Optional; @@ -25,10 +24,10 @@ List findExistingByProvenanceId(@Param("provUserId") String provenanceUs Optional findByUserId(UUID userId); - @Query("SELECT new uk.gov.hmcts.reform.pip.model.report.AccountMiData(" - + "u.userId, u.provenanceUserId, u.userProvenance, u.roles, u.createdDate, u.lastSignedInDate) " - + "FROM PiUser u") - List getAccountDataForMi(); + @Query(value = "SELECT cast(user_id as text), provenance_user_id, user_provenance, roles, created_date, " + + "last_signed_in_date FROM pi_user", + nativeQuery = true) + List getAccManDataForMI(); @Query(value = "SELECT * FROM pi_user WHERE CAST(last_verified_date AS DATE) = CURRENT_DATE - (interval '1' day)" + " * :daysAgo AND user_provenance = 'PI_AAD' AND roles = 'VERIFIED'", nativeQuery = true) diff --git a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringService.java b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringService.java index 246f8ebcc..8b1dac0e9 100644 --- a/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringService.java +++ b/src/main/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringService.java @@ -10,7 +10,6 @@ import uk.gov.hmcts.reform.pip.account.management.model.PiUser; import uk.gov.hmcts.reform.pip.model.account.Roles; import uk.gov.hmcts.reform.pip.model.account.UserProvenances; -import uk.gov.hmcts.reform.pip.model.report.AccountMiData; import java.util.List; @@ -27,13 +26,13 @@ public AccountFilteringService(UserRepository userRepository) { this.userRepository = userRepository; } - /** - * Method which will retrieve MI reporting data for all accounts. - * - * @return A list of MI Data objects for all accounts. - */ - public List getAccountDataForMi() { - return userRepository.getAccountDataForMi(); + public String getAccManDataForMiReporting() { + StringBuilder builder = new StringBuilder(85); + builder.append("user_id,provenance_user_id,user_provenance,roles,created_date,last_signed_in_date") + .append(System.lineSeparator()); + userRepository.getAccManDataForMI() + .forEach(line -> builder.append(line).append(System.lineSeparator())); + return builder.toString(); } /** diff --git a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringServiceTest.java b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringServiceTest.java index 2a354f3d4..842f4bf80 100644 --- a/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringServiceTest.java +++ b/src/test/java/uk/gov/hmcts/reform/pip/account/management/service/AccountFilteringServiceTest.java @@ -15,14 +15,13 @@ import uk.gov.hmcts.reform.pip.account.management.model.PiUser; import uk.gov.hmcts.reform.pip.model.account.Roles; import uk.gov.hmcts.reform.pip.model.account.UserProvenances; -import uk.gov.hmcts.reform.pip.model.report.AccountMiData; -import java.time.LocalDateTime; import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.UUID; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -32,7 +31,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static uk.gov.hmcts.reform.pip.model.account.Roles.ALL_NON_RESTRICTED_ADMIN_ROLES; -import static uk.gov.hmcts.reform.pip.model.account.Roles.INTERNAL_ADMIN_CTSC; import static uk.gov.hmcts.reform.pip.model.account.UserProvenances.PI_AAD; import static uk.gov.hmcts.reform.pip.model.account.UserProvenances.SSO; @@ -40,10 +38,11 @@ class AccountFilteringServiceTest { private static final String EMAIL = "test@hmcts.net"; private static final String ID = "1234"; - private static final UUID USER_ID = UUID.randomUUID(); - private static final LocalDateTime CREATED_DATE = LocalDateTime.of(2022,1, 19, 13, 45, 50); - private static final LocalDateTime LAST_SIGNED_IN = LocalDateTime.of(2023,1, 25, 14, 22, 43); + private static final List EXAMPLE_CSV = List.of( + "2fe899ff-96ed-435a-bcad-1411bbe96d2a,string,CFT_IDAM,INTERNAL_ADMIN_CTSC,2022-01-19 13:45:50.873778," + + "2023-01-25 14:22:56.434343"); private static final PiUser PI_USER = new PiUser(); + private static final String USER_NOT_FOUND_EXCEPTION_MESSAGE = "The exception when a user has not been found has been thrown"; private static final String RETURN_USER_ERROR = "Returned user does not match expected user"; @@ -64,14 +63,26 @@ static void setup() { @Test void testMiService() { - AccountMiData miRecord = new AccountMiData(USER_ID, ID, PI_AAD, INTERNAL_ADMIN_CTSC, - CREATED_DATE, LAST_SIGNED_IN); - List miData = List.of(miRecord, miRecord); - - when(userRepository.getAccountDataForMi()).thenReturn(miData); - List miReportData = accountFilteringService.getAccountDataForMi(); - - assertEquals(miData, miReportData, "Returned data does not match expected data"); + when(userRepository.getAccManDataForMI()).thenReturn(EXAMPLE_CSV); + String testString = accountFilteringService.getAccManDataForMiReporting(); + + assertThat(testString) + .as("Json parsing has probably failed") + .contains("CTSC") + .hasLineCount(2); + + String[] splitLineString = testString.split("(\r\n|\r|\n)"); + long countLine1 = splitLineString[0].chars().filter(character -> character == ',').count(); + + assertThat(splitLineString[0]) + .as("Header row does not match") + .isEqualTo("user_id,provenance_user_id,user_provenance,roles,created_date,last_signed_in_date"); + + assertThat(splitLineString) + .as("Data must be missing, are only headers printing? or Wrong data") + .hasSizeGreaterThanOrEqualTo(2) + .allSatisfy( + e -> assertThat(e.chars().filter(character -> character == ',').count()).isEqualTo(countLine1)); } @Test