RO bug fix (#2492)

* RO bug fix * RO bug fix * remove show sql
ministryofjustice · Nov 3, 2023 · 26e4bc7 · 26e4bc7
1 parent 30de378
commit 26e4bc7
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 25 deletions.
diff --git a/...integrationTest/kotlin/uk/gov/justice/digital/hmpps/AllocationMessagingIntegrationTest.kt b/...integrationTest/kotlin/uk/gov/justice/digital/hmpps/AllocationMessagingIntegrationTest.kt
@@ -19,12 +19,12 @@ import org.springframework.beans.factory.annotation.Value
 import org.springframework.boot.test.context.SpringBootTest
 import org.springframework.boot.test.mock.mockito.MockBean
 import org.springframework.boot.test.mock.mockito.SpyBean
+import org.springframework.data.repository.findByIdOrNull
 import uk.gov.justice.digital.hmpps.data.generator.PersonGenerator
 import uk.gov.justice.digital.hmpps.data.generator.ProviderGenerator
 import uk.gov.justice.digital.hmpps.integrations.delius.contact.entity.ContactRepository
 import uk.gov.justice.digital.hmpps.integrations.delius.contact.entity.ContactType
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.PrisonManagerRepository
-import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.ResponsibleOfficer
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.Staff
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.StaffRepository
 import uk.gov.justice.digital.hmpps.messaging.HmppsChannelManager
@@ -117,8 +117,7 @@ internal class AllocationMessagingIntegrationTest {
         // add RO to existing pom to test RO behaviour
         val existingPom =
             prisonManagerRepository.findActiveManagerAtDate(PersonGenerator.DEFAULT.id, ZonedDateTime.now())!!
-        existingPom.responsibleOfficer =
-            ResponsibleOfficer(existingPom.personId, existingPom, existingPom.date)
+        existingPom.makeResponsibleOfficer()
         prisonManagerRepository.save(existingPom)
 
         val notification = prepNotification(
@@ -138,12 +137,13 @@ internal class AllocationMessagingIntegrationTest {
         assertThat(prisonManager?.allocationReason?.code, equalTo("INA"))
         assertThat(prisonManager?.staff?.forename, equalTo("James"))
         assertThat(prisonManager?.staff?.surname, equalTo("Brown"))
-        assertNotNull(prisonManager?.responsibleOfficer)
-        assertNull(prisonManager?.responsibleOfficer?.endDate)
+        assertNotNull(prisonManager?.responsibleOfficer())
+        assertNull(prisonManager?.responsibleOfficer()?.endDate)
 
-        val previousPom = prisonManagerRepository.findById(existingPom.id).getOrNull()
+        val previousPom = prisonManagerRepository.findByIdOrNull(existingPom.id)
         assertNotNull(previousPom?.endDate)
-        assertNotNull(previousPom?.responsibleOfficer?.endDate)
+        previousPom?.responsibleOfficers?.forEach { assertNotNull(it.endDate) }
+        assertNull(previousPom?.responsibleOfficer())
 
         val contacts = contactRepository.findAll().filter { it.personId == PersonGenerator.DEFAULT.id }
         assertThat(
@@ -167,7 +167,8 @@ internal class AllocationMessagingIntegrationTest {
     @Order(4)
     @Test
     fun `deallocate POM successfully`() {
-        val existingPom = prisonManagerRepository.findActiveManagerAtDate(PersonGenerator.DEFAULT.id, ZonedDateTime.now())!!
+        val existingPom =
+            prisonManagerRepository.findActiveManagerAtDate(PersonGenerator.DEFAULT.id, ZonedDateTime.now())!!
 
         val notification = prepNotification(
             notification("deallocation"),
@@ -178,15 +179,17 @@ internal class AllocationMessagingIntegrationTest {
 
         verify(staffRepository, never()).save(any())
 
-        val prisonManager = prisonManagerRepository.findActiveManagerAtDate(PersonGenerator.DEFAULT.id, ZonedDateTime.now())
+        val prisonManager =
+            prisonManagerRepository.findActiveManagerAtDate(PersonGenerator.DEFAULT.id, ZonedDateTime.now())
         assertThat(prisonManager?.allocationReason?.code, equalTo("AUT"))
         assertThat(prisonManager?.staff?.code, equalTo(ProviderGenerator.UNALLOCATED_STAFF.code))
         assertThat(prisonManager?.staff?.forename, equalTo(ProviderGenerator.UNALLOCATED_STAFF.forename))
         assertThat(prisonManager?.staff?.surname, equalTo(ProviderGenerator.UNALLOCATED_STAFF.surname))
 
         val previousPom = prisonManagerRepository.findById(existingPom.id).getOrNull()
         assertNotNull(previousPom?.endDate)
-        assertNotNull(previousPom?.responsibleOfficer?.endDate)
+        previousPom?.responsibleOfficers?.forEach { assertNotNull(it.endDate) }
+        assertNull(previousPom?.responsibleOfficer())
 
         val contacts = contactRepository.findAll().filter { it.personId == PersonGenerator.DEFAULT.id }
         assertThat(

diff --git a/.../kotlin/uk/gov/justice/digital/hmpps/integrations/delius/provider/entity/PrisonManager.kt b/.../kotlin/uk/gov/justice/digital/hmpps/integrations/delius/provider/entity/PrisonManager.kt
@@ -4,16 +4,16 @@ import jakarta.persistence.CascadeType
 import jakarta.persistence.Column
 import jakarta.persistence.Entity
 import jakarta.persistence.EntityListeners
+import jakarta.persistence.FetchType
 import jakarta.persistence.GeneratedValue
 import jakarta.persistence.GenerationType
 import jakarta.persistence.Id
 import jakarta.persistence.JoinColumn
 import jakarta.persistence.ManyToOne
-import jakarta.persistence.OneToOne
+import jakarta.persistence.OneToMany
 import jakarta.persistence.SequenceGenerator
 import jakarta.persistence.Table
 import jakarta.persistence.Version
-import org.hibernate.annotations.Where
 import org.springframework.data.annotation.CreatedBy
 import org.springframework.data.annotation.CreatedDate
 import org.springframework.data.annotation.LastModifiedBy
@@ -67,6 +67,9 @@ class PrisonManager(
     @JoinColumn(name = "probation_area_id", nullable = false)
     override val probationArea: ProbationArea,
 
+    @OneToMany(fetch = FetchType.EAGER, mappedBy = "prisonManager", cascade = [CascadeType.PERSIST, CascadeType.MERGE])
+    val responsibleOfficers: MutableList<ResponsibleOfficer> = mutableListOf(),
+
     @Column(columnDefinition = "number", nullable = false)
     val softDeleted: Boolean = false
 ) : Manager {
@@ -75,15 +78,12 @@ class PrisonManager(
         set(value) {
             field = value
             active = value == null
+            responsibleOfficer()?.endDate = value
         }
 
     @Column(name = "active_flag", columnDefinition = "number", nullable = false)
     var active: Boolean = true
 
-    @OneToOne(mappedBy = "prisonManager", cascade = [CascadeType.PERSIST, CascadeType.MERGE])
-    @Where(clause = "end_date is null")
-    var responsibleOfficer: ResponsibleOfficer? = null
-
     @CreatedBy
     @Column(nullable = false, updatable = false)
     var createdByUserId: Long = 0
@@ -102,6 +102,12 @@ class PrisonManager(
 
     fun isUnallocated() = staff.code.endsWith("U")
 
+    fun responsibleOfficer(): ResponsibleOfficer? = responsibleOfficers.firstOrNull { it.endDate == null }
+
+    fun makeResponsibleOfficer() {
+        responsibleOfficers.add(ResponsibleOfficer(personId, this, date))
+    }
+
     enum class AllocationReasonCode(val value: String, val ctc: ContactType.Code) {
         AUTO("AUT", ContactType.Code.POM_AUTO_ALLOCATION),
         INTERNAL("INA", ContactType.Code.POM_INTERNAL_ALLOCATION),
@@ -122,7 +128,7 @@ class ResponsibleOfficer(
     @Column(name = "offender_id")
     val personId: Long,
 
-    @OneToOne
+    @ManyToOne
     @JoinColumn(name = "PRISON_OFFENDER_MANAGER_ID")
     var prisonManager: PrisonManager?,
 
@@ -157,12 +163,11 @@ interface PrisonManagerRepository : JpaRepository<PrisonManager, Long> {
     @Query(
         """
             select pm from PrisonManager pm
-            left join fetch pm.responsibleOfficer ro
+            left join fetch pm.responsibleOfficers ro
             where pm.personId = :personId
             and pm.softDeleted = false
             and pm.date <= :date
             and (pm.endDate is null or pm.endDate > :date)
-            and (ro is null or ro.endDate is null) 
         """
     )
     fun findActiveManagerAtDate(personId: Long, date: ZonedDateTime): PrisonManager?

diff --git a/...-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/services/PrisonManagerService.kt b/...-and-delius/src/main/kotlin/uk/gov/justice/digital/hmpps/services/PrisonManagerService.kt
@@ -11,7 +11,6 @@ import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.PrisonMa
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.PrisonManagerRepository
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.ProbationArea
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.ProbationAreaRepository
-import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.ResponsibleOfficer
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.Staff
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.Team
 import uk.gov.justice.digital.hmpps.integrations.delius.provider.entity.TeamRepository
@@ -123,15 +122,14 @@ class PrisonManagerService(
             allocationReason = referenceDataRepository.pomAllocationReason(allocationReasonCode.value),
             date = dateTime,
             staff = staff,
-            team = team
+            team = team,
+            responsibleOfficers = mutableListOf()
         )
         val notes = newPom.allocationNotes() + (this?.transferNotes() ?: "")
         contactService.createContact(personId, allocationReasonCode.ctc, dateTime, newPom, notes)
         this?.apply {
-            endDate = dateTime
-            responsibleOfficer?.also {
-                it.endDate = dateTime
-                newPom.responsibleOfficer = ResponsibleOfficer(personId, newPom, dateTime)
+            responsibleOfficer()?.also {
+                newPom.makeResponsibleOfficer()
                 contactService.createContact(
                     personId,
                     ContactType.Code.RESPONSIBLE_OFFICER_CHANGE,
@@ -146,6 +144,7 @@ class PrisonManagerService(
                     """.trimMargin()
                 )
             }
+            endDate = dateTime
         }
         newPom
     } else {