[SQSERVICES-1828] pagination of team members does not work properly for certain team sizes

changelog.d/3-bug-fixes/pr-2968

@@ -0,0 +1 @@
+Fix pagination in team user search (make search key unique)

services/brig/src/Brig/User/Search/TeamUserSearch.hs

@@ -102,12 +102,24 @@ teamUserSearchQuery tid mbSearchText _mRoleFilter mSortBy mSortOrder =
         mbQStr
     )
     teamFilter
-    ( maybe
+    -- in combination with pagination a non-unique search specification can lead to missing results
+    -- therefore we use the unique `_doc` value as a tie breaker
+    -- - see https://www.elastic.co/guide/en/elasticsearch/reference/6.8/search-request-sort.html for details on `_doc`
+    -- - see https://www.elastic.co/guide/en/elasticsearch/reference/6.8/search-request-search-after.html for details on pagination and tie breaker
+    -- in the latter article it "is advised to duplicate (client side or [...]) the content of the _id field
+    -- in another field that has doc value enabled and to use this new field as the tiebreaker for the sort"
+    -- so alternatively we could use the user ID as a tie breaker, but this would require a change in the index mapping
+    (sorting ++ sortingTieBreaker)
+  where
+    sorting :: [ES.DefaultSort]
+    sorting =
+      maybe
         [defaultSort SortByCreatedAt SortOrderDesc | isNothing mbQStr]
         (\tuSortBy -> [defaultSort tuSortBy (fromMaybe SortOrderAsc mSortOrder)])
         mSortBy
-    )
-  where
+    sortingTieBreaker :: [ES.DefaultSort]
+    sortingTieBreaker = [ES.DefaultSort (ES.FieldName "_doc") ES.Ascending Nothing Nothing Nothing Nothing]
+
     mbQStr :: Maybe Text
     mbQStr =
       case mbSearchText of

services/brig/test/integration/API/TeamUserSearch.hs

@@ -111,7 +111,7 @@ testSort brig = do
   let sortByProperty' :: (TestConstraints m, Ord a) => TeamUserSearchSortBy -> (User -> a) -> TeamUserSearchSortOrder -> m ()
       sortByProperty' = sortByProperty tid users ownerId
   for_ [SortOrderAsc, SortOrderDesc] $ \sortOrder -> do
-    -- FUTUREWORK: Test SortByRole when role is avaible in index
+    -- FUTUREWORK: Test SortByRole when role is available in index
     sortByProperty' SortByEmail userEmail sortOrder
     sortByProperty' SortByName userDisplayName sortOrder
     sortByProperty' SortByHandle (fmap fromHandle . userHandle) sortOrder
@@ -144,12 +144,17 @@ testEmptyQuerySortedWithPagination :: TestConstraints m => Brig -> m ()
 testEmptyQuerySortedWithPagination brig = do
   (tid, userId -> ownerId, _) <- createPopulatedBindingTeamWithNamesAndHandles brig 20
   refreshIndex brig
-  searchResultFirst10 <- executeTeamUserSearchWithMaybeState brig tid ownerId (Just "") Nothing Nothing Nothing (Just $ unsafeRange 10) Nothing
-  searchResultLast11 <- executeTeamUserSearchWithMaybeState brig tid ownerId (Just "") Nothing Nothing Nothing Nothing (searchPagingState searchResultFirst10)
+  let teamUserSearch mPs = executeTeamUserSearchWithMaybeState brig tid ownerId (Just "") Nothing (Just SortByRole) (Just SortOrderAsc) (Just $ unsafeRange 10) mPs
+  searchResultFirst10 <- teamUserSearch Nothing
+  searchResultNext10 <- teamUserSearch (searchPagingState searchResultFirst10)
+  searchResultLast1 <- teamUserSearch (searchPagingState searchResultNext10)
   liftIO $ do
     searchReturned searchResultFirst10 @?= 10
     searchFound searchResultFirst10 @?= 21
     searchHasMore searchResultFirst10 @?= Just True
-    searchReturned searchResultLast11 @?= 11
-    searchFound searchResultLast11 @?= 21
-    searchHasMore searchResultLast11 @?= Just False
+    searchReturned searchResultNext10 @?= 10
+    searchFound searchResultNext10 @?= 21
+    searchHasMore searchResultNext10 @?= Just True
+    searchReturned searchResultLast1 @?= 1
+    searchFound searchResultLast1 @?= 21
+    searchHasMore searchResultLast1 @?= Just False