From 1c289ac0c3b7993cfca961a9b446267b700a97c7 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 15 Sep 2015 22:38:48 -0700 Subject: Fix broken sort in merge request API Closes #2266 --- CHANGELOG | 1 + lib/api/merge_requests.rb | 2 +- spec/requests/api/merge_requests_spec.rb | 25 +++++++++++++------------ 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index eb4c59d6205..71238630d31 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.0.0 (unreleased) + - Fix broken sort in merge request API (Stan Hu) - Bump rouge to 1.10.1 to remove warning noise and fix other syntax highlighting bugs (Stan Hu) - Gracefully handle errors in syntax highlighting by leaving the block unformatted (Stan Hu) - Add "replace" and "upload" functionalities to allow user replace existing file and upload new file into current repository diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 7412274b045..63ea2f05438 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -55,7 +55,7 @@ module API else merge_requests end - merge_requests.reorder(issuable_order_by => issuable_sort) + merge_requests = merge_requests.reorder(issuable_order_by => issuable_sort) present paginate(merge_requests), with: Entities::MergeRequest end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 942768fa254..35b3d3e296a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -2,11 +2,12 @@ require "spec_helper" describe API::API, api: true do include ApiHelpers + let(:base_time) { Time.now } let(:user) { create(:user) } let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } - let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } - let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test") } - let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test") } + let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) } + let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.seconds) } + let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds) } let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") } @@ -74,8 +75,8 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - expect(json_response.last['id']).to eq(@mr_earlier.id) - expect(json_response.first['id']).to eq(@mr_later.id) + response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + expect(response_dates).to eq(response_dates.sort) end it "should return an array of merge_requests in descending order" do @@ -83,8 +84,8 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - expect(json_response.first['id']).to eq(@mr_later.id) - expect(json_response.last['id']).to eq(@mr_earlier.id) + response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + expect(response_dates).to eq(response_dates.sort.reverse) end it "should return an array of merge_requests ordered by updated_at" do @@ -92,17 +93,17 @@ describe API::API, api: true do expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - expect(json_response.last['id']).to eq(@mr_earlier.id) - expect(json_response.first['id']).to eq(@mr_later.id) + response_dates = json_response.map{ |merge_request| merge_request['updated_at'] } + expect(response_dates).to eq(response_dates.sort.reverse) end it "should return an array of merge_requests ordered by created_at" do - get api("/projects/#{project.id}/merge_requests?sort=created_at", user) + get api("/projects/#{project.id}/merge_requests?order_by=created_at&sort=asc", user) expect(response.status).to eq(200) expect(json_response).to be_an Array expect(json_response.length).to eq(3) - expect(json_response.last['id']).to eq(@mr_earlier.id) - expect(json_response.first['id']).to eq(@mr_later.id) + response_dates = json_response.map{ |merge_request| merge_request['created_at'] } + expect(response_dates).to eq(response_dates.sort) end end end -- cgit v1.2.1