diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-07-10 19:07:34 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-07-10 19:07:34 +0300 |
commit | 664bc371f309092de2915585af69a78efe69b27d (patch) | |
tree | 2366f479c047d901ecdca7541530ad0c75d20408 /spec | |
parent | b347749ab886194f34eaab7f6578bfd3d4b4415b (diff) | |
parent | 42f10974baca66621a15f0b529e4257854f37684 (diff) |
Merge branch 'sh-optimize-mr-api-emojis-and-labels' into 'master'
Remove remaining N+1 queries in merge requests API with emojis and labels
Closes #34159
See merge request !12732
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/issuable_metadata_spec.rb | 59 | ||||
-rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 25 |
2 files changed, 81 insertions, 3 deletions
diff --git a/spec/lib/gitlab/issuable_metadata_spec.rb b/spec/lib/gitlab/issuable_metadata_spec.rb new file mode 100644 index 00000000000..f9f4b290dbf --- /dev/null +++ b/spec/lib/gitlab/issuable_metadata_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Gitlab::IssuableMetadata, lib: true do + let(:user) { create(:user) } + let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace) } + + subject { Class.new { include Gitlab::IssuableMetadata }.new } + + it 'returns an empty Hash if an empty collection is provided' do + expect(subject.issuable_meta_data(Issue.none, 'Issue')).to eq({}) + end + + context 'issues' do + let!(:issue) { create(:issue, author: user, project: project) } + let!(:closed_issue) { create(:issue, state: :closed, author: user, project: project) } + let!(:downvote) { create(:award_emoji, :downvote, awardable: closed_issue) } + let!(:upvote) { create(:award_emoji, :upvote, awardable: issue) } + let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test") } + let!(:closing_issues) { create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) } + + it 'aggregates stats on issues' do + data = subject.issuable_meta_data(Issue.all, 'Issue') + + expect(data.count).to eq(2) + expect(data[issue.id].upvotes).to eq(1) + expect(data[issue.id].downvotes).to eq(0) + expect(data[issue.id].notes_count).to eq(0) + expect(data[issue.id].merge_requests_count).to eq(1) + + expect(data[closed_issue.id].upvotes).to eq(0) + expect(data[closed_issue.id].downvotes).to eq(1) + expect(data[closed_issue.id].notes_count).to eq(0) + expect(data[closed_issue.id].merge_requests_count).to eq(0) + end + end + + context 'merge requests' do + 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", source_project: project, target_project: project, title: "Closed Test") } + let!(:downvote) { create(:award_emoji, :downvote, awardable: merge_request) } + let!(:upvote) { create(:award_emoji, :upvote, awardable: merge_request) } + let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } + + it 'aggregates stats on merge requests' do + data = subject.issuable_meta_data(MergeRequest.all, 'MergeRequest') + + expect(data.count).to eq(2) + expect(data[merge_request.id].upvotes).to eq(1) + expect(data[merge_request.id].downvotes).to eq(1) + expect(data[merge_request.id].notes_count).to eq(1) + expect(data[merge_request.id].merge_requests_count).to eq(0) + + expect(data[merge_request_closed.id].upvotes).to eq(0) + expect(data[merge_request_closed.id].downvotes).to eq(0) + expect(data[merge_request_closed.id].notes_count).to eq(0) + expect(data[merge_request_closed.id].merge_requests_count).to eq(0) + end + end +end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4d0bd67c571..360a82196a8 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -16,7 +16,11 @@ describe API::MergeRequests do let!(:label) do create(:label, title: 'label', color: '#FFAABB', project: project) end + let!(:label2) { create(:label, title: 'a-test', color: '#FFFFFF', project: project) } let!(:label_link) { create(:label_link, label: label, target: merge_request) } + let!(:label_link2) { create(:label_link, label: label2, target: merge_request) } + let!(:downvote) { create(:award_emoji, :downvote, awardable: merge_request) } + let!(:upvote) { create(:award_emoji, :upvote, awardable: merge_request) } before do project.team << [user, :reporter] @@ -32,6 +36,18 @@ describe API::MergeRequests do end context "when authenticated" do + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}/merge_requests", user) + end.count + + create(:merge_request, state: 'closed', milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) + + expect do + get api("/projects/#{project.id}/merge_requests", user) + end.not_to exceed_query_limit(control_count) + end + it "returns an array of all merge_requests" do get api("/projects/#{project.id}/merge_requests", user) @@ -44,6 +60,9 @@ describe API::MergeRequests do expect(json_response.last['sha']).to eq(merge_request.diff_head_sha) expect(json_response.last['merge_commit_sha']).to be_nil expect(json_response.last['merge_commit_sha']).to eq(merge_request.merge_commit_sha) + expect(json_response.last['downvotes']).to eq(1) + expect(json_response.last['upvotes']).to eq(1) + expect(json_response.last['labels']).to eq([label2.title, label.title]) expect(json_response.first['title']).to eq(merge_request_merged.title) expect(json_response.first['sha']).to eq(merge_request_merged.diff_head_sha) expect(json_response.first['merge_commit_sha']).not_to be_nil @@ -145,7 +164,7 @@ describe API::MergeRequests do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.length).to eq(1) - expect(json_response.first['labels']).to eq([label.title]) + expect(json_response.first['labels']).to eq([label2.title, label.title]) end it 'returns an array of labeled merge requests where all labels match' do @@ -236,8 +255,8 @@ describe API::MergeRequests do expect(json_response['author']).to be_a Hash expect(json_response['target_branch']).to eq(merge_request.target_branch) expect(json_response['source_branch']).to eq(merge_request.source_branch) - expect(json_response['upvotes']).to eq(0) - expect(json_response['downvotes']).to eq(0) + expect(json_response['upvotes']).to eq(1) + expect(json_response['downvotes']).to eq(1) expect(json_response['source_project_id']).to eq(merge_request.source_project.id) expect(json_response['target_project_id']).to eq(merge_request.target_project.id) expect(json_response['work_in_progress']).to be_falsy |