Merge pull request #2711 from ministryofjustice/feature/aps-1675-add-…

…recorded-by-to-arrival-domain-events-2

APS-1675 - Capture recordedBy on CAS1 arrival domain events

doc/how-to/modifying_domain_event_schemas.md

@@ -19,13 +19,12 @@ Although if will differ on a case-by-case basis, it's only possible to make a ch
 
 ## Making a schema change that is backwards-compatible
 
-Unit and Integration tests will automatically pick up a new schema version defined in DomainEventEntity.kt and test serialization and de-serialization. To enable this when adding a new schema version:
-
 1. Add a new schema version entry to the corresponding DomainEventType (in DomainEventEntity.kt)
-2. Update Cas1DomainEventFactory.kt to ensure that JSON provided for prior schema versions reflect the legacy JSON (see createCas1DomainEventEnvelopeForSchemaVersion for an example of this)
+2. When creating the domain event, be sure to set the schema version to the latest value
 
 ## Managing a change that is not backwards-compatible
 
-In addition to the changes mentioned in the prior section:
-
-1. Update the CAS-specific DomainEventService to adapt domain event json created against the older schema version to the new schema. For an example of this, see Cas1DomainEventMigrationService
+1. Add a new schema version entry to the corresponding DomainEventType (in DomainEventEntity.kt)
+2. When creating the domain event, be sure to set the schema version to the latest value
+3. Update the CAS-specific DomainEventService to adapt domain event json created against the older schema via the Cas1DomainEventMigrationService
+4. Add a test for the older version in the Domain Event Integration Tests (DomainEventTest)

src/main/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/jpa/entity/DomainEventEntity.kt

@@ -213,6 +213,10 @@ enum class DomainEventType(
     Cas1EventType.personArrived.value,
     "Someone has arrived at an Approved Premises for their Booking",
     TimelineEventType.approvedPremisesPersonArrived,
+    schemaVersions = listOf(
+      DEFAULT_DOMAIN_EVENT_SCHEMA_VERSION,
+      DomainEventSchemaVersion(2, "Added recordedBy field"),
+    ),
   ),
   APPROVED_PREMISES_PERSON_NOT_ARRIVED(
     DomainEventCas.CAS1,

src/main/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/service/DomainEventService.kt

@@ -88,10 +88,11 @@ class DomainEventService(
     val dataJson = when {
       type == BookingCancelledEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_BOOKING_CANCELLED ->
         cas1DomainEventMigrationService.bookingCancelledJson(entity)
+      type == PersonArrivedEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_PERSON_ARRIVED ->
+        cas1DomainEventMigrationService.personArrivedJson(entity)
       (type == ApplicationSubmittedEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_APPLICATION_SUBMITTED) ||
         (type == ApplicationAssessedEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_APPLICATION_ASSESSED) ||
         (type == BookingMadeEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_BOOKING_MADE) ||
-        (type == PersonArrivedEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_PERSON_ARRIVED) ||
         (type == PersonNotArrivedEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_PERSON_NOT_ARRIVED) ||
         (type == PersonDepartedEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_PERSON_DEPARTED) ||
         (type == BookingNotMadeEnvelope::class && entity.type == DomainEventType.APPROVED_PREMISES_BOOKING_NOT_MADE) ||

src/main/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/service/cas1/Cas1DomainEventMigrationService.kt

@@ -5,32 +5,78 @@ import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.databind.node.ArrayNode
 import com.fasterxml.jackson.databind.node.ObjectNode
 import com.fasterxml.jackson.databind.node.TextNode
+import com.fasterxml.jackson.module.kotlin.convertValue
 import org.springframework.stereotype.Service
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.api.events.cas1.model.StaffMember
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.jpa.entity.DomainEventEntity
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.UserService
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.util.toLocalDate
 
 /**
 This is tested by the [DomainEventTest] integration test
  */
 @Service
-class Cas1DomainEventMigrationService(val objectMapper: ObjectMapper) {
+class Cas1DomainEventMigrationService(
+  val objectMapper: ObjectMapper,
+  val userService: UserService,
+) {
+
   fun bookingCancelledJson(entity: DomainEventEntity) =
     when (entity.schemaVersion) {
       2 -> entity.data
       else -> bookingCancelledV1JsonToV2Json(entity)
     }
 
-  fun bookingCancelledV1JsonToV2Json(domainEventEntity: DomainEventEntity): String {
-    val dataModel: JsonNode = objectMapper.readTree(domainEventEntity.data)
-    val eventDetails = dataModel["eventDetails"] as ObjectNode
+  fun personArrivedJson(entity: DomainEventEntity) =
+    when (entity.schemaVersion) {
+      2 -> entity.data
+      else -> personArrivedV1JsonToV2Json(entity)
+    }
+
+  private fun bookingCancelledV1JsonToV2Json(domainEventEntity: DomainEventEntity): String {
+    return modifyEventDetails(domainEventEntity) { eventDetailsNode ->
+      val cancellationRecordedAt = objectMapper.convertValue(domainEventEntity.occurredAt, TextNode::class.java)
+      eventDetailsNode.set<TextNode>("cancellationRecordedAt", cancellationRecordedAt)
 
-    val cancellationRecordedAt = objectMapper.convertValue(domainEventEntity.occurredAt, TextNode::class.java)
-    eventDetails.set<TextNode>("cancellationRecordedAt", cancellationRecordedAt)
+      val cancelledAt =
+        objectMapper.convertValue(eventDetailsNode["cancelledAt"], java.time.Instant::class.java)
+      val cancelledAtDate = objectMapper.convertValue(cancelledAt.toLocalDate(), TextNode::class.java)
+      eventDetailsNode.set<ArrayNode>("cancelledAtDate", cancelledAtDate)
+    }
+  }
 
-    val cancelledAt = objectMapper.convertValue(dataModel["eventDetails"]["cancelledAt"], java.time.Instant::class.java)
-    val cancelledAtDate = objectMapper.convertValue(cancelledAt.toLocalDate(), TextNode::class.java)
-    eventDetails.set<ArrayNode>("cancelledAtDate", cancelledAtDate)
+  private fun personArrivedV1JsonToV2Json(domainEventEntity: DomainEventEntity): String {
+    return modifyEventDetails(domainEventEntity) { eventDetailsNode ->
+      val triggeredByUser = domainEventEntity.triggeredByUserId?.let {
+        userService.findByIdOrNull(it)
+      }
 
+      /*
+      The primary purpose of this migration is to make v1 domain events schema valid.
+      `recordedBy` is not used to render the timeline, and these old domain events
+      will not be consumed externally, so the imperfect nature of the back fill is acceptable
+       */
+      eventDetailsNode.set<ObjectNode>(
+        "recordedBy",
+        objectMapper.convertValue(
+          StaffMember(
+            staffCode = triggeredByUser?.deliusStaffCode ?: "unknown",
+            forenames = triggeredByUser?.name ?: "unknown",
+            surname = "unknown",
+            username = triggeredByUser?.deliusUsername ?: "unknown",
+          ),
+        ),
+      )
+    }
+  }
+
+  private fun modifyEventDetails(
+    domainEventEntity: DomainEventEntity,
+    mutator: (eventDetailsNode: ObjectNode) -> Unit,
+  ): String {
+    val dataModel: JsonNode = objectMapper.readTree(domainEventEntity.data)
+    val eventDetails = dataModel["eventDetails"] as ObjectNode
+    mutator(eventDetails)
     return objectMapper.writeValueAsString(dataModel)
   }
 }

src/main/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/service/cas1/Cas1SpaceBookingManagementDomainEventService.kt

@@ -57,6 +57,7 @@ class Cas1SpaceBookingManagementDomainEventService(
     val updatedCas1SpaceBooking: Cas1SpaceBookingEntity,
     val actualArrivalDate: LocalDate,
     val actualArrivalTime: LocalTime,
+    val recordedBy: UserEntity,
   )
 
   fun arrivalRecorded(arrivalInfo: ArrivalInfo) {
@@ -66,7 +67,8 @@ class Cas1SpaceBookingManagementDomainEventService(
 
     val premises = mapApprovedPremisesEntityToPremises(updatedCas1SpaceBooking.premises)
     val offenderDetails = getOffenderForCrn(updatedCas1SpaceBooking.crn)
-    val keyWorker = getStaffMemberDetails(updatedCas1SpaceBooking.keyWorkerStaffCode)
+    val recordedByStaffDetails = getStaffDetailsByUsername(arrivalInfo.recordedBy.deliusUsername)
+    val keyWorker = getStaffDetailsByStaffCode(updatedCas1SpaceBooking.keyWorkerStaffCode)
     val eventNumber = updatedCas1SpaceBooking.deliusEventNumber!!
     val applicationId = updatedCas1SpaceBooking.applicationFacade.id
     val applicationSubmittedAt = updatedCas1SpaceBooking.applicationFacade.submittedAt
@@ -82,6 +84,7 @@ class Cas1SpaceBookingManagementDomainEventService(
         occurredAt = OffsetDateTime.now().toInstant(),
         cas1SpaceBookingId = updatedCas1SpaceBooking.id,
         bookingId = null,
+        schemaVersion = 2,
         data = PersonArrivedEnvelope(
           id = domainEventId,
           timestamp = OffsetDateTime.now().toInstant(),
@@ -101,6 +104,7 @@ class Cas1SpaceBookingManagementDomainEventService(
             arrivedAt = actualArrivalDateTime,
             expectedDepartureOn = updatedCas1SpaceBooking.expectedDepartureDate,
             notes = null,
+            recordedBy = recordedByStaffDetails.toStaffMember(),
           ),
         ),
       ),
@@ -119,7 +123,7 @@ class Cas1SpaceBookingManagementDomainEventService(
     val applicationId = updatedCas1SpaceBooking.applicationFacade.id
     val premises = mapApprovedPremisesEntityToPremises(updatedCas1SpaceBooking.premises)
     val offenderDetails = getOffenderForCrn(updatedCas1SpaceBooking.crn)
-    val staffUser = getStaffMemberDetails(user.deliusStaffCode)
+    val staffUser = getStaffDetailsByStaffCode(user.deliusStaffCode)
     val eventNumber = updatedCas1SpaceBooking.deliusEventNumber!!
 
     domainEventService.savePersonNotArrivedEvent(
@@ -175,7 +179,7 @@ class Cas1SpaceBookingManagementDomainEventService(
     val applicationId = departedCas1SpaceBooking.applicationFacade.id
     val premises = mapApprovedPremisesEntityToPremises(departedCas1SpaceBooking.premises)
     val offenderDetails = getOffenderForCrn(departedCas1SpaceBooking.crn)
-    val keyWorker = getStaffMemberDetails(departedCas1SpaceBooking.keyWorkerStaffCode)
+    val keyWorker = getStaffDetailsByStaffCode(departedCas1SpaceBooking.keyWorkerStaffCode)
     val eventNumber = departedCas1SpaceBooking.deliusEventNumber!!
 
     val actualDepartureDateTime = departureInfo.actualDepartureDate.atTime(departureInfo.actualDepartureTime).toInstant()
@@ -276,7 +280,13 @@ class Cas1SpaceBookingManagementDomainEventService(
     )
   }
 
-  private fun getStaffMemberDetails(staffCode: String?): StaffMember? {
+  private fun getStaffDetailsByUsername(deliusUsername: String) =
+    when (val staffDetailsResult = apDeliusContextApiClient.getStaffDetail(deliusUsername)) {
+      is ClientResult.Success -> staffDetailsResult.body
+      is ClientResult.Failure -> staffDetailsResult.throwException()
+    }
+
+  private fun getStaffDetailsByStaffCode(staffCode: String?): StaffMember? {
     return staffCode?.let {
       when (val staffDetailResponse = apDeliusContextApiClient.getStaffDetailByStaffCode(staffCode)) {
         is ClientResult.Success -> staffDetailResponse.body.toStaffMember()

src/main/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/service/cas1/Cas1SpaceBookingService.kt

@@ -33,6 +33,7 @@ import uk.gov.justice.digital.hmpps.approvedpremisesapi.problem.InternalServerEr
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.results.CasResult
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.PlacementRequestService
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.StaffMemberService
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.UserService
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.util.PageCriteria
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.util.extractEntityFromCasResult
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.util.getMetadata
@@ -58,6 +59,7 @@ class Cas1SpaceBookingService(
   private val cancellationReasonRepository: CancellationReasonRepository,
   private val nonArrivalReasonRepository: NonArrivalReasonRepository,
   private val lockablePlacementRequestRepository: LockablePlacementRequestRepository,
+  private val userService: UserService,
 ) {
   @Transactional
   fun createNewBooking(
@@ -199,6 +201,7 @@ class Cas1SpaceBookingService(
         existingCas1SpaceBooking,
         actualArrivalDate = arrivalDate,
         actualArrivalTime = arrivalTime,
+        recordedBy = userService.getUserForRequest(),
       ),
     )
 

src/main/resources/static/domain-events-api.yml

@@ -1523,6 +1523,8 @@ components:
           $ref: '#/components/schemas/Premises'
         keyWorker:
           $ref: '#/components/schemas/StaffMember'
+        recordedBy:
+          $ref: '#/components/schemas/StaffMember'
         arrivedAt:
           type: string
           example: '2022-11-30T14:51:30'
@@ -1548,6 +1550,7 @@ components:
         - premises
         - arrivedAt
         - expectedDepartureOn
+        - recordedBy
     PersonNotArrivedEnvelope:
       type: object
       properties:

src/test/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/factory/events/PersonArrivedFactory.kt

@@ -24,6 +24,7 @@ class PersonArrivedFactory : Factory<PersonArrived> {
   private var arrivedAt: Yielded<Instant> = { Instant.now().randomDateTimeBefore(5) }
   private var expectedDepartureOn: Yielded<LocalDate> = { LocalDate.now().minusDays(5) }
   private var notes: Yielded<String?> = { null }
+  private var recordedBy: Yielded<StaffMember> = { StaffMemberFactory().produce() }
 
   fun withApplicationId(applicationId: UUID) = apply {
     this.applicationId = { applicationId }
@@ -81,5 +82,6 @@ class PersonArrivedFactory : Factory<PersonArrived> {
     arrivedAt = this.arrivedAt(),
     expectedDepartureOn = this.expectedDepartureOn(),
     notes = this.notes(),
+    recordedBy = this.recordedBy(),
   )
 }

src/test/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/integration/DomainEventTest.kt

@@ -7,11 +7,15 @@ import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.EnumSource
 import org.junit.jupiter.params.provider.MethodSource
 import org.springframework.beans.factory.annotation.Autowired
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.api.events.cas1.model.BookingCancelledEnvelope
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.api.events.cas1.model.PersonArrivedEnvelope
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.config.DomainEventUrlConfig
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.integration.givens.givenAUser
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.jpa.entity.DomainEventCas
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.jpa.entity.DomainEventSchemaVersion
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.jpa.entity.DomainEventType
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.util.Cas1DomainEventsFactory
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.util.bodyAsObject
 import java.time.ZoneOffset
 import java.util.UUID
 
@@ -132,9 +136,54 @@ class DomainEventTest : InitialiseDatabasePerClassTestBase() {
       .exchange()
       .expectStatus()
       .isOk
-      .expectBody(domainEventAndJson.envelope::class.java)
-      .returnResult()
+      .bodyAsObject<BookingCancelledEnvelope>()
 
-    assertThat(response.responseBody).isEqualTo(domainEventAndJson.envelope)
+    val expectedEnvelope = (domainEventAndJson.envelope as BookingCancelledEnvelope)
+    assertThat(response.eventDetails.cancelledAtDate).isEqualTo(expectedEnvelope.eventDetails.cancelledAtDate)
+    assertThat(response.eventDetails.cancellationRecordedAt).isEqualTo(expectedEnvelope.eventDetails.cancellationRecordedAt)
+  }
+
+  @Test
+  fun `Get APPROVED_PREMISES_PERSON_ARRIVED v1 is correctly migrated to v2 by populating recordedBy from triggered by user`() {
+    val (user, _) = givenAUser()
+
+    val jwt = jwtAuthHelper.createClientCredentialsJwt(
+      username = "username",
+      roles = listOf("ROLE_APPROVED_PREMISES_EVENTS"),
+    )
+
+    val domainEventAndJson = domainEventsFactory.createEnvelopeLatestVersion(
+      type = DomainEventType.APPROVED_PREMISES_PERSON_ARRIVED,
+      occurredAt = clock.instant(),
+    )
+
+    val persistedJson = domainEventsFactory.removeEventDetails(
+      objectMapper.writeValueAsString(domainEventAndJson.envelope),
+      listOf("recordedBy"),
+    )
+
+    val event = domainEventFactory.produceAndPersist {
+      withType(DomainEventType.APPROVED_PREMISES_PERSON_ARRIVED)
+      withData(persistedJson)
+      withOccurredAt(clock.instant().atOffset(ZoneOffset.UTC))
+      withSchemaVersion(1)
+      withTriggeredByUserId(user.id)
+    }
+
+    val url = generateUrlForDomainEventType(DomainEventType.APPROVED_PREMISES_PERSON_ARRIVED, event.id)
+
+    val response = webTestClient.get()
+      .uri(url)
+      .header("Authorization", "Bearer $jwt")
+      .exchange()
+      .expectStatus()
+      .isOk
+      .bodyAsObject<PersonArrivedEnvelope>()
+
+    val recordedBy = response.eventDetails.recordedBy
+    assertThat(recordedBy.username).isEqualTo(user.deliusUsername)
+    assertThat(recordedBy.staffCode).isEqualTo(user.deliusStaffCode)
+    assertThat(recordedBy.forenames).isEqualTo(user.name)
+    assertThat(recordedBy.surname).isEqualTo("unknown")
   }
 }

src/test/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/unit/service/Cas1SpaceBookingServiceTest.kt

@@ -59,6 +59,7 @@ import uk.gov.justice.digital.hmpps.approvedpremisesapi.jpa.entity.cas1.SpaceAva
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.results.CasResult
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.PlacementRequestService
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.StaffMemberService
+import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.UserService
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.cas1.BlockingReason
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.cas1.Cas1ApplicationStatusService
 import uk.gov.justice.digital.hmpps.approvedpremisesapi.service.cas1.Cas1BookingDomainEventService
@@ -121,7 +122,10 @@ class Cas1SpaceBookingServiceTest {
   private lateinit var nonArrivalReasonRepository: NonArrivalReasonRepository
 
   @MockK
-  private val lockablePlacementRequestRepository = mockk<LockablePlacementRequestRepository>()
+  private lateinit var lockablePlacementRequestRepository: LockablePlacementRequestRepository
+
+  @MockK
+  private lateinit var userService: UserService
 
   @InjectMockKs
   private lateinit var service: Cas1SpaceBookingService
@@ -738,12 +742,17 @@ class Cas1SpaceBookingServiceTest {
 
     @Test
     fun `Updates space booking with arrival information and raises domain event`() {
-      val updatedSpaceBookingCaptor = slot<Cas1SpaceBookingEntity>()
+      val user = UserEntityFactory().withDefaults().produce()
 
       every { cas1PremisesService.findPremiseById(any()) } returns premises
       every { spaceBookingRepository.findByIdOrNull(any()) } returns existingSpaceBooking
+
+      val updatedSpaceBookingCaptor = slot<Cas1SpaceBookingEntity>()
       every { spaceBookingRepository.save(capture(updatedSpaceBookingCaptor)) } returnsArgument 0
-      every { cas1SpaceBookingManagementDomainEventService.arrivalRecorded(any()) } returns Unit
+
+      val arrivalInfoCaptor = slot<Cas1SpaceBookingManagementDomainEventService.ArrivalInfo>()
+      every { cas1SpaceBookingManagementDomainEventService.arrivalRecorded(capture(arrivalInfoCaptor)) } returns Unit
+      every { userService.getUserForRequest() } returns user
 
       val result = service.recordArrivalForBooking(
         premisesId = UUID.randomUUID(),
@@ -760,6 +769,12 @@ class Cas1SpaceBookingServiceTest {
       assertThat(updatedSpaceBooking.actualArrivalDate).isEqualTo(LocalDate.of(2024, 1, 2))
       assertThat(updatedSpaceBooking.actualArrivalTime).isEqualTo(LocalTime.of(3, 4, 5))
       assertThat(updatedSpaceBooking.canonicalArrivalDate).isEqualTo(LocalDate.of(2024, 1, 2))
+
+      val arrivalInfo = arrivalInfoCaptor.captured
+      assertThat(arrivalInfo.updatedCas1SpaceBooking).isEqualTo(updatedSpaceBooking)
+      assertThat(arrivalInfo.actualArrivalDate).isEqualTo(LocalDate.of(2024, 1, 2))
+      assertThat(arrivalInfo.actualArrivalTime).isEqualTo(LocalTime.of(3, 4, 5))
+      assertThat(arrivalInfo.recordedBy).isEqualTo(user)
     }
   }
 

src/test/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/unit/service/DomainEventServiceTest.kt

@@ -94,7 +94,7 @@ class DomainEventServiceTest {
     userService = userService,
     emitDomainEventsEnabled = emitDomainEventsEnabled,
     mockDomainEventUrlConfig,
-    Cas1DomainEventMigrationService(objectMapper),
+    Cas1DomainEventMigrationService(objectMapper, userService),
   )
 
   private val detailUrl = "http://example.com/1234"