From 63a39964bd1af426402b6ea490702de13ebfc31a Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Fri, 7 Mar 2025 12:23:23 +0000 Subject: [PATCH 01/13] VB-5220 - Migrate a prisoner balance Initial migration to onboard a prisoner from NOMIS to dps. This PR includes a new service, handling migrating a balance of both positive and negative values. In the current system design, negative VOs are represented as their own model with unique status and types. Add integration tests to cover new migration logic. --- .../controller/NomisSyncController.kt | 8 +- .../nomis/VisitAllocationPrisonerSyncDto.kt | 4 +- .../visitallocationapi/enums/ChangeLogType.kt | 7 ++ .../enums/NegativeVisitOrderStatus.kt | 8 ++ .../enums/NegativeVisitOrderType.kt | 7 ++ .../{ChangeSource.kt => ChangeLogSource.kt} | 2 +- .../model/entity/ChangeLog.kt | 41 +++++++ .../model/entity/NegativeVisitOrder.kt | 38 +++++++ .../repository/ChangeLogRepository.kt | 8 ++ .../NegativeVisitOrderRepository.kt | 8 ++ .../service/ChangeLogService.kt | 32 ++++++ .../service/NomisSyncService.kt | 84 +++++++++++++++ ...1_5__create_negative_visit_order_table.sql | 12 +++ ...6__create_visit_order_change_log_table.sql | 10 ++ .../integration/NomisSyncControllerTest.kt | 101 +++++++++++++++++- .../integration/helper/EntityHelper.kt | 15 +++ 16 files changed, 375 insertions(+), 10 deletions(-) create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/ChangeLogType.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderType.kt rename src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/nomis/{ChangeSource.kt => ChangeLogSource.kt} (79%) create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/ChangeLog.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/ChangeLogRepository.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/NegativeVisitOrderRepository.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt create mode 100644 src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql create mode 100644 src/main/resources/db/migration/V1_6__create_visit_order_change_log_table.sql diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/controller/NomisSyncController.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/controller/NomisSyncController.kt index 732b329..f20b48b 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/controller/NomisSyncController.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/controller/NomisSyncController.kt @@ -13,6 +13,7 @@ import org.springframework.web.bind.annotation.RequestBody import org.springframework.web.bind.annotation.RestController import uk.gov.justice.digital.hmpps.visitallocationapi.dto.nomis.VisitAllocationPrisonerMigrationDto import uk.gov.justice.digital.hmpps.visitallocationapi.dto.nomis.VisitAllocationPrisonerSyncDto +import uk.gov.justice.digital.hmpps.visitallocationapi.service.NomisSyncService import uk.gov.justice.hmpps.kotlin.common.ErrorResponse const val VO_NOMIS = "/visits/allocation/prisoner" @@ -20,7 +21,7 @@ const val VO_PRISONER_MIGRATION: String = "$VO_NOMIS/migrate" const val VO_PRISONER_SYNC: String = "$VO_NOMIS/sync" @RestController -class NomisSyncController { +class NomisSyncController(val nomisSyncService: NomisSyncService) { @PreAuthorize("hasRole('ROLE_VISIT_ALLOCATION_API__NOMIS_API')") @PostMapping(VO_PRISONER_MIGRATION) @Operation( @@ -43,7 +44,10 @@ class NomisSyncController { ), ], ) - fun migratePrisonerVisitOrders(@RequestBody @Valid visitAllocationPrisonerMigrationDto: VisitAllocationPrisonerMigrationDto): ResponseEntity = ResponseEntity.status(HttpStatus.OK).build() + fun migratePrisonerVisitOrders(@RequestBody @Valid visitAllocationPrisonerMigrationDto: VisitAllocationPrisonerMigrationDto): ResponseEntity { + nomisSyncService.migratePrisoner(visitAllocationPrisonerMigrationDto) + return ResponseEntity.status(HttpStatus.OK).build() + } @PreAuthorize("hasRole('ROLE_VISIT_ALLOCATION_API__NOMIS_API')") @PostMapping(VO_PRISONER_SYNC) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/dto/nomis/VisitAllocationPrisonerSyncDto.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/dto/nomis/VisitAllocationPrisonerSyncDto.kt index 30d5d22..3ae499c 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/dto/nomis/VisitAllocationPrisonerSyncDto.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/dto/nomis/VisitAllocationPrisonerSyncDto.kt @@ -4,7 +4,7 @@ import io.swagger.v3.oas.annotations.media.Schema import jakarta.validation.constraints.NotEmpty import jakarta.validation.constraints.NotNull import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.AdjustmentReasonCode -import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.ChangeSource +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.ChangeLogSource import java.time.LocalDate data class VisitAllocationPrisonerSyncDto( @@ -39,7 +39,7 @@ data class VisitAllocationPrisonerSyncDto( @Schema(description = "The source of the change being made", example = "SYSTEM or STAFF", required = true) @field:NotNull - val changeSource: ChangeSource, + val changeLogSource: ChangeLogSource, @Schema(description = "Additional information on the sync reason", example = "Manually adjusted for phone credit", required = true) @field:NotEmpty diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/ChangeLogType.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/ChangeLogType.kt new file mode 100644 index 0000000..f446902 --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/ChangeLogType.kt @@ -0,0 +1,7 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.enums + +@Suppress("unused") +enum class ChangeLogType { + MIGRATION, + // TODO: Add other change types as they're needed. +} diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt new file mode 100644 index 0000000..ceba412 --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt @@ -0,0 +1,8 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.enums + +@Suppress("unused") +enum class NegativeVisitOrderStatus { + SCHEDULED, + USED, + REPAID, +} diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderType.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderType.kt new file mode 100644 index 0000000..af9e7ba --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderType.kt @@ -0,0 +1,7 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.enums + +@Suppress("unused") +enum class NegativeVisitOrderType { + NEGATIVE_VO, + NEGATIVE_PVO, +} diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/nomis/ChangeSource.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/nomis/ChangeLogSource.kt similarity index 79% rename from src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/nomis/ChangeSource.kt rename to src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/nomis/ChangeLogSource.kt index 19ae486..32b39ec 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/nomis/ChangeSource.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/nomis/ChangeLogSource.kt @@ -1,7 +1,7 @@ package uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis @Suppress("unused") -enum class ChangeSource { +enum class ChangeLogSource { SYSTEM, STAFF, } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/ChangeLog.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/ChangeLog.kt new file mode 100644 index 0000000..b2a6ca1 --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/ChangeLog.kt @@ -0,0 +1,41 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.model.entity + +import jakarta.persistence.Column +import jakarta.persistence.Entity +import jakarta.persistence.EnumType +import jakarta.persistence.Enumerated +import jakarta.persistence.GeneratedValue +import jakarta.persistence.GenerationType +import jakarta.persistence.Id +import jakarta.persistence.Table +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.ChangeLogType +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.ChangeLogSource +import java.time.LocalDateTime + +@Entity +@Table(name = "CHANGE_LOG") +data class ChangeLog( + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + val id: Long = 0L, + + @Column(nullable = false) + val prisonerId: String, + + @Column(nullable = false) + val changeTimestamp: LocalDateTime = LocalDateTime.now(), + + @Column(nullable = false) + @Enumerated(EnumType.STRING) + val changeType: ChangeLogType, + + @Column(nullable = false) + @Enumerated(EnumType.STRING) + val changeSource: ChangeLogSource, + + @Column(nullable = false) + val userId: String, + + @Column(nullable = false) + val comment: String? = null, +) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt new file mode 100644 index 0000000..7b086c2 --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt @@ -0,0 +1,38 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.model.entity + +import jakarta.persistence.Column +import jakarta.persistence.Entity +import jakarta.persistence.EnumType +import jakarta.persistence.Enumerated +import jakarta.persistence.GeneratedValue +import jakarta.persistence.GenerationType +import jakarta.persistence.Id +import jakarta.persistence.Table +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderStatus +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderType +import java.time.LocalDate + +@Entity +@Table(name = "NEGATIVE_VISIT_ORDER") +data class NegativeVisitOrder( + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + val id: Long = 0L, + + @Column(nullable = false) + val prisonerId: String, + + @Column(nullable = false) + @Enumerated(EnumType.STRING) + val status: NegativeVisitOrderStatus, + + @Column(nullable = false) + @Enumerated(EnumType.STRING) + val type: NegativeVisitOrderType, + + @Column(nullable = false) + val createdDate: LocalDate = LocalDate.now(), + + @Column(nullable = false) + val repaidDate: LocalDate? = null, +) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/ChangeLogRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/ChangeLogRepository.kt new file mode 100644 index 0000000..d11f8ab --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/ChangeLogRepository.kt @@ -0,0 +1,8 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.repository + +import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.stereotype.Repository +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.ChangeLog + +@Repository +interface ChangeLogRepository : JpaRepository diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/NegativeVisitOrderRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/NegativeVisitOrderRepository.kt new file mode 100644 index 0000000..3806319 --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/NegativeVisitOrderRepository.kt @@ -0,0 +1,8 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.repository + +import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.stereotype.Repository +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.NegativeVisitOrder + +@Repository +interface NegativeVisitOrderRepository : JpaRepository diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt new file mode 100644 index 0000000..15e546d --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt @@ -0,0 +1,32 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.service + +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import org.springframework.stereotype.Service +import org.springframework.transaction.annotation.Transactional +import uk.gov.justice.digital.hmpps.visitallocationapi.dto.nomis.VisitAllocationPrisonerMigrationDto +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.ChangeLogType +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.ChangeLogSource +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.ChangeLog +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.ChangeLogRepository + +@Transactional +@Service +class ChangeLogService(private val changeLogRepository: ChangeLogRepository) { + companion object { + val LOG: Logger = LoggerFactory.getLogger(this::class.java) + } + + fun logMigrationChange(migrationChangeDto: VisitAllocationPrisonerMigrationDto) { + LOG.info("Logging change to change_log table for prisoner ${migrationChangeDto.prisonerId}, change - $migrationChangeDto") + changeLogRepository.save( + ChangeLog( + prisonerId = migrationChangeDto.prisonerId, + changeType = ChangeLogType.MIGRATION, + changeSource = ChangeLogSource.SYSTEM, + userId = "SYSTEM", + comment = "migrated prisoner ${migrationChangeDto.prisonerId}, with vo balance ${migrationChangeDto.voBalance} and pvo balance ${migrationChangeDto.pvoBalance}", + ), + ) + } +} diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt new file mode 100644 index 0000000..48dabfd --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt @@ -0,0 +1,84 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.service + +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import org.springframework.stereotype.Service +import org.springframework.transaction.annotation.Transactional +import uk.gov.justice.digital.hmpps.visitallocationapi.dto.nomis.VisitAllocationPrisonerMigrationDto +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderStatus +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderType +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.NegativeVisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.NegativeVisitOrderRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository +import java.time.LocalDate +import kotlin.math.abs + +@Service +class NomisSyncService( + private val visitOrderRepository: VisitOrderRepository, + private val negativeVisitOrderRepository: NegativeVisitOrderRepository, + private val changeLogService: ChangeLogService, +) { + companion object { + val LOG: Logger = LoggerFactory.getLogger(this::class.java) + } + + @Transactional + fun migratePrisoner(migrationDto: VisitAllocationPrisonerMigrationDto) { + LOG.info("Entered NomisSyncService - migratePrisoner with migration dto {}", migrationDto) + + migrateBalance(migrationDto, VisitOrderType.VO) + migrateBalance(migrationDto, VisitOrderType.PVO) + + changeLogService.logMigrationChange(migrationDto) + + LOG.info("Finished NomisSyncService - migratePrisoner successfully") + } + + private fun migrateBalance(migrationDto: VisitAllocationPrisonerMigrationDto, type: VisitOrderType) { + val balance = if (type == VisitOrderType.VO) { + migrationDto.voBalance + } else { + migrationDto.pvoBalance + } + + when { + balance > 0 -> { + LOG.info("Migrating prisoner ${migrationDto.prisonerId} with a ${type.name} balance of $balance") + val visitOrders = List(balance) { + VisitOrder( + prisonerId = migrationDto.prisonerId, + type = type, + status = VisitOrderStatus.AVAILABLE, + // TODO: VB-5220 - Should the PVO date be the VO date - 14 days in some instances? (To help with PVO generation spread). + createdDate = migrationDto.lastVoAllocationDate, + expiryDate = null, + ) + } + visitOrderRepository.saveAll(visitOrders) + } + balance < 0 -> { + LOG.info("Migrating prisoner ${migrationDto.prisonerId} with a negative ${type.name} balance of $balance") + val negativeVisitOrders = List(abs(balance)) { + NegativeVisitOrder( + prisonerId = migrationDto.prisonerId, + status = NegativeVisitOrderStatus.USED, + type = if (type == VisitOrderType.VO) { + NegativeVisitOrderType.NEGATIVE_VO + } else { + NegativeVisitOrderType.NEGATIVE_PVO + }, + createdDate = LocalDate.now(), + ) + } + negativeVisitOrderRepository.saveAll(negativeVisitOrders) + } + else -> { + LOG.info("Not migrating ${type.name} balance for prisoner ${migrationDto.prisonerId} as it's 0") + } + } + } +} diff --git a/src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql b/src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql new file mode 100644 index 0000000..2fa3b24 --- /dev/null +++ b/src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql @@ -0,0 +1,12 @@ +CREATE TABLE negative_visit_order +( + id serial NOT NULL PRIMARY KEY, + prisoner_id VARCHAR(80) NOT NULL, + status VARCHAR(20) NOT NULL, + type VARCHAR(20) NOT NULL, + created_date DATE default current_date, + repaid_date DATE +); + +CREATE INDEX idx_negative_visit_order_composite + ON negative_visit_order (prisoner_id, status); \ No newline at end of file diff --git a/src/main/resources/db/migration/V1_6__create_visit_order_change_log_table.sql b/src/main/resources/db/migration/V1_6__create_visit_order_change_log_table.sql new file mode 100644 index 0000000..58d27ce --- /dev/null +++ b/src/main/resources/db/migration/V1_6__create_visit_order_change_log_table.sql @@ -0,0 +1,10 @@ +CREATE TABLE change_log +( + id serial NOT NULL PRIMARY KEY, + prisoner_id VARCHAR(80) NOT NULL, + change_timestamp timestamp NOT NULL, + change_type VARCHAR(30) NOT NULL, + change_source VARCHAR(30) NOT NULL, + user_id VARCHAR(150) NOT NULL, + comment TEXT +); diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt index c0fa444..cc7e39d 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt @@ -1,24 +1,47 @@ package uk.gov.justice.digital.hmpps.visitallocationapi.integration +import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.DisplayName import org.junit.jupiter.api.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.times +import org.mockito.kotlin.verify import org.springframework.http.HttpHeaders +import org.springframework.test.context.bean.override.mockito.MockitoSpyBean import org.springframework.test.web.reactive.server.WebTestClient import org.springframework.test.web.reactive.server.WebTestClient.ResponseSpec import uk.gov.justice.digital.hmpps.visitallocationapi.controller.VO_PRISONER_MIGRATION import uk.gov.justice.digital.hmpps.visitallocationapi.controller.VO_PRISONER_SYNC import uk.gov.justice.digital.hmpps.visitallocationapi.dto.nomis.VisitAllocationPrisonerMigrationDto import uk.gov.justice.digital.hmpps.visitallocationapi.dto.nomis.VisitAllocationPrisonerSyncDto +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.ChangeLogType +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderType +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.AdjustmentReasonCode -import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.ChangeSource +import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.ChangeLogSource import uk.gov.justice.digital.hmpps.visitallocationapi.integration.helper.callPost +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.ChangeLog +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.NegativeVisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.ChangeLogRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.NegativeVisitOrderRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository import java.time.LocalDate @DisplayName("NomisSyncController tests") class NomisSyncControllerTest : IntegrationTestBase() { + @MockitoSpyBean + private lateinit var visitOrderRepository: VisitOrderRepository + + @MockitoSpyBean + private lateinit var negativeVisitOrderRepository: NegativeVisitOrderRepository + + @MockitoSpyBean + private lateinit var changeLogRepository: ChangeLogRepository + @Test - fun `migrate prisoner - when visit prisoner allocation migration endpoint is called, then prisoner information is successfully migrated to DPS service`() { + fun `migrate prisoner - when visit prisoner allocation migration endpoint is called with positive balance, then prisoner information is successfully migrated to DPS service`() { // Given val prisonerMigrationDto = VisitAllocationPrisonerMigrationDto("AA123456", 5, 2, LocalDate.now().minusDays(1)) @@ -27,6 +50,74 @@ class NomisSyncControllerTest : IntegrationTestBase() { // Then responseSpec.expectStatus().isOk + + verify(visitOrderRepository, times(2)).saveAll(any()) + verify(negativeVisitOrderRepository, times(0)).saveAll(any()) + verify(changeLogRepository, times(1)).save(any()) + + val visitOrders = visitOrderRepository.findAll() + assertThat(visitOrders.size).isEqualTo(7) + assertThat(visitOrders.filter { it.type == VisitOrderType.VO }.size).isEqualTo(5) + assertThat(visitOrders.filter { it.type == VisitOrderType.PVO }.size).isEqualTo(2) + + val changeLog = changeLogRepository.findAll() + assertThat(changeLog.size).isEqualTo(1) + assertThat(changeLog.first().changeType).isEqualTo(ChangeLogType.MIGRATION) + } + + @Test + fun `migrate prisoner - when visit prisoner allocation migration endpoint is called with negative balance, then prisoner information is successfully migrated to DPS service`() { + // Given + val prisonerMigrationDto = VisitAllocationPrisonerMigrationDto("AA123456", -5, -2, LocalDate.now().minusDays(1)) + + // When + val responseSpec = callVisitAllocationMigrationEndpoint(webTestClient, prisonerMigrationDto, setAuthorisation(roles = listOf("ROLE_VISIT_ALLOCATION_API__NOMIS_API"))) + + // Then + responseSpec.expectStatus().isOk + + verify(visitOrderRepository, times(0)).saveAll(any()) + verify(negativeVisitOrderRepository, times(2)).saveAll(any()) + verify(changeLogRepository, times(1)).save(any()) + + val negativeVisitOrders = negativeVisitOrderRepository.findAll() + assertThat(negativeVisitOrders.size).isEqualTo(7) + assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_VO }.size).isEqualTo(5) + assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_PVO }.size).isEqualTo(2) + + val changeLog = changeLogRepository.findAll() + assertThat(changeLog.size).isEqualTo(1) + assertThat(changeLog.first().changeType).isEqualTo(ChangeLogType.MIGRATION) + } + + @Test + fun `migrate prisoner - when visit prisoner allocation migration endpoint is called with a mixed balance, then prisoner information is successfully migrated to DPS service`() { + // Given + val prisonerMigrationDto = VisitAllocationPrisonerMigrationDto("AA123456", 5, -2, LocalDate.now().minusDays(1)) + + // When + val responseSpec = callVisitAllocationMigrationEndpoint(webTestClient, prisonerMigrationDto, setAuthorisation(roles = listOf("ROLE_VISIT_ALLOCATION_API__NOMIS_API"))) + + // Then + responseSpec.expectStatus().isOk + + verify(visitOrderRepository, times(1)).saveAll(any()) + verify(negativeVisitOrderRepository, times(1)).saveAll(any()) + verify(changeLogRepository, times(1)).save(any()) + + val visitOrders = visitOrderRepository.findAll() + assertThat(visitOrders.size).isEqualTo(5) + assertThat(visitOrders.filter { it.type == VisitOrderType.VO }.size).isEqualTo(5) + assertThat(visitOrders.filter { it.type == VisitOrderType.PVO }.size).isEqualTo(0) + + val negativeVisitOrders = negativeVisitOrderRepository.findAll() + assertThat(negativeVisitOrders.size).isEqualTo(2) + assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_VO }.size).isEqualTo(0) + assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_PVO }.size).isEqualTo(2) + + val changeLog = changeLogRepository.findAll() + assertThat(changeLog.size).isEqualTo(1) + assertThat(changeLog.first().changeType).isEqualTo(ChangeLogType.MIGRATION) } @Test @@ -68,7 +159,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { @Test fun `sync prisoner - when visit prisoner allocation sync endpoint is called, then prisoner information is successfully synced to DPS service`() { // Given - val prisonerSyncDto = VisitAllocationPrisonerSyncDto("AA123456", 5, 1, 2, 0, LocalDate.now().minusDays(1), AdjustmentReasonCode.VO_ISSUE, ChangeSource.SYSTEM, "issued vo") + val prisonerSyncDto = VisitAllocationPrisonerSyncDto("AA123456", 5, 1, 2, 0, LocalDate.now().minusDays(1), AdjustmentReasonCode.VO_ISSUE, ChangeLogSource.SYSTEM, "issued vo") // When val responseSpec = callVisitAllocationSyncEndpoint(webTestClient, prisonerSyncDto, setAuthorisation(roles = listOf("ROLE_VISIT_ALLOCATION_API__NOMIS_API"))) @@ -80,7 +171,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { @Test fun `sync prisoner - when request body validation fails then 400 bad request is returned`() { // Given - val prisonerSyncDto = VisitAllocationPrisonerSyncDto("", 5, 1, 2, 0, LocalDate.now().minusDays(1), AdjustmentReasonCode.VO_ISSUE, ChangeSource.SYSTEM, "issued vo") + val prisonerSyncDto = VisitAllocationPrisonerSyncDto("", 5, 1, 2, 0, LocalDate.now().minusDays(1), AdjustmentReasonCode.VO_ISSUE, ChangeLogSource.SYSTEM, "issued vo") // When val responseSpec = callVisitAllocationSyncEndpoint(webTestClient, prisonerSyncDto, setAuthorisation(roles = listOf("ROLE_VISIT_ALLOCATION_API__NOMIS_API"))) @@ -93,7 +184,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { fun `sync prisoner - access forbidden when no role`() { // Given val incorrectAuthHeaders = setAuthorisation(roles = listOf()) - val prisonerSyncDto = VisitAllocationPrisonerSyncDto("AA123456", 5, 1, 2, 0, LocalDate.now().minusDays(1), AdjustmentReasonCode.VO_ISSUE, ChangeSource.SYSTEM, "issued vo") + val prisonerSyncDto = VisitAllocationPrisonerSyncDto("AA123456", 5, 1, 2, 0, LocalDate.now().minusDays(1), AdjustmentReasonCode.VO_ISSUE, ChangeLogSource.SYSTEM, "issued vo") // When val responseSpec = callVisitAllocationSyncEndpoint(webTestClient, prisonerSyncDto, incorrectAuthHeaders) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt index 8806f3f..74a3eda 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt @@ -3,12 +3,18 @@ package uk.gov.justice.digital.hmpps.visitallocationapi.integration.helper import org.springframework.stereotype.Component import org.springframework.transaction.annotation.Transactional import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrderPrison +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.ChangeLogRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.NegativeVisitOrderRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderPrisonRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository @Component @Transactional class EntityHelper( val visitOrderPrisonRepository: VisitOrderPrisonRepository, + val visitOrderRepository: VisitOrderRepository, + val negativeVisitOrderRepository: NegativeVisitOrderRepository, + val changeLogRepository: ChangeLogRepository, ) { @Transactional @@ -20,5 +26,14 @@ class EntityHelper( fun deleteAll() { visitOrderPrisonRepository.deleteAll() visitOrderPrisonRepository.flush() + + visitOrderRepository.deleteAll() + visitOrderRepository.flush() + + negativeVisitOrderRepository.deleteAll() + negativeVisitOrderRepository.flush() + + changeLogRepository.deleteAll() + changeLogRepository.flush() } } From d45c03a4d1f9a6d3852b94f496ef3fec3a390b3f Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Fri, 7 Mar 2025 12:27:49 +0000 Subject: [PATCH 02/13] Add index on prisoner_id on new change_log table --- .../V1_7__add_index_to_visit_order_change_log_table.sql | 1 + 1 file changed, 1 insertion(+) create mode 100644 src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql diff --git a/src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql b/src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql new file mode 100644 index 0000000..147f29a --- /dev/null +++ b/src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql @@ -0,0 +1 @@ +CREATE INDEX idx_visit_prisoner_id ON change_log (prisoner_id); From 68868caf904e4f2ba3c64599f544198ea1d06a47 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Fri, 7 Mar 2025 12:42:23 +0000 Subject: [PATCH 03/13] Remove SCHEDULED state After discussions, we believe it would be simpler to have a status limited to "available" or actively being used ("used"). In cases of visits being cancelled, we could move the state back to available as we would have from scheduled. This smaller enum set will help when calculating the balance in the future as well, as we don't need to account for an additional state. --- .../hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt | 1 - .../digital/hmpps/visitallocationapi/enums/VisitOrderStatus.kt | 1 - 2 files changed, 2 deletions(-) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt index ceba412..2644077 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/NegativeVisitOrderStatus.kt @@ -2,7 +2,6 @@ package uk.gov.justice.digital.hmpps.visitallocationapi.enums @Suppress("unused") enum class NegativeVisitOrderStatus { - SCHEDULED, USED, REPAID, } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/VisitOrderStatus.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/VisitOrderStatus.kt index 4e2adf9..b7e6c8a 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/VisitOrderStatus.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/enums/VisitOrderStatus.kt @@ -5,6 +5,5 @@ enum class VisitOrderStatus { AVAILABLE, EXPIRED, ACCUMULATED, - SCHEDULED, USED, } From e3506658003248a74a13c8f9624d431e84f20b83 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Tue, 11 Mar 2025 11:00:19 +0000 Subject: [PATCH 04/13] VB-5220 - Initial migration Add ability to track prisoners last allocated date regardless of their balance size. This will allow us to simplify the allocation logic, to not care about the vo / negative_vo tables and instead go directly to the prisoner_details table to get the last allocated date. --- .../model/entity/PrisonerDetails.kt | 23 +++++++++++++++++++ .../repository/PrisonerDetailsRepository.kt | 8 +++++++ .../service/NomisSyncService.kt | 10 +++++++- ..._index_to_visit_order_change_log_table.sql | 2 +- .../V1_8__create_prisoner_details_table.sql | 9 ++++++++ .../integration/NomisSyncControllerTest.kt | 23 +++++++++++++++++++ .../integration/helper/EntityHelper.kt | 5 ++++ 7 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt create mode 100644 src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt new file mode 100644 index 0000000..38f86e5 --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt @@ -0,0 +1,23 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.model.entity + +import jakarta.persistence.Column +import jakarta.persistence.Entity +import jakarta.persistence.GeneratedValue +import jakarta.persistence.GenerationType +import jakarta.persistence.Id +import jakarta.persistence.Table +import java.time.LocalDate + +@Entity +@Table(name = "PRISONER_DETAILS") +data class PrisonerDetails( + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + val id: Long = 0L, + + @Column(nullable = false) + val prisonerId: String, + + @Column(nullable = false) + val lastAllocatedDate: LocalDate, +) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt new file mode 100644 index 0000000..68b7cb1 --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt @@ -0,0 +1,8 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.repository + +import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.stereotype.Repository +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails + +@Repository +interface PrisonerDetailsRepository : JpaRepository diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt index 48dabfd..6e3fe45 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt @@ -10,8 +10,10 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderT import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.NegativeVisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder import uk.gov.justice.digital.hmpps.visitallocationapi.repository.NegativeVisitOrderRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository import java.time.LocalDate import kotlin.math.abs @@ -20,6 +22,7 @@ import kotlin.math.abs class NomisSyncService( private val visitOrderRepository: VisitOrderRepository, private val negativeVisitOrderRepository: NegativeVisitOrderRepository, + private val prisonerDetailsRepository: PrisonerDetailsRepository, private val changeLogService: ChangeLogService, ) { companion object { @@ -32,6 +35,7 @@ class NomisSyncService( migrateBalance(migrationDto, VisitOrderType.VO) migrateBalance(migrationDto, VisitOrderType.PVO) + migrateLastAllocatedDate(migrationDto) changeLogService.logMigrationChange(migrationDto) @@ -53,7 +57,6 @@ class NomisSyncService( prisonerId = migrationDto.prisonerId, type = type, status = VisitOrderStatus.AVAILABLE, - // TODO: VB-5220 - Should the PVO date be the VO date - 14 days in some instances? (To help with PVO generation spread). createdDate = migrationDto.lastVoAllocationDate, expiryDate = null, ) @@ -81,4 +84,9 @@ class NomisSyncService( } } } + + private fun migrateLastAllocatedDate(migrationDto: VisitAllocationPrisonerMigrationDto) { + LOG.info("Migrating prisoner ${migrationDto.prisonerId} details (last allocated date - ${migrationDto.lastVoAllocationDate})") + prisonerDetailsRepository.save(PrisonerDetails(prisonerId = migrationDto.prisonerId, lastAllocatedDate = migrationDto.lastVoAllocationDate)) + } } diff --git a/src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql b/src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql index 147f29a..8f6bffe 100644 --- a/src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql +++ b/src/main/resources/db/migration/V1_7__add_index_to_visit_order_change_log_table.sql @@ -1 +1 @@ -CREATE INDEX idx_visit_prisoner_id ON change_log (prisoner_id); +CREATE INDEX idx_change_log_prisoner_id ON change_log (prisoner_id); diff --git a/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql new file mode 100644 index 0000000..b064b45 --- /dev/null +++ b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql @@ -0,0 +1,9 @@ +CREATE TABLE prisoner_details +( + id serial NOT NULL PRIMARY KEY, + prisoner_id VARCHAR(80) NOT NULL UNIQUE, + last_allocated_date DATE NOT NULL +); + +CREATE INDEX idx_prisoner_details_prisoner_id ON prisoner_details (prisoner_id); + diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt index cc7e39d..e7c9d49 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt @@ -22,9 +22,11 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.enums.nomis.ChangeLogSour import uk.gov.justice.digital.hmpps.visitallocationapi.integration.helper.callPost import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.ChangeLog import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.NegativeVisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder import uk.gov.justice.digital.hmpps.visitallocationapi.repository.ChangeLogRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.NegativeVisitOrderRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository import java.time.LocalDate @@ -37,6 +39,9 @@ class NomisSyncControllerTest : IntegrationTestBase() { @MockitoSpyBean private lateinit var negativeVisitOrderRepository: NegativeVisitOrderRepository + @MockitoSpyBean + private lateinit var prisonerDetailsRepository: PrisonerDetailsRepository + @MockitoSpyBean private lateinit var changeLogRepository: ChangeLogRepository @@ -53,6 +58,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { verify(visitOrderRepository, times(2)).saveAll(any()) verify(negativeVisitOrderRepository, times(0)).saveAll(any()) + verify(prisonerDetailsRepository, times(1)).save(any()) verify(changeLogRepository, times(1)).save(any()) val visitOrders = visitOrderRepository.findAll() @@ -60,6 +66,11 @@ class NomisSyncControllerTest : IntegrationTestBase() { assertThat(visitOrders.filter { it.type == VisitOrderType.VO }.size).isEqualTo(5) assertThat(visitOrders.filter { it.type == VisitOrderType.PVO }.size).isEqualTo(2) + val prisonerDetails = prisonerDetailsRepository.findAll() + assertThat(prisonerDetails.size).isEqualTo(1) + assertThat(prisonerDetails.first().prisonerId).isEqualTo(prisonerMigrationDto.prisonerId) + assertThat(prisonerDetails.first().lastAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) + val changeLog = changeLogRepository.findAll() assertThat(changeLog.size).isEqualTo(1) assertThat(changeLog.first().changeType).isEqualTo(ChangeLogType.MIGRATION) @@ -78,6 +89,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { verify(visitOrderRepository, times(0)).saveAll(any()) verify(negativeVisitOrderRepository, times(2)).saveAll(any()) + verify(prisonerDetailsRepository, times(1)).save(any()) verify(changeLogRepository, times(1)).save(any()) val negativeVisitOrders = negativeVisitOrderRepository.findAll() @@ -85,6 +97,11 @@ class NomisSyncControllerTest : IntegrationTestBase() { assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_VO }.size).isEqualTo(5) assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_PVO }.size).isEqualTo(2) + val prisonerDetails = prisonerDetailsRepository.findAll() + assertThat(prisonerDetails.size).isEqualTo(1) + assertThat(prisonerDetails.first().prisonerId).isEqualTo(prisonerMigrationDto.prisonerId) + assertThat(prisonerDetails.first().lastAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) + val changeLog = changeLogRepository.findAll() assertThat(changeLog.size).isEqualTo(1) assertThat(changeLog.first().changeType).isEqualTo(ChangeLogType.MIGRATION) @@ -103,6 +120,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { verify(visitOrderRepository, times(1)).saveAll(any()) verify(negativeVisitOrderRepository, times(1)).saveAll(any()) + verify(prisonerDetailsRepository, times(1)).save(any()) verify(changeLogRepository, times(1)).save(any()) val visitOrders = visitOrderRepository.findAll() @@ -115,6 +133,11 @@ class NomisSyncControllerTest : IntegrationTestBase() { assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_VO }.size).isEqualTo(0) assertThat(negativeVisitOrders.filter { it.type == NegativeVisitOrderType.NEGATIVE_PVO }.size).isEqualTo(2) + val prisonerDetails = prisonerDetailsRepository.findAll() + assertThat(prisonerDetails.size).isEqualTo(1) + assertThat(prisonerDetails.first().prisonerId).isEqualTo(prisonerMigrationDto.prisonerId) + assertThat(prisonerDetails.first().lastAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) + val changeLog = changeLogRepository.findAll() assertThat(changeLog.size).isEqualTo(1) assertThat(changeLog.first().changeType).isEqualTo(ChangeLogType.MIGRATION) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt index 74a3eda..3f9a320 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/helper/EntityHelper.kt @@ -5,6 +5,7 @@ import org.springframework.transaction.annotation.Transactional import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrderPrison import uk.gov.justice.digital.hmpps.visitallocationapi.repository.ChangeLogRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.NegativeVisitOrderRepository +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderPrisonRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository @@ -15,6 +16,7 @@ class EntityHelper( val visitOrderRepository: VisitOrderRepository, val negativeVisitOrderRepository: NegativeVisitOrderRepository, val changeLogRepository: ChangeLogRepository, + private val prisonerDetailsRepository: PrisonerDetailsRepository, ) { @Transactional @@ -35,5 +37,8 @@ class EntityHelper( changeLogRepository.deleteAll() changeLogRepository.flush() + + prisonerDetailsRepository.deleteAll() + prisonerDetailsRepository.flush() } } From 03a274c8af6d071f6124b9db279855df201e8020 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Tue, 11 Mar 2025 11:08:47 +0000 Subject: [PATCH 05/13] make prisoner_id the primary key for new prisoner_details table --- .../visitallocationapi/model/entity/PrisonerDetails.kt | 6 +----- .../db/migration/V1_8__create_prisoner_details_table.sql | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt index 38f86e5..9692e4a 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt @@ -2,8 +2,6 @@ package uk.gov.justice.digital.hmpps.visitallocationapi.model.entity import jakarta.persistence.Column import jakarta.persistence.Entity -import jakarta.persistence.GeneratedValue -import jakarta.persistence.GenerationType import jakarta.persistence.Id import jakarta.persistence.Table import java.time.LocalDate @@ -11,10 +9,8 @@ import java.time.LocalDate @Entity @Table(name = "PRISONER_DETAILS") data class PrisonerDetails( - @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) - val id: Long = 0L, + @Id @Column(nullable = false) val prisonerId: String, diff --git a/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql index b064b45..0ea7450 100644 --- a/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql +++ b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql @@ -1,9 +1,5 @@ CREATE TABLE prisoner_details ( - id serial NOT NULL PRIMARY KEY, - prisoner_id VARCHAR(80) NOT NULL UNIQUE, + prisoner_id VARCHAR(80) NOT NULL UNIQUE PRIMARY KEY, last_allocated_date DATE NOT NULL ); - -CREATE INDEX idx_prisoner_details_prisoner_id ON prisoner_details (prisoner_id); - From b3e136255b44594f22e1706a83be9a348060ccc6 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Tue, 11 Mar 2025 12:08:46 +0000 Subject: [PATCH 06/13] clean up code layout and remove not needed unique on sql column --- .../service/ChangeLogService.kt | 2 +- .../service/NomisSyncService.kt | 68 ++++++++++++------- .../V1_8__create_prisoner_details_table.sql | 2 +- 3 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt index 15e546d..6736c3c 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/ChangeLogService.kt @@ -25,7 +25,7 @@ class ChangeLogService(private val changeLogRepository: ChangeLogRepository) { changeType = ChangeLogType.MIGRATION, changeSource = ChangeLogSource.SYSTEM, userId = "SYSTEM", - comment = "migrated prisoner ${migrationChangeDto.prisonerId}, with vo balance ${migrationChangeDto.voBalance} and pvo balance ${migrationChangeDto.pvoBalance}", + comment = "migrated prisoner ${migrationChangeDto.prisonerId}, with vo balance ${migrationChangeDto.voBalance} and pvo balance ${migrationChangeDto.pvoBalance} and lastAllocatedDate ${migrationChangeDto.lastVoAllocationDate}", ), ) } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt index 6e3fe45..aa3cd72 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt @@ -39,7 +39,7 @@ class NomisSyncService( changeLogService.logMigrationChange(migrationDto) - LOG.info("Finished NomisSyncService - migratePrisoner successfully") + LOG.info("Finished NomisSyncService - migratePrisoner ${migrationDto.prisonerId} successfully") } private fun migrateBalance(migrationDto: VisitAllocationPrisonerMigrationDto, type: VisitOrderType) { @@ -51,33 +51,10 @@ class NomisSyncService( when { balance > 0 -> { - LOG.info("Migrating prisoner ${migrationDto.prisonerId} with a ${type.name} balance of $balance") - val visitOrders = List(balance) { - VisitOrder( - prisonerId = migrationDto.prisonerId, - type = type, - status = VisitOrderStatus.AVAILABLE, - createdDate = migrationDto.lastVoAllocationDate, - expiryDate = null, - ) - } - visitOrderRepository.saveAll(visitOrders) + createPositiveVisitOrders(migrationDto, type, balance) } balance < 0 -> { - LOG.info("Migrating prisoner ${migrationDto.prisonerId} with a negative ${type.name} balance of $balance") - val negativeVisitOrders = List(abs(balance)) { - NegativeVisitOrder( - prisonerId = migrationDto.prisonerId, - status = NegativeVisitOrderStatus.USED, - type = if (type == VisitOrderType.VO) { - NegativeVisitOrderType.NEGATIVE_VO - } else { - NegativeVisitOrderType.NEGATIVE_PVO - }, - createdDate = LocalDate.now(), - ) - } - negativeVisitOrderRepository.saveAll(negativeVisitOrders) + createNegativeVisitOrders(migrationDto, type, balance) } else -> { LOG.info("Not migrating ${type.name} balance for prisoner ${migrationDto.prisonerId} as it's 0") @@ -85,6 +62,45 @@ class NomisSyncService( } } + private fun createPositiveVisitOrders( + migrationDto: VisitAllocationPrisonerMigrationDto, + type: VisitOrderType, + balance: Int, + ) { + LOG.info("Migrating prisoner ${migrationDto.prisonerId} with a ${type.name} balance of $balance") + val visitOrders = List(balance) { + VisitOrder( + prisonerId = migrationDto.prisonerId, + type = type, + status = VisitOrderStatus.AVAILABLE, + createdDate = migrationDto.lastVoAllocationDate, + expiryDate = null, + ) + } + visitOrderRepository.saveAll(visitOrders) + } + + private fun createNegativeVisitOrders( + migrationDto: VisitAllocationPrisonerMigrationDto, + type: VisitOrderType, + balance: Int, + ) { + LOG.info("Migrating prisoner ${migrationDto.prisonerId} with a negative ${type.name} balance of $balance") + val negativeVisitOrders = List(abs(balance)) { + NegativeVisitOrder( + prisonerId = migrationDto.prisonerId, + status = NegativeVisitOrderStatus.USED, + type = if (type == VisitOrderType.VO) { + NegativeVisitOrderType.NEGATIVE_VO + } else { + NegativeVisitOrderType.NEGATIVE_PVO + }, + createdDate = LocalDate.now(), + ) + } + negativeVisitOrderRepository.saveAll(negativeVisitOrders) + } + private fun migrateLastAllocatedDate(migrationDto: VisitAllocationPrisonerMigrationDto) { LOG.info("Migrating prisoner ${migrationDto.prisonerId} details (last allocated date - ${migrationDto.lastVoAllocationDate})") prisonerDetailsRepository.save(PrisonerDetails(prisonerId = migrationDto.prisonerId, lastAllocatedDate = migrationDto.lastVoAllocationDate)) diff --git a/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql index 0ea7450..63ff758 100644 --- a/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql +++ b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql @@ -1,5 +1,5 @@ CREATE TABLE prisoner_details ( - prisoner_id VARCHAR(80) NOT NULL UNIQUE PRIMARY KEY, + prisoner_id VARCHAR(80) NOT NULL PRIMARY KEY, last_allocated_date DATE NOT NULL ); From 47eb2d80dec3a22194aaec79265b650792a21f52 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Tue, 11 Mar 2025 13:50:04 +0000 Subject: [PATCH 07/13] move to use timestamp instead of date for vo tables --- .../model/entity/NegativeVisitOrder.kt | 3 +- .../model/entity/VisitOrder.kt | 3 +- .../repository/VisitOrderRepository.kt | 12 ++++---- .../service/AllocationService.kt | 7 ++--- .../service/NomisSyncService.kt | 6 ++-- ...1_5__create_negative_visit_order_table.sql | 2 +- ...er_date_to_timestamp_visit_order_table.sql | 23 +++++++++++++++ .../allocations/AllocationServiceTest.kt | 28 +++++++++---------- .../VisitAllocationByPrisonJobSqsTest.kt | 11 ++++---- 9 files changed, 60 insertions(+), 35 deletions(-) create mode 100644 src/main/resources/db/migration/V1_9__alter_date_to_timestamp_visit_order_table.sql diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt index 7b086c2..4d409af 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/NegativeVisitOrder.kt @@ -11,6 +11,7 @@ import jakarta.persistence.Table import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.NegativeVisitOrderType import java.time.LocalDate +import java.time.LocalDateTime @Entity @Table(name = "NEGATIVE_VISIT_ORDER") @@ -31,7 +32,7 @@ data class NegativeVisitOrder( val type: NegativeVisitOrderType, @Column(nullable = false) - val createdDate: LocalDate = LocalDate.now(), + val createdTimestamp: LocalDateTime = LocalDateTime.now(), @Column(nullable = false) val repaidDate: LocalDate? = null, diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/VisitOrder.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/VisitOrder.kt index a6eff96..c984006 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/VisitOrder.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/VisitOrder.kt @@ -11,6 +11,7 @@ import jakarta.persistence.Table import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import java.time.LocalDate +import java.time.LocalDateTime @Entity @Table(name = "VISIT_ORDER") @@ -31,7 +32,7 @@ data class VisitOrder( val status: VisitOrderStatus, @Column(nullable = false) - val createdDate: LocalDate = LocalDate.now(), + val createdTimestamp: LocalDateTime = LocalDateTime.now(), @Column(nullable = false) val expiryDate: LocalDate? = null, diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt index 1c040e6..67783d2 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt @@ -8,17 +8,17 @@ import org.springframework.transaction.annotation.Transactional import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder -import java.time.LocalDate +import java.time.LocalDateTime @Repository interface VisitOrderRepository : JpaRepository { @Query( - "SELECT vo.createdDate FROM VisitOrder vo WHERE vo.prisonerId = :prisonerId AND vo.type = :type ORDER BY vo.createdDate DESC LIMIT 1", + "SELECT vo.createdTimestamp FROM VisitOrder vo WHERE vo.prisonerId = :prisonerId AND vo.type = :type ORDER BY vo.createdTimestamp DESC LIMIT 1", ) fun findLastAllocatedDate( prisonerId: String, type: VisitOrderType, - ): LocalDate? + ): LocalDateTime? @Query( "SELECT COUNT (vo) FROM VisitOrder vo WHERE vo.prisonerId = :prisonerId AND vo.type = :type AND vo.status = :status", @@ -40,7 +40,7 @@ interface VisitOrderRepository : JpaRepository { WHERE prisoner_id = :prisonerId AND type = 'VO' AND status = 'ACCUMULATED' - ORDER BY created_date ASC + ORDER BY created_timestamp ASC LIMIT :amount) """, nativeQuery = true, @@ -59,7 +59,7 @@ interface VisitOrderRepository : JpaRepository { WHERE prisoner_id = :prisonerId AND type = 'PVO' AND status = 'AVAILABLE' - AND created_date < CURRENT_DATE - INTERVAL '28 days' + AND created_timestamp < CURRENT_TIMESTAMP - INTERVAL '28 days' """, nativeQuery = true, ) @@ -76,7 +76,7 @@ interface VisitOrderRepository : JpaRepository { WHERE prisoner_id = :prisonerId AND type = :#{#type.name()} AND status = 'AVAILABLE' - AND created_date < CURRENT_DATE - INTERVAL '28 days' + AND created_timestamp < CURRENT_TIMESTAMP - INTERVAL '28 days' """, nativeQuery = true, ) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt index d3fc6dc..2ef5c50 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt @@ -14,7 +14,6 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderAllocationPrisonJobRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository -import java.time.LocalDate import java.time.LocalDateTime @Transactional @@ -116,13 +115,13 @@ class AllocationService( prisonerId = prisonerId, type = type, status = VisitOrderStatus.AVAILABLE, - createdDate = LocalDate.now(), + createdTimestamp = LocalDateTime.now(), expiryDate = null, ) private fun isDueVO(prisonerId: String): Boolean { val lastVODate = visitOrderRepository.findLastAllocatedDate(prisonerId, VisitOrderType.VO) - return lastVODate == null || lastVODate <= LocalDate.now().minusDays(14) + return lastVODate == null || lastVODate <= LocalDateTime.now().minusDays(14) } private fun isDuePVO(prisonerId: String): Boolean { @@ -133,7 +132,7 @@ class AllocationService( return isDueVO(prisonerId) } - return lastPVODate <= LocalDate.now().minusDays(28) + return lastPVODate <= LocalDateTime.now().minusDays(28) } private fun generateVos(prisoner: PrisonerDto, prisonIncentivesForPrisonerLevel: PrisonIncentiveAmountsDto): List { diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt index aa3cd72..3175f26 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt @@ -15,7 +15,7 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder import uk.gov.justice.digital.hmpps.visitallocationapi.repository.NegativeVisitOrderRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository -import java.time.LocalDate +import java.time.LocalDateTime import kotlin.math.abs @Service @@ -73,7 +73,7 @@ class NomisSyncService( prisonerId = migrationDto.prisonerId, type = type, status = VisitOrderStatus.AVAILABLE, - createdDate = migrationDto.lastVoAllocationDate, + createdTimestamp = migrationDto.lastVoAllocationDate.atStartOfDay(), expiryDate = null, ) } @@ -95,7 +95,7 @@ class NomisSyncService( } else { NegativeVisitOrderType.NEGATIVE_PVO }, - createdDate = LocalDate.now(), + createdTimestamp = LocalDateTime.now(), ) } negativeVisitOrderRepository.saveAll(negativeVisitOrders) diff --git a/src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql b/src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql index 2fa3b24..5f7f2a9 100644 --- a/src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql +++ b/src/main/resources/db/migration/V1_5__create_negative_visit_order_table.sql @@ -4,7 +4,7 @@ CREATE TABLE negative_visit_order prisoner_id VARCHAR(80) NOT NULL, status VARCHAR(20) NOT NULL, type VARCHAR(20) NOT NULL, - created_date DATE default current_date, + created_timestamp timestamp NOT NULL default current_timestamp, repaid_date DATE ); diff --git a/src/main/resources/db/migration/V1_9__alter_date_to_timestamp_visit_order_table.sql b/src/main/resources/db/migration/V1_9__alter_date_to_timestamp_visit_order_table.sql new file mode 100644 index 0000000..51cfca7 --- /dev/null +++ b/src/main/resources/db/migration/V1_9__alter_date_to_timestamp_visit_order_table.sql @@ -0,0 +1,23 @@ +BEGIN; + +-- Drop existing indexes that reference created_date +DROP INDEX IF EXISTS idx_visit_order_composite; +DROP INDEX IF EXISTS idx_visit_order_last_allocated; + +-- Rename and modify the column +ALTER TABLE visit_order + RENAME COLUMN created_date TO created_timestamp; + +ALTER TABLE visit_order +ALTER COLUMN created_timestamp TYPE TIMESTAMP USING created_timestamp::TIMESTAMP, +ALTER COLUMN created_timestamp SET DEFAULT current_timestamp, +ALTER COLUMN created_timestamp SET NOT NULL; + +-- Recreate indexes with the updated column name +CREATE INDEX idx_visit_order_composite + ON visit_order (prisoner_id, type, status, created_timestamp); + +CREATE INDEX idx_visit_order_last_allocated + ON visit_order (prisoner_id, type, created_timestamp DESC); + +COMMIT; \ No newline at end of file diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt index 144b7ff..a0c2198 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt @@ -87,7 +87,7 @@ class AllocationServiceTest { visitOrders.count { it.type == VisitOrderType.VO } == 2 && visitOrders.count { it.type == VisitOrderType.PVO } == 1 && visitOrders.all { it.status == VisitOrderStatus.AVAILABLE } && - visitOrders.all { it.createdDate == LocalDate.now() } + visitOrders.all { it.createdTimestamp.toLocalDate() == LocalDate.now() } }, ) } @@ -135,7 +135,7 @@ class AllocationServiceTest { visitOrders.count { it.type == VisitOrderType.VO } == 2 && visitOrders.count { it.type == VisitOrderType.PVO } == 1 && visitOrders.all { it.status == VisitOrderStatus.AVAILABLE } && - visitOrders.all { it.createdDate == LocalDate.now() } + visitOrders.all { it.createdTimestamp.toLocalDate() == LocalDate.now() } }, ) } @@ -180,7 +180,7 @@ class AllocationServiceTest { visitOrders.size == 2 && visitOrders.count { it.type == VisitOrderType.VO } == 2 && visitOrders.all { it.status == VisitOrderStatus.AVAILABLE } && - visitOrders.all { it.createdDate == LocalDate.now() } + visitOrders.all { it.createdTimestamp.toLocalDate() == LocalDate.now() } }, ) } @@ -210,8 +210,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(14)) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14)) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) // Begin test runBlocking { @@ -228,7 +228,7 @@ class AllocationServiceTest { visitOrders.size == 2 && visitOrders.count { it.type == VisitOrderType.VO } == 2 && visitOrders.all { it.status == VisitOrderStatus.AVAILABLE } && - visitOrders.all { it.createdDate == LocalDate.now() } + visitOrders.all { it.createdTimestamp.toLocalDate() == LocalDate.now() } }, ) } @@ -258,7 +258,7 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(10)) + whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(10).atStartOfDay()) whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(null) // Begin test @@ -303,8 +303,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(14)) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(28)) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(28).atStartOfDay()) // Begin test runBlocking { @@ -322,7 +322,7 @@ class AllocationServiceTest { visitOrders.count { it.type == VisitOrderType.VO } == 2 && visitOrders.count { it.type == VisitOrderType.PVO } == 1 && visitOrders.all { it.status == VisitOrderStatus.AVAILABLE } && - visitOrders.all { it.createdDate == LocalDate.now() } + visitOrders.all { it.createdTimestamp.toLocalDate() == LocalDate.now() } }, ) } @@ -354,8 +354,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(1)) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14)) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(1).atStartOfDay()) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(4) @@ -397,8 +397,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(1)) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14)) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(1).atStartOfDay()) + lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(28) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt index 5772bb4..63d5e20 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt @@ -26,6 +26,7 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrderAl import uk.gov.justice.digital.hmpps.visitallocationapi.service.sqs.VisitAllocationEventJob import uk.gov.justice.hmpps.sqs.countMessagesOnQueue import java.time.LocalDate +import java.time.LocalDateTime import java.util.concurrent.TimeUnit class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { @@ -242,8 +243,8 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { fun `when visit allocation job run for a prison then processMessage is called and visit orders are accumulated for convicted prisoners`() { // Given - Some prisoners have pre-existing VOs, and a message is sent to start allocation job for prison val existingVOs = mutableListOf().apply { - addAll(List(2) { createVisitOrder(prisoner2.prisonerId, VisitOrderType.VO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(29)) }) - addAll(List(2) { createVisitOrder(prisoner3.prisonerId, VisitOrderType.VO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(14)) }) + addAll(List(2) { createVisitOrder(prisoner2.prisonerId, VisitOrderType.VO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(29).atStartOfDay()) }) + addAll(List(2) { createVisitOrder(prisoner3.prisonerId, VisitOrderType.VO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(14).atStartOfDay()) }) } visitOrderRepository.saveAll(existingVOs) @@ -315,8 +316,8 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { fun `when visit allocation job run for a prison then processMessage is called and visit orders are expired for convicted prisoners`() { // Given - Some prisoners have pre-existing VOs / PVOs, and a message is sent to start allocation job for prison val existingVOs = mutableListOf().apply { - addAll(List(28) { createVisitOrder(prisoner2.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED, LocalDate.now().minusDays(1)) }) - addAll(List(2) { createVisitOrder(prisoner3.prisonerId, VisitOrderType.PVO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(29)) }) + addAll(List(28) { createVisitOrder(prisoner2.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED, LocalDate.now().minusDays(1).atStartOfDay()) }) + addAll(List(2) { createVisitOrder(prisoner3.prisonerId, VisitOrderType.PVO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(29).atStartOfDay()) }) } visitOrderRepository.saveAll(existingVOs) @@ -642,5 +643,5 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { assertThat(visitOrderAllocationPrisonJob.endTimestamp).isNotNull() } - private fun createVisitOrder(prisonerId: String, type: VisitOrderType, status: VisitOrderStatus, createdDate: LocalDate): VisitOrder = VisitOrder(prisonerId = prisonerId, type = type, status = status, createdDate = createdDate) + private fun createVisitOrder(prisonerId: String, type: VisitOrderType, status: VisitOrderStatus, createdDateTime: LocalDateTime): VisitOrder = VisitOrder(prisonerId = prisonerId, type = type, status = status, createdTimestamp = createdDateTime) } From 1919394346029d8e45f9dc013933f8921873cb6a Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Tue, 11 Mar 2025 14:53:14 +0000 Subject: [PATCH 08/13] compare dates to avoid timing issues --- .../visitallocationapi/repository/VisitOrderRepository.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt index 67783d2..82902c6 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt @@ -59,7 +59,7 @@ interface VisitOrderRepository : JpaRepository { WHERE prisoner_id = :prisonerId AND type = 'PVO' AND status = 'AVAILABLE' - AND created_timestamp < CURRENT_TIMESTAMP - INTERVAL '28 days' + AND CAST(created_timestamp AS DATE) < CURRENT_DATE - INTERVAL '28 days' """, nativeQuery = true, ) @@ -74,9 +74,9 @@ interface VisitOrderRepository : JpaRepository { UPDATE visit_order SET status = 'ACCUMULATED' WHERE prisoner_id = :prisonerId - AND type = :#{#type.name()} + AND type = 'VO' AND status = 'AVAILABLE' - AND created_timestamp < CURRENT_TIMESTAMP - INTERVAL '28 days' + AND CAST(created_timestamp AS DATE) < CURRENT_DATE - INTERVAL '28 days' """, nativeQuery = true, ) From c36cbb38d49485570c2e953d0302fea6ffa643b8 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Wed, 12 Mar 2025 09:49:27 +0000 Subject: [PATCH 09/13] change vo dates to use new prisoner details table - part1 --- .../repository/PrisonerDetailsRepository.kt | 4 ++- .../service/AllocationService.kt | 11 ++++--- .../allocations/AllocationServiceTest.kt | 33 ++++++++++++------- .../events/EventsIntegrationTestBase.kt | 4 +++ .../VisitAllocationByPrisonJobSqsTest.kt | 13 ++++++++ 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt index 68b7cb1..779ca95 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt @@ -5,4 +5,6 @@ import org.springframework.stereotype.Repository import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails @Repository -interface PrisonerDetailsRepository : JpaRepository +interface PrisonerDetailsRepository : JpaRepository { + fun findByPrisonerId(prisonerId: String): PrisonerDetails? +} diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt index 2ef5c50..ff89625 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt @@ -12,8 +12,10 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.dto.prisoner.search.Priso import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderAllocationPrisonJobRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository +import java.time.LocalDate import java.time.LocalDateTime @Transactional @@ -23,6 +25,7 @@ class AllocationService( private val incentivesClient: IncentivesClient, private val visitOrderRepository: VisitOrderRepository, private val visitOrderAllocationPrisonJobRepository: VisitOrderAllocationPrisonJobRepository, + private val prisonerDetailsRepository: PrisonerDetailsRepository, private val prisonerRetryService: PrisonerRetryService, @Value("\${max.visit-orders:26}") val maxAccumulatedVisitOrders: Int, ) { @@ -120,19 +123,19 @@ class AllocationService( ) private fun isDueVO(prisonerId: String): Boolean { - val lastVODate = visitOrderRepository.findLastAllocatedDate(prisonerId, VisitOrderType.VO) - return lastVODate == null || lastVODate <= LocalDateTime.now().minusDays(14) + val lastVODate = prisonerDetailsRepository.findByPrisonerId(prisonerId)?.lastAllocatedDate + return lastVODate == null || lastVODate <= LocalDate.now().minusDays(14) } private fun isDuePVO(prisonerId: String): Boolean { val lastPVODate = visitOrderRepository.findLastAllocatedDate(prisonerId, VisitOrderType.PVO) - // If they haven't been given a PVO before, we wait until their VO due date to allocate it. + // If they haven't been given a PVO before, we wait until their VO due date to allocate it, to align the dates. if (lastPVODate == null) { return isDueVO(prisonerId) } - return lastPVODate <= LocalDateTime.now().minusDays(28) + return lastPVODate.toLocalDate() <= LocalDate.now().minusDays(28) } private fun generateVos(prisoner: PrisonerDto, prisonIncentivesForPrisonerLevel: PrisonIncentiveAmountsDto): List { diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt index a0c2198..de3e87e 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt @@ -5,7 +5,6 @@ import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.mockito.Mock -import org.mockito.Mockito.lenient import org.mockito.Mockito.verify import org.mockito.junit.jupiter.MockitoExtension import org.mockito.kotlin.any @@ -20,7 +19,9 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.dto.incentives.PrisonerIn import uk.gov.justice.digital.hmpps.visitallocationapi.dto.prisoner.search.PrisonerDto import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderAllocationPrisonJobRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository import uk.gov.justice.digital.hmpps.visitallocationapi.service.AllocationService @@ -39,6 +40,9 @@ class AllocationServiceTest { @Mock private lateinit var visitOrderRepository: VisitOrderRepository + @Mock + private lateinit var prisonerDetailsRepository: PrisonerDetailsRepository + @Mock private lateinit var visitOrderAllocationPrisonJobRepository: VisitOrderAllocationPrisonJobRepository @@ -49,7 +53,7 @@ class AllocationServiceTest { @BeforeEach fun setUp() { - allocationService = AllocationService(prisonerSearchClient, incentivesClient, visitOrderRepository, visitOrderAllocationPrisonJobRepository, prisonerRetryService, 26) + allocationService = AllocationService(prisonerSearchClient, incentivesClient, visitOrderRepository, visitOrderAllocationPrisonJobRepository, prisonerDetailsRepository, prisonerRetryService, 26) } // --- Start Allocation Tests --- \\ @@ -149,6 +153,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(14)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 0, levelCode = "STD") @@ -164,6 +169,7 @@ class AllocationServiceTest { ) whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) + whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) // Begin test runBlocking { @@ -194,6 +200,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(14)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -210,8 +217,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) + whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) // Begin test runBlocking { @@ -242,6 +249,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(10)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "ENH") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 3, privilegedVisitOrders = 2, levelCode = "ENH") @@ -258,7 +266,7 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(10).atStartOfDay()) + whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(null) // Begin test @@ -287,6 +295,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(14)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -303,8 +312,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(28).atStartOfDay()) + whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) + whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(28).atStartOfDay()) // Begin test runBlocking { @@ -338,6 +347,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(1)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -354,8 +364,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(1).atStartOfDay()) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) + whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(4) @@ -381,6 +391,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(1)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -397,8 +408,8 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.VO)).thenReturn(LocalDate.now().minusDays(1).atStartOfDay()) - lenient().whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) + whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(28) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/EventsIntegrationTestBase.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/EventsIntegrationTestBase.kt index bbcc8ce..8bc1484 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/EventsIntegrationTestBase.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/EventsIntegrationTestBase.kt @@ -16,6 +16,7 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.integration.events.LocalS import uk.gov.justice.digital.hmpps.visitallocationapi.integration.wiremock.HmppsAuthApiExtension import uk.gov.justice.digital.hmpps.visitallocationapi.integration.wiremock.IncentivesMockExtension import uk.gov.justice.digital.hmpps.visitallocationapi.integration.wiremock.PrisonerSearchMockExtension +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderAllocationPrisonJobRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderPrisonRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository @@ -81,6 +82,9 @@ abstract class EventsIntegrationTestBase { @MockitoSpyBean lateinit var visitOrderRepository: VisitOrderRepository + @MockitoSpyBean + lateinit var prisonerDetailsRepository: PrisonerDetailsRepository + @MockitoSpyBean lateinit var visitOrderAllocationPrisonJobRepository: VisitOrderAllocationPrisonJobRepository diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt index 63d5e20..568cf00 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt @@ -21,6 +21,7 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.integration.wiremock.IncentivesMockExtension.Companion.incentivesMockServer import uk.gov.justice.digital.hmpps.visitallocationapi.integration.wiremock.PrisonerSearchMockExtension.Companion.prisonerSearchMockServer +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrderAllocationPrisonJob import uk.gov.justice.digital.hmpps.visitallocationapi.service.sqs.VisitAllocationEventJob @@ -29,17 +30,27 @@ import java.time.LocalDate import java.time.LocalDateTime import java.util.concurrent.TimeUnit +// TODO: To fix the PVO issue; Add a PVO lastAllocatedDate column to the prisoner details table, and then at the end of allocation +// overwrite the column with the new date. Existing VO work done can stay, just needs more changes: +// 1. New column for the PVO and rename the VO column to mention it's a VO column. +// 2. use the new PVO date column to calculate the date, if it's null, check VO date... (Same as usual). +// 3. At the end of allocation, update the prisoner_details table to set both the VO / PVO date to "LocalDate.now()" (Date the allocation ran for each). +// 4. Update migration to set the date in each call (migrated VO balance -> sets VO date at end / migrate PVO balance -> sets PVO date at end). +// 4. Finally update these tests and any in the AllocationServiceTest class, to set the right dates for the prisonerDetails table (allocation dates). + class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { @BeforeEach fun setup() { visitOrderAllocationPrisonJobRepository.deleteAll() visitOrderRepository.deleteAll() + prisonerDetailsRepository.deleteAll() } @AfterEach fun cleanUp() { visitOrderAllocationPrisonJobRepository.deleteAll() visitOrderRepository.deleteAll() + prisonerDetailsRepository.deleteAll() } companion object { @@ -320,6 +331,8 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { addAll(List(2) { createVisitOrder(prisoner3.prisonerId, VisitOrderType.PVO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(29).atStartOfDay()) }) } visitOrderRepository.saveAll(existingVOs) + prisonerDetailsRepository.save(PrisonerDetails(prisoner2.prisonerId, LocalDate.now().minusDays(1))) + prisonerDetailsRepository.save(PrisonerDetails(prisoner3.prisonerId, LocalDate.now().minusDays(29))) val sendMessageRequestBuilder = SendMessageRequest.builder().queueUrl(prisonVisitsAllocationEventJobQueueUrl) val allocationJobReference = "job-ref" From 783823b15de134bc62bae11abecd7730d138177d Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Wed, 12 Mar 2025 11:12:17 +0000 Subject: [PATCH 10/13] Update new prisoner details table to include a date per vo and pvo Using these dates to now track allocation --- .../model/entity/PrisonerDetails.kt | 6 ++- .../repository/PrisonerDetailsRepository.kt | 20 ++++++++ .../service/AllocationService.kt | 19 +++++--- .../service/NomisSyncService.kt | 9 +++- .../service/PrisonerDetailsService.kt | 44 +++++++++++++++++ .../V1_8__create_prisoner_details_table.sql | 5 +- .../integration/NomisSyncControllerTest.kt | 6 +-- .../allocations/AllocationServiceTest.kt | 35 ++++++-------- .../VisitAllocationByPrisonJobSqsTest.kt | 48 ++++++++++++++----- 9 files changed, 146 insertions(+), 46 deletions(-) create mode 100644 src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt index 9692e4a..295433c 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/model/entity/PrisonerDetails.kt @@ -9,11 +9,13 @@ import java.time.LocalDate @Entity @Table(name = "PRISONER_DETAILS") data class PrisonerDetails( - @Id @Column(nullable = false) val prisonerId: String, @Column(nullable = false) - val lastAllocatedDate: LocalDate, + val lastVoAllocatedDate: LocalDate, + + @Column(nullable = true) + val lastPvoAllocatedDate: LocalDate?, ) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt index 779ca95..e4fc0e4 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt @@ -1,10 +1,30 @@ package uk.gov.justice.digital.hmpps.visitallocationapi.repository import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.data.jpa.repository.Modifying +import org.springframework.data.jpa.repository.Query import org.springframework.stereotype.Repository +import org.springframework.transaction.annotation.Transactional import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails +import java.time.LocalDate @Repository interface PrisonerDetailsRepository : JpaRepository { fun findByPrisonerId(prisonerId: String): PrisonerDetails? + + @Transactional + @Modifying + @Query( + value = "UPDATE prisoner_details SET last_vo_allocated_date = :newLastAllocatedDate WHERE prisoner_id = :prisonerId", + nativeQuery = true, + ) + fun updatePrisonerLastVoAllocatedDate(prisonerId: String, newLastAllocatedDate: LocalDate) + + @Transactional + @Modifying + @Query( + value = "UPDATE prisoner_details SET last_pvo_allocated_date = :newLastAllocatedDate WHERE prisoner_id = :prisonerId", + nativeQuery = true, + ) + fun updatePrisonerLastPvoAllocatedDate(prisonerId: String, newLastAllocatedDate: LocalDate) } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt index ff89625..6d023da 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/AllocationService.kt @@ -12,7 +12,6 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.dto.prisoner.search.Priso import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder -import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderAllocationPrisonJobRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository import java.time.LocalDate @@ -25,7 +24,7 @@ class AllocationService( private val incentivesClient: IncentivesClient, private val visitOrderRepository: VisitOrderRepository, private val visitOrderAllocationPrisonJobRepository: VisitOrderAllocationPrisonJobRepository, - private val prisonerDetailsRepository: PrisonerDetailsRepository, + private val prisonerDetailsService: PrisonerDetailsService, private val prisonerRetryService: PrisonerRetryService, @Value("\${max.visit-orders:26}") val maxAccumulatedVisitOrders: Int, ) { @@ -85,11 +84,20 @@ class AllocationService( visitOrderRepository.saveAll(visitOrders) + updateLastAllocatedDates(prisoner, visitOrders) + LOG.info( "Successfully generated ${visitOrders.size} visit orders for prisoner prisoner $prisonerId: " + "${visitOrders.count { it.type == VisitOrderType.PVO }} PVOs and ${visitOrders.count { it.type == VisitOrderType.VO }} VOs", ) } + private fun updateLastAllocatedDates(prisoner: PrisonerDto, visitOrders: MutableList) { + prisonerDetailsService.updateVoLastCreatedDateOrCreatePrisoner(prisonerId = prisoner.prisonerId, LocalDate.now()) + if (visitOrders.any { it.type == VisitOrderType.PVO }) { + prisonerDetailsService.updatePvoLastCreatedDate(prisonerId = prisoner.prisonerId, LocalDate.now()) + } + } + private fun processPrisonerAccumulation(prisonerId: String) { LOG.info("Entered AllocationService - processPrisonerAccumulation with prisonerId: $prisonerId") @@ -123,19 +131,19 @@ class AllocationService( ) private fun isDueVO(prisonerId: String): Boolean { - val lastVODate = prisonerDetailsRepository.findByPrisonerId(prisonerId)?.lastAllocatedDate + val lastVODate = prisonerDetailsService.getPrisoner(prisonerId)?.lastVoAllocatedDate return lastVODate == null || lastVODate <= LocalDate.now().minusDays(14) } private fun isDuePVO(prisonerId: String): Boolean { - val lastPVODate = visitOrderRepository.findLastAllocatedDate(prisonerId, VisitOrderType.PVO) + val lastPVODate = prisonerDetailsService.getPrisoner(prisonerId)?.lastPvoAllocatedDate // If they haven't been given a PVO before, we wait until their VO due date to allocate it, to align the dates. if (lastPVODate == null) { return isDueVO(prisonerId) } - return lastPVODate.toLocalDate() <= LocalDate.now().minusDays(28) + return lastPVODate <= LocalDate.now().minusDays(28) } private fun generateVos(prisoner: PrisonerDto, prisonIncentivesForPrisonerLevel: PrisonIncentiveAmountsDto): List { @@ -146,7 +154,6 @@ class AllocationService( visitOrders.add(createVisitOrder(prisoner.prisonerId, VisitOrderType.VO)) } } - return visitOrders.toList() } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt index 3175f26..866052c 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/NomisSyncService.kt @@ -103,6 +103,13 @@ class NomisSyncService( private fun migrateLastAllocatedDate(migrationDto: VisitAllocationPrisonerMigrationDto) { LOG.info("Migrating prisoner ${migrationDto.prisonerId} details (last allocated date - ${migrationDto.lastVoAllocationDate})") - prisonerDetailsRepository.save(PrisonerDetails(prisonerId = migrationDto.prisonerId, lastAllocatedDate = migrationDto.lastVoAllocationDate)) + + val lastPvoAllocatedDate = if (migrationDto.pvoBalance != 0) { + migrationDto.lastVoAllocationDate + } else { + null + } + + prisonerDetailsRepository.save(PrisonerDetails(prisonerId = migrationDto.prisonerId, lastVoAllocatedDate = migrationDto.lastVoAllocationDate, lastPvoAllocatedDate = lastPvoAllocatedDate)) } } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt new file mode 100644 index 0000000..3efc91a --- /dev/null +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt @@ -0,0 +1,44 @@ +package uk.gov.justice.digital.hmpps.visitallocationapi.service + +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import org.springframework.stereotype.Service +import org.springframework.transaction.annotation.Transactional +import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails +import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository +import java.time.LocalDate + +@Transactional +@Service +class PrisonerDetailsService(private val prisonerDetailsRepository: PrisonerDetailsRepository) { + companion object { + val LOG: Logger = LoggerFactory.getLogger(this::class.java) + } + + fun getPrisoner(prisonerId: String): PrisonerDetails? = prisonerDetailsRepository.findByPrisonerId(prisonerId) + + fun updateVoLastCreatedDateOrCreatePrisoner(prisonerId: String, newLastAllocatedDate: LocalDate) { + LOG.info("Entered PrisonerDetailsService updateVoLastCreatedDateOrCreatePrisoner for prisoner $prisonerId with date $newLastAllocatedDate") + // Check if the prisoner exists + val prisonerDetails = prisonerDetailsRepository.findByPrisonerId(prisonerId) + + if (prisonerDetails != null) { + LOG.info("Prisoner $prisonerId not found, creating new record") + // If prisoner exists, update the record + prisonerDetailsRepository.updatePrisonerLastVoAllocatedDate(prisonerId, newLastAllocatedDate) + } else { + // If prisoner does not exist, create a new record + val newPrisoner = PrisonerDetails( + prisonerId = prisonerId, + lastVoAllocatedDate = newLastAllocatedDate, + lastPvoAllocatedDate = null, + ) + prisonerDetailsRepository.save(newPrisoner) + } + } + + fun updatePvoLastCreatedDate(prisonerId: String, newLastAllocatedDate: LocalDate) { + LOG.info("Entered PrisonerDetailsService updatePvoLastCreatedDate for prisoner $prisonerId with date $newLastAllocatedDate") + prisonerDetailsRepository.updatePrisonerLastPvoAllocatedDate(prisonerId, newLastAllocatedDate) + } +} diff --git a/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql index 63ff758..f002d25 100644 --- a/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql +++ b/src/main/resources/db/migration/V1_8__create_prisoner_details_table.sql @@ -1,5 +1,6 @@ CREATE TABLE prisoner_details ( - prisoner_id VARCHAR(80) NOT NULL PRIMARY KEY, - last_allocated_date DATE NOT NULL + prisoner_id VARCHAR(80) NOT NULL PRIMARY KEY, + last_vo_allocated_date DATE NOT NULL, + last_pvo_allocated_date DATE ); diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt index e7c9d49..16cc8b8 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/NomisSyncControllerTest.kt @@ -69,7 +69,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { val prisonerDetails = prisonerDetailsRepository.findAll() assertThat(prisonerDetails.size).isEqualTo(1) assertThat(prisonerDetails.first().prisonerId).isEqualTo(prisonerMigrationDto.prisonerId) - assertThat(prisonerDetails.first().lastAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) + assertThat(prisonerDetails.first().lastVoAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) val changeLog = changeLogRepository.findAll() assertThat(changeLog.size).isEqualTo(1) @@ -100,7 +100,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { val prisonerDetails = prisonerDetailsRepository.findAll() assertThat(prisonerDetails.size).isEqualTo(1) assertThat(prisonerDetails.first().prisonerId).isEqualTo(prisonerMigrationDto.prisonerId) - assertThat(prisonerDetails.first().lastAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) + assertThat(prisonerDetails.first().lastVoAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) val changeLog = changeLogRepository.findAll() assertThat(changeLog.size).isEqualTo(1) @@ -136,7 +136,7 @@ class NomisSyncControllerTest : IntegrationTestBase() { val prisonerDetails = prisonerDetailsRepository.findAll() assertThat(prisonerDetails.size).isEqualTo(1) assertThat(prisonerDetails.first().prisonerId).isEqualTo(prisonerMigrationDto.prisonerId) - assertThat(prisonerDetails.first().lastAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) + assertThat(prisonerDetails.first().lastVoAllocatedDate).isEqualTo(prisonerMigrationDto.lastVoAllocationDate) val changeLog = changeLogRepository.findAll() assertThat(changeLog.size).isEqualTo(1) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt index de3e87e..2a25d5e 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt @@ -21,10 +21,10 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderStatus import uk.gov.justice.digital.hmpps.visitallocationapi.enums.VisitOrderType import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.PrisonerDetails import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder -import uk.gov.justice.digital.hmpps.visitallocationapi.repository.PrisonerDetailsRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderAllocationPrisonJobRepository import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository import uk.gov.justice.digital.hmpps.visitallocationapi.service.AllocationService +import uk.gov.justice.digital.hmpps.visitallocationapi.service.PrisonerDetailsService import uk.gov.justice.digital.hmpps.visitallocationapi.service.PrisonerRetryService import java.time.LocalDate @@ -41,7 +41,7 @@ class AllocationServiceTest { private lateinit var visitOrderRepository: VisitOrderRepository @Mock - private lateinit var prisonerDetailsRepository: PrisonerDetailsRepository + private lateinit var prisonerDetailsService: PrisonerDetailsService @Mock private lateinit var visitOrderAllocationPrisonJobRepository: VisitOrderAllocationPrisonJobRepository @@ -53,7 +53,7 @@ class AllocationServiceTest { @BeforeEach fun setUp() { - allocationService = AllocationService(prisonerSearchClient, incentivesClient, visitOrderRepository, visitOrderAllocationPrisonJobRepository, prisonerDetailsRepository, prisonerRetryService, 26) + allocationService = AllocationService(prisonerSearchClient, incentivesClient, visitOrderRepository, visitOrderAllocationPrisonJobRepository, prisonerDetailsService, prisonerRetryService, 26) } // --- Start Allocation Tests --- \\ @@ -153,7 +153,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) - val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(14)) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastVoAllocatedDate = LocalDate.now().minusDays(14), null) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 0, levelCode = "STD") @@ -169,7 +169,7 @@ class AllocationServiceTest { ) whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) + whenever(prisonerDetailsService.getPrisoner(prisonerId)).thenReturn(prisonerDetails) // Begin test runBlocking { @@ -200,7 +200,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) - val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(14)) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastVoAllocatedDate = LocalDate.now().minusDays(14), LocalDate.now().minusDays(14)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -217,8 +217,7 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) - whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + whenever(prisonerDetailsService.getPrisoner(prisonerId)).thenReturn(prisonerDetails) // Begin test runBlocking { @@ -249,7 +248,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) - val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(10)) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastVoAllocatedDate = LocalDate.now().minusDays(10), null) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "ENH") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 3, privilegedVisitOrders = 2, levelCode = "ENH") @@ -266,8 +265,7 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) - whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(null) + whenever(prisonerDetailsService.getPrisoner(prisonerId)).thenReturn(prisonerDetails) // Begin test runBlocking { @@ -295,7 +293,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) - val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(14)) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastVoAllocatedDate = LocalDate.now().minusDays(14), LocalDate.now().minusDays(28)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -312,8 +310,7 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) - whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(28).atStartOfDay()) + whenever(prisonerDetailsService.getPrisoner(prisonerId)).thenReturn(prisonerDetails) // Begin test runBlocking { @@ -347,7 +344,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) - val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(1)) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastVoAllocatedDate = LocalDate.now().minusDays(1), LocalDate.now().minusDays(14)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -364,8 +361,7 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) - whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + whenever(prisonerDetailsService.getPrisoner(prisonerId)).thenReturn(prisonerDetails) whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(4) @@ -391,7 +387,7 @@ class AllocationServiceTest { val prisonerId = "AA123456" val prisonId = "MDI" val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId) - val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastAllocatedDate = LocalDate.now().minusDays(1)) + val prisonerDetails = PrisonerDetails(prisonerId = prisonerId, lastVoAllocatedDate = LocalDate.now().minusDays(1), LocalDate.now().minusDays(14)) val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD") val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD") @@ -408,8 +404,7 @@ class AllocationServiceTest { whenever(incentivesClient.getPrisonIncentiveLevels(prisonId)).thenReturn(listOf(prisonIncentiveAmounts)) whenever(incentivesClient.getPrisonerIncentiveReviewHistory(prisoner.prisonerId)).thenReturn(prisonerIncentive) - whenever(prisonerDetailsRepository.findByPrisonerId(prisonerId)).thenReturn(prisonerDetails) - whenever(visitOrderRepository.findLastAllocatedDate(prisoner.prisonerId, VisitOrderType.PVO)).thenReturn(LocalDate.now().minusDays(14).atStartOfDay()) + whenever(prisonerDetailsService.getPrisoner(prisonerId)).thenReturn(prisonerDetails) whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(28) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt index 568cf00..1a1e449 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/VisitAllocationByPrisonJobSqsTest.kt @@ -30,14 +30,6 @@ import java.time.LocalDate import java.time.LocalDateTime import java.util.concurrent.TimeUnit -// TODO: To fix the PVO issue; Add a PVO lastAllocatedDate column to the prisoner details table, and then at the end of allocation -// overwrite the column with the new date. Existing VO work done can stay, just needs more changes: -// 1. New column for the PVO and rename the VO column to mention it's a VO column. -// 2. use the new PVO date column to calculate the date, if it's null, check VO date... (Same as usual). -// 3. At the end of allocation, update the prisoner_details table to set both the VO / PVO date to "LocalDate.now()" (Date the allocation ran for each). -// 4. Update migration to set the date in each call (migrated VO balance -> sets VO date at end / migrate PVO balance -> sets PVO date at end). -// 4. Finally update these tests and any in the AllocationServiceTest class, to set the right dates for the prisonerDetails table (allocation dates). - class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { @BeforeEach fun setup() { @@ -119,10 +111,13 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { verify(visitOrderAllocationPrisonJobRepository, times(1)).updateEndTimestampAndStats(any(), any(), any(), any(), any(), any()) val visitOrderAllocationPrisonJobs = visitOrderAllocationPrisonJobRepository.findAll() assertVisitOrderAllocationPrisonJob(visitOrderAllocationPrisonJobs[0], null, convictedPrisoners = 3, processedPrisoners = 3, failedPrisoners = 0) + + verify(prisonerDetailsRepository, times(3)).save(any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastPvoAllocatedDate(any(), any()) } /** - * Scenario - Allocation: Visit allocation job is run, and all prisoners are allocated visit orders (VO / PVO). + * Scenario - Allocation: Visit allocation job is run with lowercase, and all prisoners are still allocated visit orders (VO / PVO). * Prisoner1 - Standard incentive, Gets 1 VO, 0 PVO. * Prisoner2 - Enhanced incentive, Gets 2 VO, 1 PVO. * Prisoner3 - Enhanced2 incentive, Gets 3 VO, 2 PVOs. @@ -182,10 +177,13 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { verify(visitOrderAllocationPrisonJobRepository, times(1)).updateEndTimestampAndStats(any(), any(), any(), any(), any(), any()) val visitOrderAllocationPrisonJobs = visitOrderAllocationPrisonJobRepository.findAll() assertVisitOrderAllocationPrisonJob(visitOrderAllocationPrisonJobs[0], null, convictedPrisoners = 3, processedPrisoners = 3, failedPrisoners = 0) + + verify(prisonerDetailsRepository, times(3)).save(any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastPvoAllocatedDate(any(), any()) } /** - * Scenario - Allocation: Visit allocation job is run, and all prisoners are allocated visit orders (VO / PVO). + * Scenario - Allocation: Visit allocation job is run but getAllPrisonIncentiveLevels fails, and all prisoners are still allocated visit orders (VO / PVO). * Prisoner1 - Standard incentive, Gets 1 VO, 0 PVO. * Prisoner2 - Enhanced incentive, Gets 2 VO, 1 PVO. * Prisoner3 - Enhanced2 incentive, Gets 3 VO, 2 PVOs. @@ -242,6 +240,9 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { verify(visitOrderAllocationPrisonJobRepository, times(1)).updateEndTimestampAndStats(any(), any(), any(), any(), any(), any()) val visitOrderAllocationPrisonJobs = visitOrderAllocationPrisonJobRepository.findAll() assertVisitOrderAllocationPrisonJob(visitOrderAllocationPrisonJobs[0], null, convictedPrisoners = 3, processedPrisoners = 3, failedPrisoners = 0) + + verify(prisonerDetailsRepository, times(3)).save(any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastPvoAllocatedDate(any(), any()) } /** @@ -259,6 +260,12 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { } visitOrderRepository.saveAll(existingVOs) + val existingPrisonerDetails = mutableListOf().apply { + add(PrisonerDetails(prisoner2.prisonerId, LocalDate.now().minusDays(14), null)) + add(PrisonerDetails(prisoner3.prisonerId, LocalDate.now().minusDays(14), null)) + } + prisonerDetailsRepository.saveAll(existingPrisonerDetails) + val sendMessageRequestBuilder = SendMessageRequest.builder().queueUrl(prisonVisitsAllocationEventJobQueueUrl) val allocationJobReference = "job-ref" val event = VisitAllocationEventJob(allocationJobReference, PRISON_CODE) @@ -311,6 +318,10 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { val visitOrderAllocationPrisonJobs = visitOrderAllocationPrisonJobRepository.findAll() assertVisitOrderAllocationPrisonJob(visitOrderAllocationPrisonJobs[0], null, convictedPrisoners = 3, processedPrisoners = 3, failedPrisoners = 0) + + verify(prisonerDetailsRepository, times(1)).save(any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastVoAllocatedDate(any(), any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastPvoAllocatedDate(any(), any()) } } @@ -331,8 +342,11 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { addAll(List(2) { createVisitOrder(prisoner3.prisonerId, VisitOrderType.PVO, VisitOrderStatus.AVAILABLE, LocalDate.now().minusDays(29).atStartOfDay()) }) } visitOrderRepository.saveAll(existingVOs) - prisonerDetailsRepository.save(PrisonerDetails(prisoner2.prisonerId, LocalDate.now().minusDays(1))) - prisonerDetailsRepository.save(PrisonerDetails(prisoner3.prisonerId, LocalDate.now().minusDays(29))) + val existingPrisonerDetails = mutableListOf().apply { + add(PrisonerDetails(prisoner2.prisonerId, LocalDate.now().minusDays(1), null)) + add(PrisonerDetails(prisoner3.prisonerId, LocalDate.now().minusDays(14), LocalDate.now().minusDays(29))) + } + prisonerDetailsRepository.saveAll(existingPrisonerDetails) val sendMessageRequestBuilder = SendMessageRequest.builder().queueUrl(prisonVisitsAllocationEventJobQueueUrl) val allocationJobReference = "job-ref" @@ -386,6 +400,10 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { assertVisitOrdersAssignedBy(visitOrders, prisoner3.prisonerId, VisitOrderType.PVO, VisitOrderStatus.EXPIRED, 2) val visitOrderAllocationPrisonJobs = visitOrderAllocationPrisonJobRepository.findAll() assertVisitOrderAllocationPrisonJob(visitOrderAllocationPrisonJobs[0], null, convictedPrisoners = 3, processedPrisoners = 3, failedPrisoners = 0) + + verify(prisonerDetailsRepository, times(1)).save(any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastVoAllocatedDate(any(), any()) + verify(prisonerDetailsRepository, times(1)).updatePrisonerLastPvoAllocatedDate(any(), any()) } } @@ -456,6 +474,9 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { verify(visitOrderAllocationPrisonJobRepository, times(1)).updateEndTimestampAndStats(any(), any(), any(), any(), any(), any()) val visitOrderAllocationPrisonJobs = visitOrderAllocationPrisonJobRepository.findAll() assertVisitOrderAllocationPrisonJob(visitOrderAllocationPrisonJobs[0], null, convictedPrisoners = 4, processedPrisoners = 3, failedPrisoners = 1) + + verify(prisonerDetailsRepository, times(3)).save(any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastPvoAllocatedDate(any(), any()) } /** @@ -525,6 +546,9 @@ class VisitAllocationByPrisonJobSqsTest : EventsIntegrationTestBase() { verify(visitOrderAllocationPrisonJobRepository, times(1)).updateEndTimestampAndStats(any(), any(), any(), any(), any(), any()) val visitOrderAllocationPrisonJobs = visitOrderAllocationPrisonJobRepository.findAll() assertVisitOrderAllocationPrisonJob(visitOrderAllocationPrisonJobs[0], null, convictedPrisoners = 4, processedPrisoners = 3, failedPrisoners = 1) + + verify(prisonerDetailsRepository, times(3)).save(any()) + verify(prisonerDetailsRepository, times(2)).updatePrisonerLastPvoAllocatedDate(any(), any()) } /** From 73d828e8106e7a1e922f0091048305e222ee4e67 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Wed, 12 Mar 2025 11:15:45 +0000 Subject: [PATCH 11/13] add cleanup before each test --- .../integration/events/PrisonerRetryQueueHandlerTest.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt index 1cafee4..17fb823 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt @@ -5,6 +5,7 @@ import org.awaitility.kotlin.await import org.awaitility.kotlin.matches import org.awaitility.kotlin.untilAsserted import org.awaitility.kotlin.untilCallTo +import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.mockito.kotlin.any import org.mockito.kotlin.times @@ -26,6 +27,13 @@ class PrisonerRetryQueueHandlerTest : EventsIntegrationTestBase() { @MockitoSpyBean lateinit var visitAllocationPrisonerRetryQueueListenerSpy: VisitAllocationPrisonerRetryQueueListener + @BeforeEach + fun setup() { + visitOrderAllocationPrisonJobRepository.deleteAll() + visitOrderRepository.deleteAll() + prisonerDetailsRepository.deleteAll() + } + @Test fun `when prisoner put on retry queue the message is processed`() { // Given From e6a532c1dcacfe81fe07bd7dbd9c55661ecb6e7a Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Wed, 12 Mar 2025 11:20:15 +0000 Subject: [PATCH 12/13] Attempt fix test with cleanup after each test --- .../integration/events/PrisonerRetryQueueHandlerTest.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt index 17fb823..32ba5a2 100644 --- a/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt +++ b/src/test/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/integration/events/PrisonerRetryQueueHandlerTest.kt @@ -5,6 +5,7 @@ import org.awaitility.kotlin.await import org.awaitility.kotlin.matches import org.awaitility.kotlin.untilAsserted import org.awaitility.kotlin.untilCallTo +import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.mockito.kotlin.any @@ -34,6 +35,13 @@ class PrisonerRetryQueueHandlerTest : EventsIntegrationTestBase() { prisonerDetailsRepository.deleteAll() } + @AfterEach + fun cleanup() { + visitOrderAllocationPrisonJobRepository.deleteAll() + visitOrderRepository.deleteAll() + prisonerDetailsRepository.deleteAll() + } + @Test fun `when prisoner put on retry queue the message is processed`() { // Given From a254c49e426a6f8f4a4fbe62fdfe13753a93b919 Mon Sep 17 00:00:00 2001 From: adaviesMOJ Date: Wed, 12 Mar 2025 12:27:32 +0000 Subject: [PATCH 13/13] Update as per PR comments --- .../repository/PrisonerDetailsRepository.kt | 10 ++-------- .../service/PrisonerDetailsService.kt | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt index e4fc0e4..fb92e8e 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/PrisonerDetailsRepository.kt @@ -14,17 +14,11 @@ interface PrisonerDetailsRepository : JpaRepository { @Transactional @Modifying - @Query( - value = "UPDATE prisoner_details SET last_vo_allocated_date = :newLastAllocatedDate WHERE prisoner_id = :prisonerId", - nativeQuery = true, - ) + @Query("UPDATE PrisonerDetails pd SET pd.lastVoAllocatedDate = :newLastAllocatedDate WHERE pd.prisonerId = :prisonerId") fun updatePrisonerLastVoAllocatedDate(prisonerId: String, newLastAllocatedDate: LocalDate) @Transactional @Modifying - @Query( - value = "UPDATE prisoner_details SET last_pvo_allocated_date = :newLastAllocatedDate WHERE prisoner_id = :prisonerId", - nativeQuery = true, - ) + @Query("UPDATE PrisonerDetails pd SET pd.lastPvoAllocatedDate = :newLastAllocatedDate WHERE pd.prisonerId = :prisonerId") fun updatePrisonerLastPvoAllocatedDate(prisonerId: String, newLastAllocatedDate: LocalDate) } diff --git a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt index 3efc91a..4777ab0 100644 --- a/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt +++ b/src/main/kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/service/PrisonerDetailsService.kt @@ -23,20 +23,25 @@ class PrisonerDetailsService(private val prisonerDetailsRepository: PrisonerDeta val prisonerDetails = prisonerDetailsRepository.findByPrisonerId(prisonerId) if (prisonerDetails != null) { - LOG.info("Prisoner $prisonerId not found, creating new record") + LOG.info("Existing prisoner $prisonerId found - Updating last VO allocated date to $newLastAllocatedDate") // If prisoner exists, update the record prisonerDetailsRepository.updatePrisonerLastVoAllocatedDate(prisonerId, newLastAllocatedDate) } else { // If prisoner does not exist, create a new record - val newPrisoner = PrisonerDetails( - prisonerId = prisonerId, - lastVoAllocatedDate = newLastAllocatedDate, - lastPvoAllocatedDate = null, - ) - prisonerDetailsRepository.save(newPrisoner) + createNewPrisonerDetails(prisonerId, newLastAllocatedDate) } } + private fun createNewPrisonerDetails(prisonerId: String, newLastAllocatedDate: LocalDate) { + LOG.info("Prisoner $prisonerId not found, creating new record") + val newPrisoner = PrisonerDetails( + prisonerId = prisonerId, + lastVoAllocatedDate = newLastAllocatedDate, + lastPvoAllocatedDate = null, + ) + prisonerDetailsRepository.save(newPrisoner) + } + fun updatePvoLastCreatedDate(prisonerId: String, newLastAllocatedDate: LocalDate) { LOG.info("Entered PrisonerDetailsService updatePvoLastCreatedDate for prisoner $prisonerId with date $newLastAllocatedDate") prisonerDetailsRepository.updatePrisonerLastPvoAllocatedDate(prisonerId, newLastAllocatedDate)