PRC-507: Fixes a bug where all addresses included the phone numbers for the other addresses in this organisation

Author: timharrison-moj

src/main/kotlin/uk/gov/justice/digital/hmpps/organisationsapi/mapping/OrganisationAddressDetailsMappers.kt

@@ -7,7 +7,7 @@ import uk.gov.justice.digital.hmpps.organisationsapi.model.response.Organisation
 import uk.gov.justice.digital.hmpps.organisationsapi.model.response.OrganisationAddressPhoneDetails
 
 fun OrganisationAddressDetailsEntity.toModel(
-  phoneNumbers: List<Pair<OrganisationAddressPhoneEntity, OrganisationPhoneDetailsEntity>>,
+  phoneNumbers: List<Pair<OrganisationAddressPhoneEntity, OrganisationPhoneDetailsEntity?>>,
 ): OrganisationAddressDetails = OrganisationAddressDetails(
   organisationAddressId = organisationAddressId,
   organisationId = organisationId,
@@ -35,21 +35,24 @@ fun OrganisationAddressDetailsEntity.toModel(
   specialNeedsCodeDescription = specialNeedsCodeDescription,
   contactPersonName = contactPersonName,
   businessHours = businessHours,
-  phoneNumbers = phoneNumbers.map { (addressPhoneEntity, phoneEntity) ->
-    OrganisationAddressPhoneDetails(
-      organisationAddressPhoneId = addressPhoneEntity.organisationAddressPhoneId,
-      organisationPhoneId = phoneEntity.organisationPhoneId,
-      organisationAddressId = organisationAddressId,
-      organisationId = phoneEntity.organisationId,
-      phoneType = phoneEntity.phoneType,
-      phoneTypeDescription = phoneEntity.phoneTypeDescription,
-      phoneNumber = phoneEntity.phoneNumber,
-      extNumber = phoneEntity.extNumber,
-      createdBy = phoneEntity.createdBy,
-      createdTime = phoneEntity.createdTime,
-      updatedBy = phoneEntity.updatedBy,
-      updatedTime = phoneEntity.updatedTime,
-    )
+  phoneNumbers = phoneNumbers.mapNotNull { (addressPhoneEntity, phoneEntity) ->
+    when {
+      phoneEntity != null -> OrganisationAddressPhoneDetails(
+        organisationAddressPhoneId = addressPhoneEntity.organisationAddressPhoneId,
+        organisationPhoneId = phoneEntity.organisationPhoneId,
+        organisationAddressId = organisationAddressId,
+        organisationId = phoneEntity.organisationId,
+        phoneType = phoneEntity.phoneType,
+        phoneTypeDescription = phoneEntity.phoneTypeDescription,
+        phoneNumber = phoneEntity.phoneNumber,
+        extNumber = phoneEntity.extNumber,
+        createdBy = phoneEntity.createdBy,
+        createdTime = phoneEntity.createdTime,
+        updatedBy = phoneEntity.updatedBy,
+        updatedTime = phoneEntity.updatedTime,
+      )
+      else -> null
+    }
   },
   createdBy = createdBy,
   createdTime = createdTime,

src/main/kotlin/uk/gov/justice/digital/hmpps/organisationsapi/service/OrganisationService.kt

@@ -40,10 +40,13 @@ class OrganisationService(
   fun getOrganisationById(id: Long): OrganisationDetails {
     val organisationEntity = organisationRepository.findById(id)
       .orElseThrow { EntityNotFoundException("Organisation with id $id not found") }
+
+    // Split the address phones from the global phones
     val phoneNumbers = organisationPhoneDetailsRepository.findByOrganisationId(id)
     val organisationAddressPhones = organisationAddressPhoneRepository.findByOrganisationId(id)
     val organisationAddressPhoneIds = organisationAddressPhones.map { it.organisationPhoneId }
     val (organisationAddressPhoneNumbers, organisationPhoneNumbers) = phoneNumbers.partition { it.organisationPhoneId in organisationAddressPhoneIds }
+
     return OrganisationDetails(
       organisationId = organisationEntity.organisationId!!,
       organisationName = organisationEntity.organisationName,
@@ -76,7 +79,15 @@ class OrganisationService(
     organisationAddressPhoneNumbers: List<OrganisationPhoneDetailsEntity>,
     organisationAddressPhones: List<OrganisationAddressPhoneEntity>,
   ): List<OrganisationAddressDetails> = organisationAddressRepository.findByOrganisationId(id)
-    .map { address -> address.toModel(organisationAddressPhones.map { addressPhoneEntity -> addressPhoneEntity to organisationAddressPhoneNumbers.find { it.organisationPhoneId == addressPhoneEntity.organisationPhoneId }!! }) }
+    .map { address ->
+      address.toModel(
+        organisationAddressPhones.map { addressPhoneEntity ->
+          addressPhoneEntity to organisationAddressPhoneNumbers.find {
+            (it.organisationPhoneId == addressPhoneEntity.organisationPhoneId && addressPhoneEntity.organisationAddressId == address.organisationAddressId)
+          }
+        },
+      )
+    }
 
   fun search(request: OrganisationSearchRequest, pageable: Pageable): Page<OrganisationSummary> = organisationSearchRepository.search(request, pageable).toModel()
 

src/test/kotlin/uk/gov/justice/digital/hmpps/organisationsapi/integration/resource/GetOrganisationByOrganisationIdIntegrationTest.kt

@@ -101,6 +101,19 @@ class GetOrganisationByOrganisationIdIntegrationTest : SecureApiIntegrationTestB
             ).setCreatedAndModified(),
           ),
         ).setCreatedAndModified(),
+        MigrateOrganisationAddress(
+          nomisAddressId = RandomUtils.secure().randomLong(),
+          type = "BUS",
+          primaryAddress = false,
+          mailAddress = false,
+          serviceAddress = false,
+          noFixedAddress = false,
+          locality = "locality",
+          postCode = "D2 2DN",
+          startDate = LocalDate.of(2020, 3, 3),
+          endDate = LocalDate.of(2021, 4, 4),
+          phoneNumbers = emptyList(),
+        ).setCreatedAndModified(),
       ),
     ).setCreatedAndModified()
 
@@ -158,7 +171,7 @@ class GetOrganisationByOrganisationIdIntegrationTest : SecureApiIntegrationTestB
         assertThat(updatedBy).isEqualTo("MODIFIED")
         assertThat(updatedTime).isEqualTo(LocalDateTime.of(2020, 3, 4, 11, 45))
       }
-      assertThat(addresses).hasSize(1)
+      assertThat(addresses).hasSize(2)
       with(addresses[0]) {
         assertThat(addressType).isEqualTo("BUS")
         assertThat(addressTypeDescription).isEqualTo("Business address")
@@ -200,6 +213,13 @@ class GetOrganisationByOrganisationIdIntegrationTest : SecureApiIntegrationTestB
           assertThat(updatedTime).isEqualTo(LocalDateTime.of(2020, 3, 4, 11, 45))
         }
       }
+      with(addresses[1]) {
+        // Making sure phone numbers are propagated across ALL addresses for this organisation
+        assertThat(phoneNumbers).isEmpty()
+        assertThat(postcode).isEqualTo("D2 2DN")
+        assertThat(startDate).isEqualTo(LocalDate.of(2020, 3, 3))
+        assertThat(endDate).isEqualTo(LocalDate.of(2021, 4, 4))
+      }
     }
   }