VB-5050: Accumulation / Expiration (#25)

* add integration test * fix checkstyle * fix assertions * fix expiring test * fix unit tests * Add cleanup after each * fix waiting for routine to finish * fix checkstyles * add more logging and split accumulation/expiration logic * optimise query after PR comments
ministryofjustice · Feb 7, 2025 · 99153e8 · 99153e8
1 parent bb89b10
commit 99153e8
Show file tree

Hide file tree

Showing 5 changed files with 218 additions and 40 deletions.
diff --git a/...kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt b/...kotlin/uk/gov/justice/digital/hmpps/visitallocationapi/repository/VisitOrderRepository.kt
@@ -1,8 +1,11 @@
 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.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
@@ -16,4 +19,53 @@ interface VisitOrderRepository : JpaRepository<VisitOrder, Long> {
     prisonerId: String,
     type: VisitOrderType,
   ): LocalDate?
+
+  @Query(
+    "SELECT COUNT (vo) FROM VisitOrder vo WHERE vo.prisonerId = :prisonerId AND vo.type = :type AND vo.status = :status",
+  )
+  fun countAllVisitOrders(
+    prisonerId: String,
+    type: VisitOrderType,
+    status: VisitOrderStatus,
+  ): Int
+
+  @Transactional
+  @Modifying
+  @Query(
+    value = """
+        UPDATE visit_order 
+        SET status = 'EXPIRED', expiry_date = CURRENT_DATE
+        WHERE id IN (SELECT id 
+            FROM visit_order 
+            WHERE prisoner_id = :prisonerId 
+              AND type = :#{#type.name()}
+              AND status = 'ACCUMULATED' 
+            ORDER BY created_date ASC 
+            LIMIT :amount)
+    """,
+    nativeQuery = true,
+  )
+  fun expireOldestAccumulatedVisitOrders(
+    prisonerId: String,
+    type: VisitOrderType,
+    amount: Int,
+  ): Int
+
+  @Transactional
+  @Modifying
+  @Query(
+    value = """
+        UPDATE visit_order
+        SET status = 'ACCUMULATED'
+            WHERE prisoner_id = :prisonerId
+              AND type = :#{#type.name()}
+              AND status = 'AVAILABLE'
+              AND created_date < CURRENT_DATE - INTERVAL '28 days'
+    """,
+    nativeQuery = true,
+  )
+  fun updateAvailableVisitOrdersOver28DaysToAccumulated(
+    prisonerId: String,
+    type: VisitOrderType,
+  )
 }
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
@@ -2,6 +2,7 @@ package uk.gov.justice.digital.hmpps.visitallocationapi.service
 
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
+import org.springframework.beans.factory.annotation.Value
 import org.springframework.stereotype.Service
 import org.springframework.transaction.annotation.Transactional
 import uk.gov.justice.digital.hmpps.visitallocationapi.clients.IncentivesClient
@@ -14,17 +15,33 @@ import uk.gov.justice.digital.hmpps.visitallocationapi.model.entity.VisitOrder
 import uk.gov.justice.digital.hmpps.visitallocationapi.repository.VisitOrderRepository
 import java.time.LocalDate
 
+@Transactional
 @Service
 class AllocationService(
   private val prisonerSearchClient: PrisonerSearchClient,
   private val incentivesClient: IncentivesClient,
   private val visitOrderRepository: VisitOrderRepository,
+  @Value("\${max.visit-orders:26}") val maxAccumulatedVisitOrders: Int,
 ) {
   companion object {
     val LOG: Logger = LoggerFactory.getLogger(this::class.java)
   }
 
-  @Transactional
+  suspend fun processPrison(prisonId: String) {
+    LOG.info("Entered AllocationService - processPrisonAllocation with prisonCode: $prisonId")
+
+    val allPrisoners = prisonerSearchClient.getConvictedPrisonersByPrisonId(prisonId)
+    val allIncentiveLevels = incentivesClient.getPrisonIncentiveLevels(prisonId)
+
+    for (prisoner in allPrisoners) {
+      processPrisonerAllocation(prisoner.prisonerId, prisoner, allIncentiveLevels)
+      processPrisonerAccumulation(prisoner.prisonerId)
+      processPrisonerExpiration(prisoner.prisonerId)
+    }
+
+    LOG.info("Finished AllocationService - processPrisonAllocation with prisonCode: $prisonId, total records processed : ${allPrisoners.size}")
+  }
+
   suspend fun processPrisonerAllocation(prisonerId: String, prisonerDto: PrisonerDto? = null, allPrisonIncentiveAmounts: List<PrisonIncentiveAmountsDto>? = null) {
     LOG.info("Entered AllocationService - processPrisonerAllocation with prisonerId $prisonerId")
 
@@ -48,18 +65,21 @@ class AllocationService(
     )
   }
 
-  @Transactional
-  suspend fun processPrisonAllocation(prisonId: String) {
-    LOG.info("Entered AllocationService - processPrisonAllocation with prisonCode: $prisonId")
+  private fun processPrisonerAccumulation(prisonerId: String) {
+    LOG.info("Entered AllocationService - processPrisonerAccumulation with prisonerId: $prisonerId")
 
-    val allPrisoners = prisonerSearchClient.getConvictedPrisonersByPrisonId(prisonId)
-    val allIncentiveLevels = incentivesClient.getPrisonIncentiveLevels(prisonId)
+    visitOrderRepository.updateAvailableVisitOrdersOver28DaysToAccumulated(prisonerId, VisitOrderType.VO)
+  }
 
-    for (prisoner in allPrisoners) {
-      processPrisonerAllocation(prisoner.prisonerId, prisoner, allIncentiveLevels)
-    }
+  private fun processPrisonerExpiration(prisonerId: String) {
+    LOG.info("Entered AllocationService - processPrisonerExpiration with prisonerId: $prisonerId")
 
-    LOG.info("Finished AllocationService - processPrisonAllocation with prisonCode: $prisonId, total records processed : ${allPrisoners.size}")
+    val currentAccumulatedVoCount = visitOrderRepository.countAllVisitOrders(prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)
+    if (currentAccumulatedVoCount > maxAccumulatedVisitOrders) {
+      val amountToExpire = currentAccumulatedVoCount - maxAccumulatedVisitOrders
+      LOG.info("prisoner $prisonerId has $currentAccumulatedVoCount VOs. This is more than maximum allowed accumulated VOs $maxAccumulatedVisitOrders. Expiring $amountToExpire VOs")
+      visitOrderRepository.expireOldestAccumulatedVisitOrders(prisonerId, VisitOrderType.VO, amountToExpire)
+    }
   }
 
   private fun createVisitOrder(prisonerId: String, type: VisitOrderType): VisitOrder {

diff --git a/...ice/digital/hmpps/visitallocationapi/service/VisitAllocationByPrisonJobListenerService.kt b/...ice/digital/hmpps/visitallocationapi/service/VisitAllocationByPrisonJobListenerService.kt
@@ -14,6 +14,6 @@ class VisitAllocationByPrisonJobListenerService(
 
   suspend fun handleVisitAllocationJob(prisonCode: String) {
     log.info("received allocation job event: {}", prisonCode)
-    allocationService.processPrisonAllocation(prisonCode)
+    allocationService.processPrison(prisonCode)
   }
 }
diff --git a/...justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt b/...justice/digital/hmpps/visitallocationapi/integration/allocations/AllocationServiceTest.kt
@@ -1,14 +1,16 @@
 package uk.gov.justice.digital.hmpps.visitallocationapi.integration.allocations
 
 import kotlinx.coroutines.runBlocking
+import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
-import org.mockito.InjectMocks
 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
 import org.mockito.kotlin.argThat
+import org.mockito.kotlin.never
 import org.mockito.kotlin.whenever
 import uk.gov.justice.digital.hmpps.visitallocationapi.clients.IncentivesClient
 import uk.gov.justice.digital.hmpps.visitallocationapi.clients.PrisonerSearchClient
@@ -34,9 +36,13 @@ class AllocationServiceTest {
   @Mock
   private lateinit var visitOrderRepository: VisitOrderRepository
 
-  @InjectMocks
   private lateinit var allocationService: AllocationService
 
+  @BeforeEach
+  fun setUp() {
+    allocationService = AllocationService(prisonerSearchClient, incentivesClient, visitOrderRepository, 26)
+  }
+
   // --- Start Allocation Tests --- \\
 
   /**
@@ -98,7 +104,7 @@ class AllocationServiceTest {
 
     // Begin test
     runBlocking {
-      allocationService.processPrisonAllocation(prisonId)
+      allocationService.processPrison(prisonId)
     }
 
     // THEN - 3 Visit orders should be generated (2 VOs and 1 PVO).
@@ -136,7 +142,7 @@ class AllocationServiceTest {
 
     // Begin test
     runBlocking {
-      allocationService.processPrisonAllocation(prisonId)
+      allocationService.processPrison(prisonId)
     }
 
     // THEN - 2 Visit orders should be generated (2 VOs but no PVOs).
@@ -176,7 +182,7 @@ class AllocationServiceTest {
 
     // Begin test
     runBlocking {
-      allocationService.processPrisonAllocation(prisonId)
+      allocationService.processPrison(prisonId)
     }
 
     // THEN - 2 Visit orders should be generated (2 VOs but no PVOs).
@@ -203,8 +209,8 @@ class AllocationServiceTest {
     val prisonerId = "AA123456"
     val prisonId = "MDI"
     val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId)
-    val prisonerIncentive = PrisonerIncentivesDto(iepCode = "Enhanced")
-    val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 3, privilegedVisitOrders = 2, levelCode = "Enhanced")
+    val prisonerIncentive = PrisonerIncentivesDto(iepCode = "ENH")
+    val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 3, privilegedVisitOrders = 2, levelCode = "ENH")
 
     // WHEN
     whenever(prisonerSearchClient.getConvictedPrisonersByPrisonId(prisonId)).thenReturn(listOf(prisoner))
@@ -216,7 +222,7 @@ class AllocationServiceTest {
 
     // Begin test
     runBlocking {
-      allocationService.processPrisonAllocation(prisonId)
+      allocationService.processPrison(prisonId)
     }
 
     // THEN - No VO / PVOs are saved.
@@ -232,7 +238,7 @@ class AllocationServiceTest {
   }
 
   /**
-   * Scenario 5: Existing prisoner is given VO and PVO as it was last given within 14days & 28 days.
+   * Scenario 5: Existing prisoner is given VO and PVO as it was last given 14days & 28 days ago.
    */
   @Test
   fun `Continue Allocation - Given an existing prisoner has STD incentive level for MDI prison and is due VO and PVO`() {
@@ -253,7 +259,7 @@ class AllocationServiceTest {
 
     // Begin test
     runBlocking {
-      allocationService.processPrisonAllocation(prisonId)
+      allocationService.processPrison(prisonId)
     }
 
     // THEN - 3 Visit orders should be generated (2 VOs and 1 PVO).
@@ -271,4 +277,71 @@ class AllocationServiceTest {
       },
     )
   }
+
+  // --- Accumulation & Expiration Tests --- \\
+
+  /**
+   * Scenario 1: Existing prisoner with VOs older than 28 days, has VO status updated form Available to Accumulated. But no VOs are expired.
+   */
+  @Test
+  fun `Accumulation - Given an existing prisoner has existing VOs older than 28 days, they are moved to accumulated`() {
+    // GIVEN - A new prisoner with Standard incentive level, in prison MDI
+    val prisonerId = "AA123456"
+    val prisonId = "MDI"
+    val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId)
+    val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD")
+    val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD")
+
+    // WHEN
+    whenever(prisonerSearchClient.getConvictedPrisonersByPrisonId(prisonId)).thenReturn(listOf(prisoner))
+    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))
+
+    whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(4)
+
+    // Begin test
+    runBlocking {
+      allocationService.processPrison(prisonId)
+    }
+
+    // THEN - updateAvailableVisitOrdersOver28DaysToAccumulated is called but no interactions with expireOldestAccumulatedVisitOrders.
+    verify(visitOrderRepository).updateAvailableVisitOrdersOver28DaysToAccumulated(prisoner.prisonerId, VisitOrderType.VO)
+    verify(visitOrderRepository).countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)
+    verify(visitOrderRepository, never()).expireOldestAccumulatedVisitOrders(any(), any(), any())
+  }
+
+  /**
+   * Scenario 2: Existing prisoner with more than 26 VOs has their oldest VOs over 26 days expired.
+   */
+  @Test
+  fun `Expiration - Given an existing prisoner has more than 26 VOs, the extra VOs are moved to expired`() {
+    // GIVEN - A new prisoner with Standard incentive level, in prison MDI
+    val prisonerId = "AA123456"
+    val prisonId = "MDI"
+    val prisoner = PrisonerDto(prisonerId = prisonerId, prisonId = prisonId)
+    val prisonerIncentive = PrisonerIncentivesDto(iepCode = "STD")
+    val prisonIncentiveAmounts = PrisonIncentiveAmountsDto(visitOrders = 2, privilegedVisitOrders = 1, levelCode = "STD")
+
+    // WHEN
+    whenever(prisonerSearchClient.getConvictedPrisonersByPrisonId(prisonId)).thenReturn(listOf(prisoner))
+    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))
+
+    whenever(visitOrderRepository.countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)).thenReturn(28)
+
+    // Begin test
+    runBlocking {
+      allocationService.processPrison(prisonId)
+    }
+
+    // THEN - updateAvailableVisitOrdersOver28DaysToAccumulated is called but no interactions with expireOldestAccumulatedVisitOrders.
+    verify(visitOrderRepository).countAllVisitOrders(prisoner.prisonerId, VisitOrderType.VO, VisitOrderStatus.ACCUMULATED)
+    verify(visitOrderRepository).expireOldestAccumulatedVisitOrders(prisoner.prisonerId, VisitOrderType.VO, 2)
+  }
 }