Revised prison filter logic and rectified tests

Author: wcdkj

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/gateways/PrisonerOffenderSearchGateway.kt

@@ -60,7 +60,7 @@ class PrisonerOffenderSearchGateway(
     }
   }
 
-  fun getPrisonerByCriteria(
+  fun getPrisonerDetails(
     firstName: String?,
     lastName: String?,
     dateOfBirth: String?,

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/models/hmpps/Person.kt

@@ -27,7 +27,6 @@ open class Person(
   @Schema(description = "HMPPS identifier", example = "2008/0545166T")
   val hmppsId: String? = null,
   val contactDetails: ContactDetailsWithEmailAndPhone? = null,
-  val prisonId: String? = null,
   val currentRestriction: Boolean? = null,
   val restrictionMessage: String? = null,
   val currentExclusion: Boolean? = null,

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPrisonersService.kt

@@ -22,7 +22,7 @@ class GetPrisonersService(
   ): Response<List<Person>> {
     val prisonIds = filters?.prisons
     val responseFromPrisonerOffenderSearch =
-      if (prisonIds != null && prisonIds.isEmpty()) {
+      if (prisonIds == null) {
         // Hit global-search endpoint
         prisonerOffenderSearchGateway.getPersons(
           firstName,
@@ -32,13 +32,9 @@ class GetPrisonersService(
         )
       } else {
         // Hit prisoner-details endpoint
-        prisonerOffenderSearchGateway.getPrisonerByCriteria(firstName, lastName, dateOfBirth, searchWithinAliases, prisonIds)
+        prisonerOffenderSearchGateway.getPrisonerDetails(firstName, lastName, dateOfBirth, searchWithinAliases, prisonIds)
       }
 
-    if (responseFromPrisonerOffenderSearch == null) {
-      return Response(emptyList(), listOf(UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "Not found")))
-    }
-
     if (responseFromPrisonerOffenderSearch.errors.isNotEmpty()) {
       return Response(emptyList(), responseFromPrisonerOffenderSearch.errors)
     }
@@ -47,20 +43,6 @@ class GetPrisonersService(
       return Response(emptyList(), listOf(UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "Not found")))
     }
 
-    if (!prisonIds.isNullOrEmpty()) {
-      // Check each prisoner has a valid prisonId that matches the consumers filter config, if prison filter exists against the consumer
-      responseFromPrisonerOffenderSearch.data.forEach { prisoner ->
-        if (
-          !filters.matchesPrison(prisoner.prisonId)
-        ) {
-          return Response(
-            data = emptyList(),
-            errors = listOf(UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "Not found")),
-          )
-        }
-      }
-    }
-
     return Response(data = responseFromPrisonerOffenderSearch.data)
   }
 }

src/main/resources/application-test.yml

@@ -87,18 +87,12 @@ authorisation:
         - "/v1/prison/prisoners/[^/]*$"
         - "/v1/prison/prisoners"
         - "/v1/prison/.*/prisoners/.*/balances$"
-      filters:
-        prisons:
     config-test:
       include:
         - "/v1/config/authorisation"
     no-include:
     empty-include:
       include:
-    populated-prison-filters:
-      filters:
-        prisons:
-          - "NOMATCH_PRISON"
     limited-prisons:
       include:
         - "/v1/prison/.*/prisoners/.*/balances$"

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/BalancesControllerTest.kt

@@ -53,11 +53,11 @@ class BalancesControllerTest(
     it("gets the balances for a person with the matching ID") {
       mockMvc.performAuthorised(basePath)
 
-      verify(getBalancesForPersonService, VerificationModeFactory.times(1)).execute(prisonId, hmppsId, ConsumerFilters(emptyList()))
+      verify(getBalancesForPersonService, VerificationModeFactory.times(1)).execute(prisonId, hmppsId, null)
     }
 
     it("returns the correct balances data") {
-      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, ConsumerFilters(emptyList()))).thenReturn(
+      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, null)).thenReturn(
         Response(
           data = balance,
         ),
@@ -91,7 +91,7 @@ class BalancesControllerTest(
     }
 
     it("returns a 404 NOT FOUND status code when person isn't found in probation offender search") {
-      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, ConsumerFilters(emptyList()))).thenReturn(
+      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, null)).thenReturn(
         Response(
           data = null,
           errors =
@@ -110,7 +110,7 @@ class BalancesControllerTest(
     }
 
     it("returns a 404 NOT FOUND status code when person isn't found in Nomis") {
-      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, ConsumerFilters(emptyList()))).thenReturn(
+      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, null)).thenReturn(
         Response(
           data = null,
           errors =
@@ -129,7 +129,7 @@ class BalancesControllerTest(
     }
 
     it("returns a 400 BAD REQUEST status code when account isn't found in the upstream API") {
-      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, ConsumerFilters(emptyList()))).thenReturn(
+      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, null)).thenReturn(
         Response(
           data = null,
           errors =
@@ -148,7 +148,7 @@ class BalancesControllerTest(
     }
 
     it("returns a 500 INTERNAL SERVER ERROR status code when balance isn't found in the upstream API") {
-      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, ConsumerFilters(emptyList()))).thenThrow(
+      whenever(getBalancesForPersonService.execute(prisonId, hmppsId, null)).thenThrow(
         IllegalStateException("Error occurred while trying to get accounts for person with id: $hmppsId"),
       )
 

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/person/PersonControllerTest.kt

@@ -334,7 +334,6 @@ internal class PersonControllerTest(
                    "pncId":null,
                    "hmppsId":null,
                    "contactDetails":null,
-                   "prisonId": null,
                    "currentRestriction": null,
                    "restrictionMessage": null,
                    "currentExclusion": null,
@@ -358,7 +357,6 @@ internal class PersonControllerTest(
                    "pncId":null,
                    "hmppsId":null,
                    "contactDetails":null,
-                   "prisonId": null,
                    "currentRestriction": null,
                    "restrictionMessage": null,
                    "currentExclusion": null,

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/prison/PrisonControllerTest.kt

@@ -45,7 +45,7 @@ internal class PrisonControllerTest(
       val firstName = "Barry"
       val lastName = "Allen"
       val dateOfBirth = "2023-03-01"
-      val emptyConsumerFilter = ConsumerFilters(emptyList())
+      val emptyConsumerFilter = null
 
       describe("GET $basePath") {
       }
@@ -80,7 +80,6 @@ internal class PrisonControllerTest(
                 exclusionMessage = "An exclusion is present",
                 currentRestriction = true,
                 restrictionMessage = "A restriction is present",
-                prisonId = "VALID_PRISON",
               ),
           ),
         )
@@ -106,7 +105,6 @@ internal class PrisonControllerTest(
                    "pncId": "PNC123456",
                    "hmppsId": null,
                    "contactDetails": null,
-                   "prisonId": "VALID_PRISON",
                    "currentRestriction": true,
                    "restrictionMessage": "A restriction is present",
                    "currentExclusion": true,
@@ -211,9 +209,9 @@ internal class PrisonControllerTest(
         result.response.status.shouldBe(500)
       }
 
-      it("returns 404 when prisoner query gets no result") {
+      it("returns 500 when prisoner query gets no result") {
 
-        whenever(getPrisonersService.execute("Barry", "Allen", "2023-03-01", false, emptyConsumerFilter)).thenReturn(
+        whenever(getPrisonersService.execute("Barry", "Allen", "2023-03-01", false, ConsumerFilters(emptyList()))).thenReturn(
           Response(
             data = emptyList(),
             errors =
@@ -228,7 +226,7 @@ internal class PrisonControllerTest(
         )
 
         val result = mockMvc.performAuthorised("$basePath/prisoners?first_name=$firstName&last_name=$lastName&date_of_birth=$dateOfBirth")
-        result.response.status.shouldBe(404)
+        result.response.status.shouldBe(500)
       }
 
       it("returns a 200 OK status code") {
@@ -255,32 +253,5 @@ internal class PrisonControllerTest(
 
         result.response.status.shouldBe(HttpStatus.OK.value())
       }
-
-      it("returns a 403 when the consumer filters prisonIds does not match the prisonId") {
-        whenever(getPrisonersService.execute(firstName, lastName, dateOfBirth, false, ConsumerFilters(prisons = listOf("NOMATCH_PRISON")))).thenReturn(
-          Response(
-            data =
-              listOf(
-                Person(
-                  firstName = "Barry",
-                  lastName = "Allen",
-                  middleName = "Jonas",
-                  dateOfBirth = LocalDate.parse("2023-03-01"),
-                  prisonId = "INVALID_PRISON",
-                ),
-                Person(
-                  firstName = "Barry",
-                  lastName = "Allen",
-                  middleName = "Rock",
-                  dateOfBirth = LocalDate.parse("2022-07-22"),
-                  prisonId = "INVALID_PRISON",
-                ),
-              ),
-          ),
-        )
-        val result = mockMvc.performAuthorisedWithCN("$basePath/prisoners?first_name=$firstName&last_name=$lastName&date_of_birth=$dateOfBirth&search_within_aliases=false", "populated-prison-filters")
-
-        result.response.status.shouldBe(HttpStatus.FORBIDDEN.value())
-      }
     },
   )

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/services/GetPrisonersServiceTest.kt

@@ -46,7 +46,7 @@ internal class GetPrisonersServiceTest(
           ),
         )
 
-        val result = getPrisonersService.execute("Qui-gon", "Jin", "1966-10-25", false, ConsumerFilters(emptyList()))
+        val result = getPrisonersService.execute("Qui-gon", "Jin", "1966-10-25", false, null)
         result?.errors.shouldBe(
           listOf(
             UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "MockError"),
@@ -62,7 +62,7 @@ internal class GetPrisonersServiceTest(
           ),
         )
 
-        val result = getPrisonersService.execute("Qui-gon", "Jin", "1966-10-25", false, ConsumerFilters(emptyList()))
+        val result = getPrisonersService.execute("Qui-gon", "Jin", "1966-10-25", false, null)
         result?.data?.shouldBe(
           people,
         )
@@ -80,18 +80,18 @@ internal class GetPrisonersServiceTest(
           ),
         )
 
-        val result = getPrisonersService.execute("Qui-gon", "Jin", "1966-10-25", false, ConsumerFilters(prisons = listOf("")))
+        val result = getPrisonersService.execute("Qui-gon", "Jin", "1966-10-25", false, null)
         result?.errors.shouldBe(
           listOf(
-            UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "Not found"),
+            UpstreamApiError(UpstreamApi.PRISONER_OFFENDER_SEARCH, UpstreamApiError.Type.ENTITY_NOT_FOUND, "MockError"),
           ),
         )
       }
 
       it("returns an error when the person queried for is not in a prisonId matching their config") {
         val prisonIds = ConsumerFilters(prisons = listOf("FAKE_PRISON"))
         whenever(request.getAttribute("filters")).thenReturn(prisonIds)
-        whenever(prisonerOffenderSearchGateway.getPrisonerByCriteria("Qui-gon", "Jin", "1966-10-25", false, prisonIds.prisons)).thenReturn(
+        whenever(prisonerOffenderSearchGateway.getPrisonerDetails("Qui-gon", "Jin", "1966-10-25", false, prisonIds.prisons)).thenReturn(
           Response(
             errors =
               listOf(
@@ -110,10 +110,10 @@ internal class GetPrisonersServiceTest(
       }
 
       it("returns the persons data when the person queried for is in a prisonId matching their config") {
-        val people = listOf(Person(firstName = "Qui-gon", lastName = "Jin", prisonId = "VALID_PRISON"), Person(firstName = "John", lastName = "Jin", prisonId = "VALID_PRISON"))
+        val people = listOf(Person(firstName = "Qui-gon", lastName = "Jin"), Person(firstName = "John", lastName = "Jin"))
         val prisonIds = ConsumerFilters(prisons = listOf("VALID_PRISON"))
         whenever(request.getAttribute("filters")).thenReturn(prisonIds)
-        whenever(prisonerOffenderSearchGateway.getPrisonerByCriteria("Qui-gon", "Jin", "1966-10-25", false, prisonIds.prisons)).thenReturn(
+        whenever(prisonerOffenderSearchGateway.getPrisonerDetails("Qui-gon", "Jin", "1966-10-25", false, prisonIds.prisons)).thenReturn(
           Response(
             data = people,
           ),