From 55f322085d0507640366b7a774fe7819771ff54b Mon Sep 17 00:00:00 2001 From: Jacopo Date: Sat, 18 Nov 2017 15:06:55 +0100 Subject: Adds ordering to projects contributors in API Allows ordering in GET api/v4/projects/:project_id/repository/contributors through `order_by` and `sort` params. The available `order_by` options are: name|email|commits. The available `sort` options are: asc|desc. --- app/models/commit.rb | 14 +++ app/models/repository.rb | 9 +- .../unreleased/13695-order-contributors-in-api.yml | 5 + doc/api/repositories.md | 2 + lib/api/repositories.rb | 4 +- spec/factories/commits.rb | 19 +++- spec/fixtures/api/schemas/contributor.json | 18 ++++ spec/fixtures/api/schemas/contributors.json | 4 + spec/models/repository_spec.rb | 107 +++++++++++++++++++++ spec/requests/api/repositories_spec.rb | 22 +++++ 10 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/13695-order-contributors-in-api.yml create mode 100644 spec/fixtures/api/schemas/contributor.json create mode 100644 spec/fixtures/api/schemas/contributors.json diff --git a/app/models/commit.rb b/app/models/commit.rb index 307e4fcedfe..13c31111134 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -52,6 +52,20 @@ class Commit diffs.reduce(0) { |sum, d| sum + Gitlab::Git::Util.count_lines(d.diff) } end + def order_by(collection:, order_by:, sort:) + return collection unless %w[email name commits].include?(order_by) + return collection unless %w[asc desc].include?(sort) + + collection.sort do |a, b| + operands = [a, b].tap { |o| o.reverse! if sort == 'desc' } + + attr1, attr2 = operands.first.public_send(order_by), operands.second.public_send(order_by) # rubocop:disable PublicSend + + # use case insensitive comparison for string values + order_by.in?(%w[email name]) ? attr1.casecmp(attr2) : attr1 <=> attr2 + end + end + # Truncate sha to 8 characters def truncate_sha(sha) sha[0..MIN_SHA_LENGTH] diff --git a/app/models/repository.rb b/app/models/repository.rb index d1ae3304e4a..c01188315fd 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -701,10 +701,14 @@ class Repository end end - def contributors + # Params: + # + # order_by: name|email|commits + # sort: asc|desc default: 'asc' + def contributors(order_by: nil, sort: 'asc') commits = self.commits(nil, limit: 2000, offset: 0, skip_merges: true) - commits.group_by(&:author_email).map do |email, commits| + commits = commits.group_by(&:author_email).map do |email, commits| contributor = Gitlab::Contributor.new contributor.email = email @@ -718,6 +722,7 @@ class Repository contributor end + Commit.order_by(collection: commits, order_by: order_by, sort: sort) end def refs_contains_sha(ref_type, sha) diff --git a/changelogs/unreleased/13695-order-contributors-in-api.yml b/changelogs/unreleased/13695-order-contributors-in-api.yml new file mode 100644 index 00000000000..26bf8650a4a --- /dev/null +++ b/changelogs/unreleased/13695-order-contributors-in-api.yml @@ -0,0 +1,5 @@ +--- +title: Adds ordering to projects contributors in API +merge_request: 15469 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/doc/api/repositories.md b/doc/api/repositories.md index 594babc74be..03b32577872 100644 --- a/doc/api/repositories.md +++ b/doc/api/repositories.md @@ -182,6 +182,8 @@ GET /projects/:id/repository/contributors Parameters: - `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `order_by` (optional) - Return contributors ordered by `name`, `email`, or `commits` fields. If not given contributors are ordered by commit date. +- `sort` (optional) - Return contributors sorted in `asc` or `desc` order. Default is `asc` Response: diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 7887b886c03..4f36bbd760f 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -110,10 +110,12 @@ module API end params do use :pagination + optional :order_by, type: String, values: %w[email name commits], default: nil, desc: 'Return contributors ordered by `name` or `email` or `commits`' + optional :sort, type: String, values: %w[asc desc], default: nil, desc: 'Sort by asc (ascending) or desc (descending)' end get ':id/repository/contributors' do begin - contributors = ::Kaminari.paginate_array(user_project.repository.contributors) + contributors = ::Kaminari.paginate_array(user_project.repository.contributors(order_by: params[:order_by], sort: params[:sort])) present paginate(contributors), with: Entities::Contributor rescue not_found! diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb index f4f12a095fc..4e2d8e8969e 100644 --- a/spec/factories/commits.rb +++ b/spec/factories/commits.rb @@ -2,15 +2,28 @@ require_relative '../support/repo_helpers' FactoryGirl.define do factory :commit do - git_commit RepoHelpers.sample_commit + transient do + author nil + end + + git_commit do + commit = RepoHelpers.sample_commit + + if author + commit.author_email = author.email + commit.author_name = author.name + end + + commit + end project initialize_with do new(git_commit, project) end - after(:build) do |commit| - allow(commit).to receive(:author).and_return build(:author) + after(:build) do |commit, evaluator| + allow(commit).to receive(:author).and_return(evaluator.author || build(:author)) end trait :without_author do diff --git a/spec/fixtures/api/schemas/contributor.json b/spec/fixtures/api/schemas/contributor.json new file mode 100644 index 00000000000..e88470a2363 --- /dev/null +++ b/spec/fixtures/api/schemas/contributor.json @@ -0,0 +1,18 @@ +{ + "type": "object", + "required" : [ + "name", + "email", + "commits", + "additions", + "deletions" + ], + "properties" : { + "name": { "type": "string" }, + "email": { "type": "string" }, + "commits": { "type": "integer" }, + "additions": { "type": "integer" }, + "deletions": { "type": "integer" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/contributors.json b/spec/fixtures/api/schemas/contributors.json new file mode 100644 index 00000000000..a9f1d1ea64f --- /dev/null +++ b/spec/fixtures/api/schemas/contributors.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "contributor.json" } +} diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 358bc3dfb94..9ec9e447a8f 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2343,4 +2343,111 @@ describe Repository do end end end + + describe '#contributors' do + let(:author_a) { build(:author, email: 'tiagonbotelho@hotmail.com', name: 'tiagonbotelho') } + let(:author_b) { build(:author, email: 'gitlab@winniehell.de', name: 'Winnie') } + let(:author_c) { build(:author, email: 'douwe@gitlab.com', name: 'Douwe Maan') } + let(:stubbed_commits) do + [build(:commit, author: author_a), + build(:commit, author: author_a), + build(:commit, author: author_b), + build(:commit, author: author_c), + build(:commit, author: author_c), + build(:commit, author: author_c)] + end + let(:order_by) { nil } + let(:sort) { nil } + + before do + allow(repository).to receive(:commits).with(nil, limit: 2000, offset: 0, skip_merges: true).and_return(stubbed_commits) + end + + subject { repository.contributors(order_by: order_by, sort: sort) } + + def expect_contributors(*contributors) + expect(subject.map(&:email)).to eq(contributors.map(&:email)) + end + + it 'returns the array of Gitlab::Contributor for the repository' do + expect_contributors(author_a, author_b, author_c) + end + + context 'order_by email' do + let(:order_by) { 'email' } + + context 'asc' do + let(:sort) { 'asc' } + + it 'returns all the contributors ordered by email asc case insensitive' do + expect_contributors(author_c, author_b, author_a) + end + end + + context 'desc' do + let(:sort) { 'desc' } + + it 'returns all the contributors ordered by email desc case insensitive' do + expect_contributors(author_a, author_b, author_c) + end + end + end + + context 'order_by name' do + let(:order_by) { 'name' } + + context 'asc' do + let(:sort) { 'asc' } + + it 'returns all the contributors ordered by name asc case insensitive' do + expect_contributors(author_c, author_a, author_b) + end + end + + context 'desc' do + let(:sort) { 'desc' } + + it 'returns all the contributors ordered by name desc case insensitive' do + expect_contributors(author_b, author_a, author_c) + end + end + end + + context 'order_by commits' do + let(:order_by) { 'commits' } + + context 'asc' do + let(:sort) { 'asc' } + + it 'returns all the contributors ordered by commits asc' do + expect_contributors(author_b, author_a, author_c) + end + end + + context 'desc' do + let(:sort) { 'desc' } + + it 'returns all the contributors ordered by commits desc' do + expect_contributors(author_c, author_a, author_b) + end + end + end + + context 'invalid ordering' do + let(:order_by) { 'unknown' } + + it 'returns the contributors unsorted' do + expect_contributors(author_a, author_b, author_c) + end + end + + context 'invalid sorting' do + let(:order_by) { 'name' } + let(:sort) { 'unknown' } + + it 'returns the contributors unsorted' do + expect_contributors(author_a, author_b, author_c) + end + end + end end diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 9f2ff3b5af6..741800ff61d 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -378,6 +378,28 @@ describe API::Repositories do expect(first_contributor['additions']).to eq(0) expect(first_contributor['deletions']).to eq(0) end + + context 'using sorting' do + context 'by commits desc' do + it 'returns the repository contribuors sorted by commits desc' do + get api(route, current_user), { order_by: 'commits', sort: 'desc' } + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('contributors') + expect(json_response.first['commits']).to be > json_response.last['commits'] + end + end + + context 'by name desc' do + it 'returns the repository contribuors sorted by name asc case insensitive' do + get api(route, current_user), { order_by: 'name', sort: 'asc' } + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('contributors') + expect(json_response.first['name'].downcase).to be < json_response.last['name'].downcase + end + end + end end context 'when unauthenticated', 'and project is public' do -- cgit v1.2.1