From a78eeefd6e76c956751a2a7f03efeaee28f83b46 Mon Sep 17 00:00:00 2001 From: Jeff Stubler Date: Tue, 25 Oct 2016 19:03:42 -0500 Subject: [PATCH] Change issues sentence to use natural sorting --- app/helpers/merge_requests_helper.rb | 9 +-- .../20378-natural-sort-issue-numbers.yml | 4 ++ lib/gitlab/issuable_sorter.rb | 29 +++++++++ spec/helpers/merge_requests_helper_spec.rb | 37 +++++++++-- spec/lib/gitlab/issuable_sorter_spec.rb | 62 +++++++++++++++++++ 5 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 changelogs/unreleased/20378-natural-sort-issue-numbers.yml create mode 100644 lib/gitlab/issuable_sorter.rb create mode 100644 spec/lib/gitlab/issuable_sorter_spec.rb diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 38be073c8dc..e347f61fb8d 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -56,11 +56,12 @@ module MergeRequestsHelper end def issues_sentence(issues) - # Sorting based on the `#123` or `group/project#123` reference will sort - # local issues first. - issues.map do |issue| + # Issuable sorter will sort local issues, then issues from the same + # namespace, then all other issues. + issues = Gitlab::IssuableSorter.sort(@project, issues).map do |issue| issue.to_reference(@project) - end.sort.to_sentence + end + issues.to_sentence end def mr_closes_issues diff --git a/changelogs/unreleased/20378-natural-sort-issue-numbers.yml b/changelogs/unreleased/20378-natural-sort-issue-numbers.yml new file mode 100644 index 00000000000..2ebc8485ddf --- /dev/null +++ b/changelogs/unreleased/20378-natural-sort-issue-numbers.yml @@ -0,0 +1,4 @@ +--- +title: Change issues list in MR to natural sorting +merge_request: 7110 +author: Jeff Stubler diff --git a/lib/gitlab/issuable_sorter.rb b/lib/gitlab/issuable_sorter.rb new file mode 100644 index 00000000000..d392214867a --- /dev/null +++ b/lib/gitlab/issuable_sorter.rb @@ -0,0 +1,29 @@ +module Gitlab + module IssuableSorter + class << self + def sort(project, issuables, &sort_key) + grouped_items = issuables.group_by do |issuable| + if issuable.project.id == project.id + :project_ref + elsif issuable.project.namespace.id == project.namespace.id + :namespace_ref + else + :full_ref + end + end + + natural_sort_issuables(grouped_items[:project_ref], project) + + natural_sort_issuables(grouped_items[:namespace_ref], project) + + natural_sort_issuables(grouped_items[:full_ref], project) + end + + private + + def natural_sort_issuables(issuables, project) + VersionSorter.sort(issuables || []) do |issuable| + issuable.to_reference(project) + end + end + end + end +end diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb index 25f23826648..e9037749ef2 100644 --- a/spec/helpers/merge_requests_helper_spec.rb +++ b/spec/helpers/merge_requests_helper_spec.rb @@ -22,24 +22,51 @@ describe MergeRequestsHelper do end describe '#issues_sentence' do + let(:project) { create :project } + subject { issues_sentence(issues) } let(:issues) do - [build(:issue, iid: 1), build(:issue, iid: 2), build(:issue, iid: 3)] + [build(:issue, iid: 2, project: project), + build(:issue, iid: 3, project: project), + build(:issue, iid: 1, project: project)] end - it { is_expected.to eq('#1, #2, and #3') } + it do + @project = project + + is_expected.to eq('#1, #2, and #3') + end context 'for JIRA issues' do let(:project) { create(:empty_project) } let(:issues) do [ - ExternalIssue.new('JIRA-123', project), ExternalIssue.new('JIRA-456', project), - ExternalIssue.new('FOOBAR-7890', project) + ExternalIssue.new('FOOBAR-7890', project), + ExternalIssue.new('JIRA-123', project) ] end - it { is_expected.to eq('FOOBAR-7890, JIRA-123, and JIRA-456') } + it do + @project = project + is_expected.to eq('FOOBAR-7890, JIRA-123, and JIRA-456') + end + end + + context 'for issues from multiple namespaces' do + let(:project) { create(:project) } + let(:other_project) { create(:project) } + let(:issues) do + [build(:issue, iid: 2, project: project), + build(:issue, iid: 3, project: other_project), + build(:issue, iid: 1, project: project)] + end + + it do + @project = project + + is_expected.to eq("#1, #2, and #{other_project.namespace.path}/#{other_project.path}#3") + end end end diff --git a/spec/lib/gitlab/issuable_sorter_spec.rb b/spec/lib/gitlab/issuable_sorter_spec.rb new file mode 100644 index 00000000000..c9a434b2bcf --- /dev/null +++ b/spec/lib/gitlab/issuable_sorter_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Gitlab::IssuableSorter, lib: true do + let(:namespace1) { build(:namespace, id: 1) } + let(:project1) { build(:project, id: 1, namespace: namespace1) } + + let(:project2) { build(:project, id: 2, path: "a", namespace: project1.namespace) } + let(:project3) { build(:project, id: 3, path: "b", namespace: project1.namespace) } + + let(:namespace2) { build(:namespace, id: 2, path: "a") } + let(:namespace3) { build(:namespace, id: 3, path: "b") } + let(:project4) { build(:project, id: 4, path: "a", namespace: namespace2) } + let(:project5) { build(:project, id: 5, path: "b", namespace: namespace2) } + let(:project6) { build(:project, id: 6, path: "a", namespace: namespace3) } + + let(:unsorted) { [sorted[2], sorted[3], sorted[0], sorted[1]] } + + let(:sorted) do + [build(:issue, iid: 1, project: project1), + build(:issue, iid: 2, project: project1), + build(:issue, iid: 10, project: project1), + build(:issue, iid: 20, project: project1)] + end + + it 'sorts references by a given key' do + expect(described_class.sort(project1, unsorted)).to eq(sorted) + end + + context 'for JIRA issues' do + let(:sorted) do + [ExternalIssue.new('JIRA-1', project1), + ExternalIssue.new('JIRA-2', project1), + ExternalIssue.new('JIRA-10', project1), + ExternalIssue.new('JIRA-20', project1)] + end + + it 'sorts references by a given key' do + expect(described_class.sort(project1, unsorted)).to eq(sorted) + end + end + + context 'for references from multiple projects and namespaces' do + let(:sorted) do + [build(:issue, iid: 1, project: project1), + build(:issue, iid: 2, project: project1), + build(:issue, iid: 10, project: project1), + build(:issue, iid: 1, project: project2), + build(:issue, iid: 1, project: project3), + build(:issue, iid: 1, project: project4), + build(:issue, iid: 1, project: project5), + build(:issue, iid: 1, project: project6)] + end + let(:unsorted) do + [sorted[3], sorted[1], sorted[4], sorted[2], + sorted[6], sorted[5], sorted[0], sorted[7]] + end + + it 'sorts references by project and then by a given key' do + expect(subject.sort(project1, unsorted)).to eq(sorted) + end + end +end