Fix for VB-6479 - to add a sort order to paged results to ensure results are not repeated in subsequent paged results.

Author: dhirajshettymoj

src/main/kotlin/uk/gov/justice/digital/hmpps/personalrelationships/repository/ContactSearchRepository.kt

@@ -24,7 +24,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
     select c.contactId
     from ContactEntity c
     where c.dateOfBirth = :dateOfBirth
-    order by c.contactId
     """,
   )
   fun findAllByDateOfBirthEquals(dateOfBirth: LocalDate, pageable: Pageable): Page<Long>
@@ -37,7 +36,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
       and (:lastName is null or c.lastName ilike %:lastName% escape '#')
       and (:firstName is null or c.firstName ilike %:firstName% escape '#')
       and (:middleNames is null or c.middleNames ilike %:middleNames% escape '#')
-      order by c.contactId
     """,
   )
   fun findAllByDateOfBirthAndNamesMatch(dateOfBirth: LocalDate, firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -50,7 +48,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
        and (:lastName is null or c.lastName ilike :lastName)
        and (:firstName is null or c.firstName ilike :firstName)
        and (:middleNames is null or c.middleNames ilike :middleNames)
-       order by c.contactId
     """,
   )
   fun findAllByDateOfBirthAndNamesExact(dateOfBirth: LocalDate, firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -63,7 +60,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
        and ( :lastName is null or c.lastNameSoundex = CAST(function('soundex', CAST(:lastName AS string)) AS char(4)))
        and ( :firstName is null or c.firstNameSoundex = CAST(function('soundex', CAST(:firstName AS string)) AS char(4)))
        and (:middleNames is null or c.middleNamesSoundex = CAST(function('soundex', CAST(:middleNames AS string)) AS char(4)))
-       order by c.contactId
     """,
   )
   fun findAllByDateOfBirthAndNamesSoundLike(dateOfBirth: LocalDate, firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -75,7 +71,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
     where (:lastName is null or c.lastName ilike :lastName)
       and (:firstName is null or c.firstName ilike :firstName)
       and (:middleNames is null or c.middleNames ilike :middleNames)
-      order by c.contactId
     """,
   )
   fun findAllByNamesExact(firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -87,7 +82,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
     where (:lastName is null or c.lastName ilike %:lastName% escape '#')
       and (:firstName is null or c.firstName ilike %:firstName% escape '#')
       and (:middleNames is null or c.middleNames ilike %:middleNames% escape '#')
-      order by c.contactId
     """,
   )
   fun findAllByNamesMatch(firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -99,7 +93,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
     where (:lastName is null or c.lastNameSoundex = CAST(function('soundex', CAST(:lastName AS string)) AS char(4)))
       and (:firstName is null or c.firstNameSoundex = CAST(function('soundex', CAST(:firstName AS string)) AS char(4)))
       and (:middleNames is null or c.middleNamesSoundex = CAST(function('soundex', CAST(:middleNames AS string)) AS char(4)))
-    order by c.contactId
     """,
   )
   fun findAllByNamesSoundLike(firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -114,8 +107,7 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
         and (:firstName is null or ca.firstName ilike :firstName)
         and (:middleNames is null or ca.middleNames ilike :middleNames)
         and ca.revType in (0, 1)
-    )
-    order by c.contactId
+    ) 
     """,
   )
   fun findAllByNamesExactAndHistory(firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -134,7 +126,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
       select c.contact_id
       from contact c
       where c.contact_id in (select contact_id from filtered_contacts)
-      order by c.contact_id
     """,
     countQuery = """
       with filtered_contacts AS (
@@ -168,7 +159,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
   select c.contact_id
   from contact c
   where c.contact_id in (select contact_id from filtered_contacts)
-  order by c.contact_id
 """,
     countQuery = """
   with filtered_contacts AS (
@@ -201,7 +191,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
         and (:middleNames is null or ca.middleNames ilike :middleNames)
         and ca.revType in (0, 1)
     )
-    order by c.contactId
     """,
   )
   fun findAllByDateOfBirthAndNamesExactAndHistory(dateOfBirth: LocalDate, firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -218,8 +207,7 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
         and (:firstName is null or ca.firstName ilike %:firstName% escape '#')
         and (:middleNames is null or ca.middleNames ilike %:middleNames% escape '#')
         and ca.revType in (0, 1)
-    )
-        order by c.contactId
+    )    
     """,
   )
   fun findAllByDateOfBirthAndNamesMatchAndHistory(dateOfBirth: LocalDate, firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>
@@ -237,7 +225,6 @@ interface ContactSearchRepository : JpaRepository<ContactEntity, Long> {
         and (:middleNames is null or ca.middleNamesSoundex = CAST(function('soundex', CAST(:middleNames as string)) AS char(4)))
         and ca.revType in (0, 1)
     )
-    order by c.contactId
     """,
   )
   fun findAllByDateOfBirthAndNamesSoundLikeAndHistory(dateOfBirth: LocalDate, firstName: String?, middleNames: String?, lastName: String?, pageable: Pageable): Page<Long>

src/main/kotlin/uk/gov/justice/digital/hmpps/personalrelationships/service/ContactSearchService.kt

@@ -42,12 +42,20 @@ class ContactSearchService(
 ) {
   fun searchContacts(request: ContactSearchRequest, pageable: Pageable): Page<ContactSearchResultItem> {
     validateRequest(request)
-    val pageOfContactIds = getPageOfContactIds(request, pageable)
+
+    // force adding contactId sort to the pageable as without it same ID is encountered across multiple pages.
+    val pageableWithIdSort = PageRequest.of(
+      pageable.pageNumber,
+      pageable.pageSize,
+      pageable.sort.and(Sort.by("contactId").ascending()),
+    )
+
+    val pageOfContactIds = getPageOfContactIds(request, pageableWithIdSort)
 
     logger.info("PageOfContacts is (elementsInThisPage) ${pageOfContactIds.content.size} and (totalElements) ${pageOfContactIds.totalElements}")
 
     return if (!pageOfContactIds.isEmpty) {
-      enrichOnePage(pageOfContactIds, request, pageable, pageOfContactIds.totalElements)
+      enrichOnePage(pageOfContactIds, request, pageableWithIdSort, pageOfContactIds.totalElements)
     } else {
       Page.empty()
     }

src/test/kotlin/uk/gov/justice/digital/hmpps/personalrelationships/integration/resource/SearchContactsPaginationIntegrationTest.kt

@@ -9,6 +9,7 @@ import org.springframework.web.util.UriComponentsBuilder
 import uk.gov.justice.digital.hmpps.personalrelationships.integration.SecureAPIIntegrationTestBase
 import uk.gov.justice.digital.hmpps.personalrelationships.util.StubUser
 import java.net.URI
+import java.time.LocalDate
 import kotlin.collections.toList
 
 class SearchContactsPaginationIntegrationTest : SecureAPIIntegrationTestBase() {
@@ -25,22 +26,56 @@ class SearchContactsPaginationIntegrationTest : SecureAPIIntegrationTestBase() {
     .accept(MediaType.APPLICATION_JSON)
 
   @Test
-  fun `when contacts search is done then proper search results are returned irrespective of sort order`() {
+  fun `when contacts search with results sorted by last name ascending then all results are returned in correct order`() {
     // test to check fix for VB-6479
     // 50 records have been added with lastName starting with ABCD and firstName as Test to replicate live scenario
     // starting from contact ID 2001 to 2050
     // the asserts check that all Ids starting from 2001 to 2050 are being returned which was not the case before the fix
-    var sortValues = listOf("lastName,asc")
+    val sortValues = listOf("lastName,asc")
+
+    // assert that all Ids from 2001 t0 2050 are there
     assertPagedData(sortValues)
+    val contactLastNames = getLastNames(sortValues)
+    val sortedContactLastNames = contactLastNames.sorted()
+    for (i in 0 until sortedContactLastNames.size) {
+      assertThat(contactLastNames[i]).isEqualTo(sortedContactLastNames[i])
+    }
+  }
 
-    sortValues = listOf("lastName,desc")
+  @Test
+  fun `when contacts search with results sorted by last name descending then all results are returned in correct order`() {
+    val sortValues = listOf("lastName,desc")
     assertPagedData(sortValues)
+    val contactLastNamesDesc = getLastNames(sortValues)
+    val sortedContactLastNamesDesc = contactLastNamesDesc.sortedDescending()
+
+    for (i in 0 until sortedContactLastNamesDesc.size) {
+      assertThat(contactLastNamesDesc[i]).isEqualTo(sortedContactLastNamesDesc[i])
+    }
+  }
 
-    sortValues = listOf("dateOfBirth,asc")
+  @Test
+  fun `when contacts search with results sorted by dateOfBirth ascending then all results are returned in correct order`() {
+    val sortValues = listOf("dateOfBirth,asc")
     assertPagedData(sortValues)
+    val contactDOBs = getDateOfBirths(sortValues)
+    val sortedDOBs = contactDOBs.sortedWith(nullsLast(naturalOrder()))
+
+    for (i in 0 until sortedDOBs.size) {
+      assertThat(contactDOBs[i]).isEqualTo(sortedDOBs[i])
+    }
+  }
 
-    sortValues = listOf("dateOfBirth,desc")
+  @Test
+  fun `when contacts search with results sorted by dateOfBirth descending then all results are returned in correct order`() {
+    val sortValues = listOf("dateOfBirth,desc")
     assertPagedData(sortValues)
+    val contactDOBsDesc = getDateOfBirths(sortValues)
+    val sortedDOBsDesc = contactDOBsDesc.sortedWith(nullsFirst(reverseOrder()))
+
+    for (i in 0 until contactDOBsDesc.size) {
+      assertThat(contactDOBsDesc[i]).isEqualTo(sortedDOBsDesc[i])
+    }
   }
 
   private fun getContactSearchUrl(pageNumber: Int? = 0, sortValues: List<String>): URI {
@@ -49,7 +84,7 @@ class SearchContactsPaginationIntegrationTest : SecureAPIIntegrationTestBase() {
       .queryParam("lastName", "ABCD")
       .queryParam("firstName", "Test")
       .queryParam("page", pageNumber)
-      .queryParam("sort", sortValues.joinToString(","))
+      .queryParam("sort", sortValues)
       .build()
       .toUri()
 
@@ -87,6 +122,32 @@ class SearchContactsPaginationIntegrationTest : SecureAPIIntegrationTestBase() {
     assertThat(contactIds.max()).isEqualTo(2050)
   }
 
+  private fun getLastNames(sortValues: List<String>): List<String> {
+    val contactLastNames = mutableListOf<String>()
+
+    // iterate over the 5 pages and collate all results into a list
+    for (pageNumber in 0..4) {
+      val uri = getContactSearchUrl(pageNumber, sortValues)
+      val body = testAPIClient.getSearchContactResults(uri)
+      contactLastNames.addAll(body!!.content.map { it.lastName })
+    }
+
+    return contactLastNames
+  }
+
+  private fun getDateOfBirths(sortValues: List<String>): List<LocalDate?> {
+    val contactDateOfBirths = mutableListOf<LocalDate?>()
+
+    // iterate over the 5 pages and collate all results into a list
+    for (pageNumber in 0..4) {
+      val uri = getContactSearchUrl(pageNumber, sortValues)
+      val body = testAPIClient.getSearchContactResults(uri)
+      contactDateOfBirths.addAll(body!!.content.map { it.dateOfBirth })
+    }
+
+    return contactDateOfBirths
+  }
+
   companion object {
     private val CONTACT_SEARCH_URL = UriComponentsBuilder.fromPath("contact/search")
       .queryParam("searchType", "PARTIAL")

src/test/kotlin/uk/gov/justice/digital/hmpps/personalrelationships/service/ContactSearchServiceTest.kt

@@ -141,6 +141,7 @@ class ContactSearchServiceTest {
         Sort.Order(Sort.Direction.ASC, "last_name"),
         Sort.Order(Sort.Direction.ASC, "first_name"),
         Sort.Order(Sort.Direction.ASC, "date_of_birth"),
+        Sort.Order(Sort.Direction.ASC, "contact_id"),
       )
     }