diff --git a/clients/node/README.md b/clients/node/README.md index 6e5143d5..27e38a4f 100644 --- a/clients/node/README.md +++ b/clients/node/README.md @@ -110,6 +110,7 @@ General notes regarding permissions and roles: - Users also having the `GLOBAL_SEARCH` role can also _add_, _update_ and _close_ non-associations for prisoners in transfer and where one prisoner is not in a prison that’s not in their caseloads - Users also having the `INACTIVE_BOOKINGS` role can also _add_, _update_ and _close_ non-associations for prisoners outside any establishment / released - Users must _close_ rather than _delete_ non-associations +- No users should be able to _add_, _update_ or _close_ non-associations for prisoners without a booking / with a null location Release a new version --------------------- diff --git a/clients/node/src/errorTypes.ts b/clients/node/src/errorTypes.ts index 1065bd5b..c1197e09 100644 --- a/clients/node/src/errorTypes.ts +++ b/clients/node/src/errorTypes.ts @@ -2,7 +2,7 @@ * Structure representing an error response from the api, wrapped in SanitisedError. * * Defined in uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.ErrorResponse class - * https://github.com/ministryofjustice/hmpps-non-associations-api/blob/f6002aa1da50b8c4ccd3613e970327d5c67c44ae/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt#L240-L261 + * see https://github.com/ministryofjustice/hmpps-non-associations-api */ export interface ErrorResponse { status: number @@ -34,11 +34,12 @@ export function isErrorResponse(obj: unknown): obj is ErrorResponse { * Unique codes to discriminate errors returned from the api. * * Defined in uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.ErrorCode enumeration - * https://github.com/ministryofjustice/hmpps-non-associations-api/blob/f6002aa1da50b8c4ccd3613e970327d5c67c44ae/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt#L227-L238 + * see https://github.com/ministryofjustice/hmpps-non-associations-api */ export enum ErrorCode { NonAssociationAlreadyClosed = 100, OpenNonAssociationAlreadyExist = 101, ValidationFailure = 102, + NullPrisonerLocations = 103, UserInContextMissing = 401, } diff --git a/clients/node/src/responseTypes.ts b/clients/node/src/responseTypes.ts index 85ff024d..568dcb63 100644 --- a/clients/node/src/responseTypes.ts +++ b/clients/node/src/responseTypes.ts @@ -26,8 +26,8 @@ interface BaseNonAssociationsListItem extends ObjectWithDates { roleDescription: Role[keyof Role] firstName: string lastName: string - prisonId: string - prisonName: string + prisonId?: string + prisonName?: string cellLocation?: string } } @@ -58,8 +58,8 @@ export interface NonAssociationsList< prisonerNumber: string firstName: string lastName: string - prisonId: string - prisonName: string + prisonId?: string + prisonName?: string cellLocation?: string openCount: number closedCount: number diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt index 5293ae77..7fbdb2ad 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/config/HmppsNonAssociationsApiExceptionHandler.kt @@ -219,6 +219,21 @@ class HmppsNonAssociationsApiExceptionHandler { ) } + @ExceptionHandler(NullPrisonerLocationsException::class) + fun handleNullPrisonerLocationsException(e: NullPrisonerLocationsException): ResponseEntity? { + log.debug("Non-association cannot be created when prisoner locations are null: {}", e.message) + return ResponseEntity + .status(CONFLICT) + .body( + ErrorResponse( + status = CONFLICT, + errorCode = ErrorCode.NullPrisonerLocations, + userMessage = "Non-association cannot be created when prisoner locations are null: ${e.message}", + developerMessage = e.message, + ), + ) + } + companion object { private val log = LoggerFactory.getLogger(this::class.java) } @@ -235,6 +250,7 @@ enum class ErrorCode(val errorCode: Int) { UserInContextMissing(401), OpenNonAssociationAlreadyExist(101), ValidationFailure(102), + NullPrisonerLocations(103), } @Schema(description = "Error response") @@ -267,3 +283,5 @@ class UserInContextMissingException : Exception("There is no user in context for class OpenNonAssociationAlreadyExistsException(prisoners: List) : Exception("Prisoners $prisoners already have open non-associations") class NonAssociationNotFoundException(id: Long) : Exception("There is no non-association found for ID = $id") + +class NullPrisonerLocationsException(prisoners: List) : Exception("Prisoners $prisoners have null locations") diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/LegacyPrisonerNonAssociations.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/LegacyPrisonerNonAssociations.kt index 4e58cdfb..aec6b351 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/LegacyPrisonerNonAssociations.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/LegacyPrisonerNonAssociations.kt @@ -14,10 +14,10 @@ data class LegacyPrisonerNonAssociations( val firstName: String, @Schema(description = "Last name", required = true, example = "Hall") val lastName: String, - @Schema(description = "Prison ID where prisoner resides or OUT for released", required = true, example = "MDI") - val agencyId: String, - @Schema(description = "Description of the agency (e.g. prison) the offender is assigned to", required = true, example = "Moorland (HMP & YOI)") - val agencyDescription: String, + @Schema(description = "Prison ID where prisoner resides or OUT for released", required = false, example = "MDI") + val agencyId: String?, + @Schema(description = "Description of the agency (e.g. prison) the offender is assigned to", required = false, example = "Moorland (HMP & YOI)") + val agencyDescription: String?, @Schema(description = "Description of living unit (e.g. cell) the offender is assigned to.", required = false, example = "MDI-1-1-3") val assignedLivingUnitDescription: String?, @Schema(description = "Non-associations with other prisoners", required = true) @@ -64,10 +64,10 @@ data class LegacyOtherPrisonerDetails( val reasonCode: LegacyReason, @Schema(description = "Reason for the non-association", required = true, example = "Perpetrator") val reasonDescription: String, - @Schema(description = "Prison ID where prisoner resides or OUT for released", required = true, example = "MDI") - val agencyId: String, - @Schema(description = "Description of the agency (e.g. prison) the offender is assigned to", required = true, example = "Moorland (HMP & YOI)") - val agencyDescription: String, + @Schema(description = "Prison ID where prisoner resides or OUT for released", required = false, example = "MDI") + val agencyId: String?, + @Schema(description = "Description of the agency (e.g. prison) the offender is assigned to", required = false, example = "Moorland (HMP & YOI)") + val agencyDescription: String?, @Schema(description = "Description of living unit (e.g. cell) the offender is assigned to.", required = false, example = "MDI-2-3-4") val assignedLivingUnitDescription: String?, ) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/PrisonerNonAssociations.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/PrisonerNonAssociations.kt index 86f0f031..7e34dfbc 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/PrisonerNonAssociations.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/PrisonerNonAssociations.kt @@ -16,10 +16,10 @@ data class PrisonerNonAssociations( val firstName: String, @Schema(description = "Last name", required = true, example = "Hall") val lastName: String, - @Schema(description = "ID of the prison the prisoner is assigned to", required = true, example = "MDI") - val prisonId: String, - @Schema(description = "Name of the prison the prisoner is assigned to", required = true, example = "Moorland (HMP & YOI)") - val prisonName: String, + @Schema(description = "ID of the prison the prisoner is assigned to", required = false, example = "MDI") + val prisonId: String?, + @Schema(description = "Name of the prison the prisoner is assigned to", required = false, example = "Moorland (HMP & YOI)") + val prisonName: String?, @Schema(description = "Cell the prisoner is assigned to", required = false, example = "A-1-002") val cellLocation: String?, @Schema(description = "Number of open non-associations (follows includeOtherPrisons filter)", required = true, example = "1", minimum = "0", type = "integer", format = "int32") @@ -93,10 +93,10 @@ data class OtherPrisonerDetails( val firstName: String, @Schema(description = "Last name", required = true, example = "Bloggs") val lastName: String, - @Schema(description = "ID of the prison the prisoner is assigned to", required = true, example = "MDI") - val prisonId: String, - @Schema(description = "Name of the prison the prisoner is assigned to", required = true, example = "Moorland (HMP & YOI)") - val prisonName: String, + @Schema(description = "ID of the prison the prisoner is assigned to", required = false, example = "MDI") + val prisonId: String?, + @Schema(description = "Name of the prison the prisoner is assigned to", required = false, example = "Moorland (HMP & YOI)") + val prisonName: String?, @Schema(description = "Cell the prisoner is assigned to", required = false, example = "B-2-007") val cellLocation: String?, ) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/offendersearch/OffenderSearchPrisoner.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/offendersearch/OffenderSearchPrisoner.kt index 9dd3fd36..92c149de 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/offendersearch/OffenderSearchPrisoner.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/dto/offendersearch/OffenderSearchPrisoner.kt @@ -5,7 +5,7 @@ data class OffenderSearchPrisoner( val firstName: String, val lastName: String, - val prisonId: String, - val prisonName: String, + val prisonId: String?, + val prisonName: String?, val cellLocation: String?, ) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResource.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResource.kt index 76b0e47d..3299d6b9 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResource.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResource.kt @@ -187,7 +187,12 @@ class NonAssociationsResource( ), ApiResponse( responseCode = "404", - description = "Any of the prisoners could not be found.", + description = "Some of the prisoners were not be found.", + content = [Content(mediaType = "application/json", schema = Schema(implementation = ErrorResponse::class))], + ), + ApiResponse( + responseCode = "409", + description = "Open non-association already exists or some prisoner’s location is null.", content = [Content(mediaType = "application/json", schema = Schema(implementation = ErrorResponse::class))], ), ], diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsService.kt index 4d9fb7c3..d5eb173d 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsService.kt @@ -14,6 +14,7 @@ import org.springframework.web.server.ResponseStatusException import uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.AuthenticationFacade import uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.FeatureFlagsConfig import uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.NonAssociationAlreadyClosedException +import uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.NullPrisonerLocationsException import uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.OpenNonAssociationAlreadyExistsException import uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.config.UserInContextMissingException import uk.gov.justice.digital.hmpps.hmppsnonassociationsapi.dto.CloseNonAssociationRequest @@ -67,7 +68,14 @@ class NonAssociationsService( throw OpenNonAssociationAlreadyExistsException(prisonersToKeepApart) } - offenderSearch.searchByPrisonerNumbers(prisonersToKeepApart) + // ensure that prisoner numbers exist and their locations are not null + val prisonersWithNullLocations = offenderSearch.searchByPrisonerNumbers(prisonersToKeepApart) + .values + .filter { it.prisonId == null } + .map { it.prisonerNumber } + if (prisonersWithNullLocations.isNotEmpty()) { + throw NullPrisonerLocationsException(prisonersWithNullLocations) + } val nonAssociationJpa = createNonAssociationRequest.toNewEntity( updatedBy = authenticationFacade.currentUsername diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResourceTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResourceTest.kt index 4b43edaf..59feb2ec 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResourceTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/resource/NonAssociationsResourceTest.kt @@ -323,6 +323,41 @@ class NonAssociationsResourceTest : SqsIntegrationTestBase() { .expectStatus().isEqualTo(409) } + @Test + fun `cannot create NA if some prisoner has a null location`() { + val firstPrisoner = offenderSearchPrisoners["A1234BC"]!! + val secondPrisoner = offenderSearchPrisoners["D1234DD"]!! + + offenderSearchMockServer.stubSearchByPrisonerNumbers( + listOf("A1234BC", "D1234DD"), + listOf(firstPrisoner, secondPrisoner), + ) + + val request = createNonAssociationRequest( + firstPrisonerNumber = firstPrisoner.prisonerNumber, + firstPrisonerRole = Role.VICTIM, + secondPrisonerNumber = secondPrisoner.prisonerNumber, + secondPrisonerRole = Role.PERPETRATOR, + reason = Reason.VIOLENCE, + restrictionType = RestrictionType.CELL, + comment = "They keep fighting", + ) + + webTestClient.post() + .uri(url) + .headers( + setAuthorisation( + user = expectedUsername, + roles = listOf("ROLE_WRITE_NON_ASSOCIATIONS"), + scopes = listOf("write", "read"), + ), + ) + .header("Content-Type", "application/json") + .bodyValue(jsonString(request)) + .exchange() + .expectStatus().isEqualTo(409) + } + @Test fun `can create NA for already closed NA between same prisoners`() { val firstPrisoner = offenderSearchPrisoners["A1234BC"]!! @@ -2512,15 +2547,15 @@ class NonAssociationsResourceTest : SqsIntegrationTestBase() { @Test fun `when non-associations are requested between more than 2 prisoners`() { - // language=mermaid - """ + /* + %% language=mermaid classDiagram A0000AA -- A1111AA A0000AA -- A2222AA A2222AA -- A3333AA A1111AA -- A4444AA A4444AA -- A2222AA : closed - """ + */ createNonAssociation("A0000AA", "A1111AA") // never returned createNonAssociation("A0000AA", "A2222AA") // returned createNonAssociation("A2222AA", "A3333AA") // never returned @@ -2865,15 +2900,15 @@ class NonAssociationsResourceTest : SqsIntegrationTestBase() { @Test fun `when non-associations are requested involving more than 2 prisoners`() { - // language=mermaid - """ + /* + %% language=mermaid classDiagram A0000AA -- A1111AA A0000AA -- A2222AA A2222AA -- A3333AA A1111AA -- A4444AA A4444AA -- A2222AA : closed - """ + */ createNonAssociation("A0000AA", "A1111AA") // never returned createNonAssociation("A0000AA", "A2222AA") // returned createNonAssociation("A2222AA", "A3333AA") // returned diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsServiceTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsServiceTest.kt index e3b55f1c..22b598ed 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsServiceTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/service/NonAssociationsServiceTest.kt @@ -167,9 +167,9 @@ class NonAssociationsServiceTest { NonAssociationsSort.LAST_NAME -> { n -> n.otherPrisonerDetails.lastName } NonAssociationsSort.FIRST_NAME -> { n -> n.otherPrisonerDetails.firstName } NonAssociationsSort.PRISONER_NUMBER -> { n -> n.otherPrisonerDetails.prisonerNumber } - NonAssociationsSort.PRISON_ID -> { n -> n.otherPrisonerDetails.prisonId } - NonAssociationsSort.PRISON_NAME -> { n -> n.otherPrisonerDetails.prisonName } - NonAssociationsSort.CELL_LOCATION -> { n -> n.otherPrisonerDetails.cellLocation!! } + NonAssociationsSort.PRISON_ID -> { n -> n.otherPrisonerDetails.prisonId ?: "" } + NonAssociationsSort.PRISON_NAME -> { n -> n.otherPrisonerDetails.prisonName ?: "" } + NonAssociationsSort.CELL_LOCATION -> { n -> n.otherPrisonerDetails.cellLocation ?: "" } } Sort.Direction.entries.forEach { sortDirection -> val listOptions = NonAssociationListOptions(sortBy = sortBy, sortDirection = sortDirection) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/util/TestData.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/util/TestData.kt index 3e8f51e8..45c0c8ec 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/util/TestData.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsnonassociationsapi/util/TestData.kt @@ -104,4 +104,31 @@ val offenderSearchPrisoners = mapOf( prisonName = "Forest Bank", cellLocation = "FBI-C-2", ), + // In transfer + "C1234CC" to OffenderSearchPrisoner( + prisonerNumber = "C1234CC", + firstName = "MAX", + lastName = "CLARKE", + prisonId = "TRN", + prisonName = "Transfer", + cellLocation = null, + ), + // Outside any establishment + "B1234BB" to OffenderSearchPrisoner( + prisonerNumber = "B1234BB", + firstName = "JOE", + lastName = "PETERS", + prisonId = "OUT", + prisonName = "Outside - released from Moorland (HMP)", + cellLocation = null, + ), + // Null location, allegedly indicates no booking + "D1234DD" to OffenderSearchPrisoner( + prisonerNumber = "D1234DD", + firstName = "NATHAN", + lastName = "LOST", + prisonId = null, + prisonName = null, + cellLocation = null, + ), )