Merge branch '54796-add-tiebreaker-to-sorts' into 'master'

Added: Include order by ID desc as tie break in pagination

Closes #54796

See merge request gitlab-org/gitlab-ce!25311

3 files changed, 162 insertions, 14 deletions

diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 04908378a24..033cd1b2b38 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -359,10 +359,38 @@ describe API::Issues do
         expect_paginated_array_response([])
       end
 
-      it 'sorts by created_at descending by default' do
-        get api('/issues', user)
+      context 'without sort params' do
+        it 'sorts by created_at descending by default' do
+          get api('/issues', user)
 
-        expect_paginated_array_response([issue.id, closed_issue.id])
+          expect_paginated_array_response([issue.id, closed_issue.id])
+        end
+
+        context 'with 2 issues with same created_at' do
+          let!(:closed_issue2) do
+            create :closed_issue,
+                   author: user,
+                   assignees: [user],
+                   project: project,
+                   milestone: milestone,
+                   created_at: closed_issue.created_at,
+                   updated_at: 1.hour.ago,
+                   title: issue_title,
+                   description: issue_description
+          end
+
+          it 'page breaks first page correctly' do
+            get api('/issues?per_page=2', user)
+
+            expect_paginated_array_response([issue.id, closed_issue2.id])
+          end
+
+          it 'page breaks second page correctly' do
+            get api('/issues?per_page=2&page=2', user)
+
+            expect_paginated_array_response([closed_issue.id])
+          end
+        end
       end
 
       it 'sorts ascending when requested' do
@@ -613,10 +641,38 @@ describe API::Issues do
         expect_paginated_array_response(group_confidential_issue.id)
       end
 
-      it 'sorts by created_at descending by default' do
-        get api(base_url, user)
+      context 'without sort params' do
+        it 'sorts by created_at descending by default' do
+          get api(base_url, user)
 
-        expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id, group_issue.id])
+          expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id, group_issue.id])
+        end
+
+        context 'with 2 issues with same created_at' do
+          let!(:group_issue2) do
+            create :issue,
+                   author: user,
+                   assignees: [user],
+                   project: group_project,
+                   milestone: group_milestone,
+                   updated_at: 1.hour.ago,
+                   title: issue_title,
+                   description: issue_description,
+                   created_at: group_issue.created_at
+          end
+
+          it 'page breaks first page correctly' do
+            get api("#{base_url}?per_page=3", user)
+
+            expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id, group_issue2.id])
+          end
+
+          it 'page breaks second page correctly' do
+            get api("#{base_url}?per_page=3&page=2", user)
+
+            expect_paginated_array_response([group_issue.id])
+          end
+        end
       end
 
       it 'sorts ascending when requested' do
@@ -828,10 +884,38 @@ describe API::Issues do
       expect_paginated_array_response([issue.id, closed_issue.id])
     end
 
-    it 'sorts by created_at descending by default' do
-      get api("#{base_url}/issues", user)
+    context 'without sort params' do
+      it 'sorts by created_at descending by default' do
+        get api("#{base_url}/issues", user)
 
-      expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue.id])
+        expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue.id])
+      end
+
+      context 'with 2 issues with same created_at' do
+        let!(:closed_issue2) do
+          create :closed_issue,
+                 author: user,
+                 assignees: [user],
+                 project: project,
+                 milestone: milestone,
+                 created_at: closed_issue.created_at,
+                 updated_at: 1.hour.ago,
+                 title: issue_title,
+                 description: issue_description
+        end
+
+        it 'page breaks first page correctly' do
+          get api("#{base_url}/issues?per_page=3", user)
+
+          expect_paginated_array_response([issue.id, confidential_issue.id, closed_issue2.id])
+        end
+
+        it 'page breaks second page correctly' do
+          get api("#{base_url}/issues?per_page=3&page=2", user)
+
+          expect_paginated_array_response([closed_issue.id])
+        end
+      end
     end
 
     it 'sorts ascending when requested' do
diff --git a/spec/support/shared_examples/requests/api/merge_requests_list.rb b/spec/support/shared_examples/requests/api/merge_requests_list.rb
index 453f42251c8..6713ec47ace 100644
--- a/spec/support/shared_examples/requests/api/merge_requests_list.rb
+++ b/spec/support/shared_examples/requests/api/merge_requests_list.rb
@@ -257,6 +257,38 @@ shared_examples 'merge requests list' do
         expect(response_dates).to eq(response_dates.sort.reverse)
       end
 
+      context '2 merge requests with equal created_at' do
+        let!(:closed_mr2) do
+          create :merge_request,
+                 state: 'closed',
+                 milestone: milestone1,
+                 author: user,
+                 assignee: user,
+                 source_project: project,
+                 target_project: project,
+                 title: "Test",
+                 created_at: @mr_earlier.created_at
+        end
+
+        it 'page breaks first page correctly' do
+          get api("#{endpoint_path}?sort=desc&per_page=4", user)
+
+          response_ids = json_response.map { |merge_request| merge_request['id'] }
+
+          expect(response_ids).to include(closed_mr2.id)
+          expect(response_ids).not_to include(@mr_earlier.id)
+        end
+
+        it 'page breaks second page correctly' do
+          get api("#{endpoint_path}?sort=desc&per_page=4&page=2", user)
+
+          response_ids = json_response.map { |merge_request| merge_request['id'] }
+
+          expect(response_ids).not_to include(closed_mr2.id)
+          expect(response_ids).to include(@mr_earlier.id)
+        end
+      end
+
       it 'returns an array of merge_requests ordered by updated_at' do
         path = endpoint_path + '?order_by=updated_at'
 
diff --git a/spec/support/shared_examples/requests/api/notes.rb b/spec/support/shared_examples/requests/api/notes.rb
index 71499e85654..57eefd5ef01 100644
--- a/spec/support/shared_examples/requests/api/notes.rb
+++ b/spec/support/shared_examples/requests/api/notes.rb
@@ -8,13 +8,45 @@ shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
         create_list(:note, 3, params)
       end
 
-      it 'sorts by created_at in descending order by default' do
-        get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user)
+      context 'without sort params' do
+        it 'sorts by created_at in descending order by default' do
+          get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user)
 
-        response_dates = json_response.map { |note| note['created_at'] }
+          response_dates = json_response.map { |note| note['created_at'] }
 
-        expect(json_response.length).to eq(4)
-        expect(response_dates).to eq(response_dates.sort.reverse)
+          expect(json_response.length).to eq(4)
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        context '2 notes with equal created_at' do
+          before do
+            @first_note = Note.first
+
+            params = { noteable: noteable, author: user }
+            params[:project] = parent if parent.is_a?(Project)
+            params[:created_at] = @first_note.created_at
+
+            @note2 = create(:note, params)
+          end
+
+          it 'page breaks first page correctly' do
+            get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?per_page=4", user)
+
+            response_ids = json_response.map { |note| note['id'] }
+
+            expect(response_ids).to include(@note2.id)
+            expect(response_ids).not_to include(@first_note.id)
+          end
+
+          it 'page breaks second page correctly' do
+            get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?per_page=4&page=2", user)
+
+            response_ids = json_response.map { |note| note['id'] }
+
+            expect(response_ids).not_to include(@note2.id)
+            expect(response_ids).to include(@first_note.id)
+          end
+        end
       end
 
       it 'sorts by ascending order when requested' do