HMAI-312 - Add prison filtering to case notes (#747)

* Added integration tests

* Service changes

* Controller changes

* Add test to verify filters get passed into service

* Added params to integration tests

Author: tomwatkins1994

src/main/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/controllers/v1/person/CaseNotesController.kt

@@ -6,20 +6,22 @@ import io.swagger.v3.oas.annotations.media.Content
 import io.swagger.v3.oas.annotations.media.Schema
 import io.swagger.v3.oas.annotations.responses.ApiResponse
 import io.swagger.v3.oas.annotations.tags.Tag
+import jakarta.validation.ValidationException
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.format.annotation.DateTimeFormat
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.PathVariable
+import org.springframework.web.bind.annotation.RequestAttribute
 import org.springframework.web.bind.annotation.RequestMapping
 import org.springframework.web.bind.annotation.RequestParam
 import org.springframework.web.bind.annotation.RestController
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.exception.EntityNotFoundException
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.exception.ForbiddenByUpstreamServiceException
-import uk.gov.justice.digital.hmpps.hmppsintegrationapi.extensions.decodeUrlCharacters
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.filters.CaseNoteFilter
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.CaseNote
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError
+import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.ConsumerFilters
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.GetCaseNotesForPersonService
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.internal.AuditService
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.util.PaginatedResponse
@@ -33,18 +35,20 @@ class CaseNotesController(
   @Autowired val getCaseNoteForPersonService: GetCaseNotesForPersonService,
   @Autowired val auditService: AuditService,
 ) {
-  @GetMapping("{encodedHmppsId}/case-notes")
+  @GetMapping("{hmppsId}/case-notes")
   @Operation(
     summary = "Returns case notes associated with a person.",
+    description = "<b>Applicable filters</b>: <ul><li>prisons</li></ul>",
     responses = [
       ApiResponse(responseCode = "200", useReturnTypeSchema = true, description = "Successfully found case notes for a person with the provided HMPPS ID."),
-      ApiResponse(responseCode = "404", content = [Content(schema = Schema(ref = "#/components/schemas/PersonNotFound"))]),
+      ApiResponse(responseCode = "400", content = [Content(schema = Schema(ref = "#/components/schemas/BadRequest"))]),
       ApiResponse(responseCode = "403", content = [Content(schema = Schema(ref = "#/components/schemas/ForbiddenResponse"))]),
+      ApiResponse(responseCode = "404", content = [Content(schema = Schema(ref = "#/components/schemas/PersonNotFound"))]),
       ApiResponse(responseCode = "500", content = [Content(schema = Schema(ref = "#/components/schemas/InternalServerError"))]),
     ],
   )
   fun getCaseNotesForPerson(
-    @Parameter(description = "A URL-encoded HMPPS identifier", example = "2008%2F0545166T") @PathVariable encodedHmppsId: String,
+    @Parameter(description = "The HMPPS ID of the person", example = "G2996UX") @PathVariable hmppsId: String,
     @Parameter(description = "Filter case notes from this date")
     @RequestParam(required = false, name = "startDate")
     @DateTimeFormat(iso = DateTimeFormat.ISO.DATE)
@@ -57,11 +61,15 @@ class CaseNotesController(
     @RequestParam(required = false, name = "locationId") locationId: String?,
     @RequestParam(required = false, defaultValue = "1", name = "page") page: Int,
     @RequestParam(required = false, defaultValue = "10", name = "perPage") perPage: Int,
+    @RequestAttribute filters: ConsumerFilters?,
   ): PaginatedResponse<CaseNote> {
-    val hmppsId = encodedHmppsId.decodeUrlCharacters()
-    val response = getCaseNoteForPersonService.execute(CaseNoteFilter(hmppsId, startDate, endDate, locationId))
+    val response = getCaseNoteForPersonService.execute(CaseNoteFilter(hmppsId, startDate, endDate, locationId), filters)
 
-    if (response.hasErrorCausedBy(UpstreamApiError.Type.ENTITY_NOT_FOUND, causedBy = UpstreamApi.CASE_NOTES)) {
+    if (response.hasError(UpstreamApiError.Type.BAD_REQUEST)) {
+      throw ValidationException("Invalid id: $hmppsId")
+    }
+
+    if (response.hasError(UpstreamApiError.Type.ENTITY_NOT_FOUND)) {
       throw EntityNotFoundException("Could not find person with id: $hmppsId")
     }
 
@@ -70,6 +78,7 @@ class CaseNotesController(
     }
 
     auditService.createEvent("GET_CASE_NOTES", mapOf("hmppsId" to hmppsId))
-    return response.data.paginateWith(page, perPage)
+
+    return response.data.orEmpty().paginateWith(page, perPage)
   }
 }

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

@@ -2,7 +2,7 @@ package uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.filters
 
 import java.time.LocalDateTime
 
-class CaseNoteFilter(
+data class CaseNoteFilter(
   val hmppsId: String,
   val startDate: LocalDateTime? = null,
   val endDate: LocalDateTime? = null,

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

@@ -6,25 +6,31 @@ import uk.gov.justice.digital.hmpps.hmppsintegrationapi.gateways.CaseNotesGatewa
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.filters.CaseNoteFilter
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.CaseNote
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Response
+import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi
+import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError
+import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.ConsumerFilters
 
 @Service
 class GetCaseNotesForPersonService(
   @Autowired val caseNotesGateway: CaseNotesGateway,
   @Autowired val getPersonService: GetPersonService,
 ) {
-  fun execute(filter: CaseNoteFilter): Response<List<CaseNote>> {
-    val personResponse = getPersonService.execute(hmppsId = filter.hmppsId)
-    val nomisNumber = personResponse.data?.identifiers?.nomisNumber
+  fun execute(
+    filter: CaseNoteFilter,
+    filters: ConsumerFilters?,
+  ): Response<List<CaseNote>?> {
+    val personResponse = getPersonService.getNomisNumberWithPrisonFilter(filter.hmppsId, filters)
+    if (personResponse.errors.isNotEmpty()) {
+      return Response(data = emptyList(), errors = personResponse.errors)
+    }
 
-    var caseNotes: Response<List<CaseNote>> = Response(data = emptyList())
+    val nomisNumber = personResponse.data?.nomisNumber ?: return Response(data = null, errors = listOf(UpstreamApiError(UpstreamApi.NOMIS, UpstreamApiError.Type.ENTITY_NOT_FOUND)))
 
-    if (nomisNumber != null) {
-      caseNotes = caseNotesGateway.getCaseNotesForPerson(id = nomisNumber, filter)
-    }
+    val caseNotes = caseNotesGateway.getCaseNotesForPerson(id = nomisNumber, filter)
 
     return Response(
       data = caseNotes.data,
-      errors = personResponse.errors + caseNotes.errors,
+      errors = caseNotes.errors,
     )
   }
 }

src/main/resources/globals.yml

@@ -14,6 +14,7 @@ globals:
         - "/v1/persons/.*/visit-orders"
         - "/v1/persons/.*/visit/future"
         - "/v1/persons/.*/alerts"
+        - "/v1/persons/.*/case-notes"
         - "/v1/persons/.*/name"
         - "/v1/persons/.*/cell-location"
         - "/v1/persons/.*/risks/categories"

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

@@ -4,10 +4,8 @@ import io.kotest.core.spec.style.DescribeSpec
 import io.kotest.matchers.shouldBe
 import io.kotest.matchers.string.shouldContain
 import org.mockito.Mockito
-import org.mockito.internal.verification.VerificationModeFactory
-import org.mockito.kotlin.any
-import org.mockito.kotlin.argThat
 import org.mockito.kotlin.doThrow
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import org.springframework.beans.factory.annotation.Autowired
@@ -24,10 +22,10 @@ import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.CaseNote
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.Response
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApi
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.hmpps.UpstreamApiError
+import uk.gov.justice.digital.hmpps.hmppsintegrationapi.models.roleconfig.ConsumerFilters
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.GetCaseNotesForPersonService
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.services.internal.AuditService
-import java.net.URLEncoder
-import java.nio.charset.StandardCharsets
+import java.time.LocalDateTime
 
 @WebMvcTest(controllers = [CaseNotesController::class])
 @ActiveProfiles("test")
@@ -37,43 +35,108 @@ class CaseNotesControllerTest(
   @MockitoBean val auditService: AuditService,
 ) : DescribeSpec(
     {
-      val hmppsId = "9999/11111A"
-      val encodedHmppsId = URLEncoder.encode(hmppsId, StandardCharsets.UTF_8)
-      val path = "/v1/persons/$encodedHmppsId/case-notes"
+      val hmppsId = "G2996UX"
+      val locationId = "MDI"
+      val startDate: LocalDateTime = LocalDateTime.now()
+      val endDate: LocalDateTime = LocalDateTime.now()
+      val path = "/v1/persons/$hmppsId/case-notes?startDate=$startDate&endDate=$endDate&locationId=$locationId"
+      val caseNoteFilter = CaseNoteFilter(hmppsId, startDate, endDate, locationId)
       val mockMvc = IntegrationAPIMockMvc(springMockMvc)
       val pageCaseNote =
         listOf(
           CaseNote(caseNoteId = "abcd1234"),
         )
+      val filters = null
+
       describe("GET $path") {
         beforeTest {
           Mockito.reset(getCaseNotesForPersonService)
           Mockito.reset(auditService)
-          whenever(getCaseNotesForPersonService.execute(any())).thenReturn(
+
+          whenever(getCaseNotesForPersonService.execute(caseNoteFilter, filters)).thenReturn(
             Response(
               data = pageCaseNote,
               errors = emptyList(),
             ),
           )
         }
 
-        it("returns a 200 OK status code") {
-          val result = mockMvc.performAuthorised(path)
+        it("logs audit") {
+          mockMvc.performAuthorised(path)
 
-          result.response.status.shouldBe(HttpStatus.OK.value())
+          verify(
+            auditService,
+            times(1),
+          ).createEvent("GET_CASE_NOTES", mapOf("hmppsId" to hmppsId))
         }
 
-        it("gets the case notes for a person with the matching ID") {
-          mockMvc.performAuthorised(path)
+        it("passes filters into service") {
+          mockMvc.performAuthorisedWithCN(path, "limited-prisons")
 
           verify(
             getCaseNotesForPersonService,
-            VerificationModeFactory.times(1),
-          ).execute(argThat<CaseNoteFilter> { it -> it.hmppsId == hmppsId })
+            times(1),
+          ).execute(
+            caseNoteFilter,
+            ConsumerFilters(prisons = listOf("XYZ")),
+          )
+        }
+
+        it("returns the case notes for a person with the matching ID with a 200 status code") {
+          val result = mockMvc.performAuthorised(path)
+          result.response.status.shouldBe(HttpStatus.OK.value())
+          result.response.contentAsString.shouldContain(
+            """
+            {
+              "data": [
+                {
+                  "caseNoteId": "abcd1234",
+                  "offenderIdentifier": null,
+                  "type": null,
+                  "typeDescription": null,
+                  "subType": null,
+                  "subTypeDescription": null,
+                  "creationDateTime": null,
+                  "occurrenceDateTime": null,
+                  "text": null,
+                  "locationId": null,
+                  "sensitive": false,
+                  "amendments": []
+                }
+              ],
+              "pagination": {
+                "isLastPage": true,
+                "count": 1,
+                "page": 1,
+                "perPage": 10,
+                "totalCount": 1,
+                "totalPages": 1
+              }
+            }
+            """.removeWhitespaceAndNewlines(),
+          )
+        }
+
+        it("returns a 400 when the upstream service returns bad request") {
+          whenever(getCaseNotesForPersonService.execute(caseNoteFilter, filters)).thenReturn(
+            Response(
+              data = emptyList(),
+              errors =
+                listOf(
+                  UpstreamApiError(
+                    type = UpstreamApiError.Type.BAD_REQUEST,
+                    causedBy = UpstreamApi.NOMIS,
+                  ),
+                ),
+            ),
+          )
+
+          val result = mockMvc.performAuthorised(path)
+          result.response.status.shouldBe(HttpStatus.BAD_REQUEST.value())
         }
 
         it("returns a 403 when the upstream service provides a 403") {
-          whenever(getCaseNotesForPersonService.execute(any())).thenReturn(
+          whenever(getCaseNotesForPersonService.execute(caseNoteFilter, filters)).thenReturn(
             Response(
               data = emptyList(),
               errors =
@@ -85,61 +148,35 @@ class CaseNotesControllerTest(
                 ),
             ),
           )
+
           val result = mockMvc.performAuthorised(path)
           result.response.status.shouldBe(HttpStatus.FORBIDDEN.value())
         }
 
-        it("returns the case notes for a person with the matching ID") {
-          val result = mockMvc.performAuthorised(path)
-
-          result.response.contentAsString.shouldContain(
-            """
-          {
-          "data": [
-            {
-              "caseNoteId": "abcd1234",
-              "offenderIdentifier": null,
-              "type": null,
-              "typeDescription": null,
-              "subType": null,
-              "subTypeDescription": null,
-              "creationDateTime": null,
-              "occurrenceDateTime": null,
-              "text": null,
-              "locationId": null,
-              "sensitive": false,
-              "amendments": []
-            }
-          ],
-          "pagination": {
-            "isLastPage": true,
-            "count": 1,
-            "page": 1,
-            "perPage": 10,
-            "totalCount": 1,
-            "totalPages": 1
-          }
-        }
-        """.removeWhitespaceAndNewlines(),
+        it("returns a 400 when the upstream service returns entity not found") {
+          whenever(getCaseNotesForPersonService.execute(caseNoteFilter, filters)).thenReturn(
+            Response(
+              data = emptyList(),
+              errors =
+                listOf(
+                  UpstreamApiError(
+                    type = UpstreamApiError.Type.ENTITY_NOT_FOUND,
+                    causedBy = UpstreamApi.NOMIS,
+                  ),
+                ),
+            ),
           )
-        }
-
-        it("logs audit") {
-          mockMvc.performAuthorised(path)
 
-          verify(
-            auditService,
-            VerificationModeFactory.times(1),
-          ).createEvent("GET_CASE_NOTES", mapOf("hmppsId" to hmppsId))
+          val result = mockMvc.performAuthorised(path)
+          result.response.status.shouldBe(HttpStatus.NOT_FOUND.value())
         }
 
         it("fails with the appropriate error when an upstream service is down") {
-          whenever(getCaseNotesForPersonService.execute(any())).doThrow(
+          whenever(getCaseNotesForPersonService.execute(caseNoteFilter, filters)).doThrow(
             WebClientResponseException(500, "MockError", null, null, null, null),
           )
 
           val response = mockMvc.performAuthorised(path)
-
           assert(response.response.status == 500)
           assert(
             response.response.contentAsString.equals(

src/test/kotlin/uk/gov/justice/digital/hmpps/hmppsintegrationapi/integration/person/CaseNotesIntegrationTest.kt

@@ -4,12 +4,36 @@ import org.junit.jupiter.api.Test
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
 import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
 import uk.gov.justice.digital.hmpps.hmppsintegrationapi.integration.IntegrationTestBase
+import java.time.LocalDateTime
 
 class CaseNotesIntegrationTest : IntegrationTestBase() {
+  private final val locationId = "MDI"
+  private final val startDate: LocalDateTime = LocalDateTime.now()
+  private final val endDate: LocalDateTime = LocalDateTime.now()
+  private final val path = "$basePath/$crn/case-notes?startDate=$startDate&endDate=$endDate&locationId=$locationId"
+
   @Test
   fun `returns case notes for a person`() {
-    callApi("$basePath/$crn/case-notes")
+    callApi(path)
       .andExpect(status().isOk)
       .andExpect(content().json(getExpectedResponse("person-case-notes")))
   }
+
+  @Test
+  fun `returns a 400 if the hmppsId is invalid`() {
+    callApi("$basePath/$invalidNomsId/case-notes")
+      .andExpect(status().isBadRequest)
+  }
+
+  @Test
+  fun `returns a 404 for if consumer has empty list of prisons`() {
+    callApiWithCN(path, noPrisonsCn)
+      .andExpect(status().isNotFound)
+  }
+
+  @Test
+  fun `returns a 404 for prisoner in wrong prison`() {
+    callApiWithCN(path, limitedPrisonsCn)
+      .andExpect(status().isNotFound)
+  }
 }