From 2eb0beb661cf1a97e3f36ae2785bb1ad44afd5d0 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Dec 2016 14:31:42 -0600 Subject: [PATCH 1/6] add natural sorting token for build names --- app/models/commit_status.rb | 6 ++++++ app/views/projects/stage/_graph.html.haml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 31cd381dcd2..897e53fc7bd 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -137,4 +137,10 @@ class CommitStatus < ActiveRecord::Base .new(self, current_user) .fabricate! end + + def natsort_name + name.split(/(\d+)/).map do |v| + v =~ /\d+/ ? v.to_i : v + end + end end diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index d9d392fa02f..faadcfee30c 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -1,6 +1,6 @@ - stage = local_assigns.fetch(:stage) - statuses = stage.statuses.latest -- status_groups = statuses.sort_by(&:name).group_by(&:group_name) +- status_groups = statuses.sort_by(&:natsort_name).group_by(&:group_name) %li.stage-column .stage-name %a{ name: stage.name } From 19c7fc75a8aae79214fec0d71e5bdd54a58f5964 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Dec 2016 14:31:46 -0600 Subject: [PATCH 2/6] add tests for natural sorting of build names --- .../projects/pipelines/show.html.haml_spec.rb | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/spec/views/projects/pipelines/show.html.haml_spec.rb b/spec/views/projects/pipelines/show.html.haml_spec.rb index a066ea078e6..afda286c089 100644 --- a/spec/views/projects/pipelines/show.html.haml_spec.rb +++ b/spec/views/projects/pipelines/show.html.haml_spec.rb @@ -45,6 +45,41 @@ describe 'projects/pipelines/show' do expect(rendered).to have_text('jenkins') end + it 'lists builds in the correct sort order' do + create_build('test', 1, 'karma 0 20', :created) + create_build('test', 1, 'karma 12 20', :created) + create_build('test', 1, 'karma 1 20', :created) + create_build('test', 1, 'karma 10 20', :created) + create_build('test', 1, 'karma 11 20', :created) + create_build('test', 1, 'karma 2 20', :created) + create_build('test', 1, 'test 1.10', :created) + create_build('test', 1, 'test 1.5.1', :created) + create_build('test', 1, 'test 1 a', :created) + + render + + # spaced builds order + expected_order_1 = [ + 'karma 0 20', + 'karma 1 20', + 'karma 2 20', + 'karma 10 20', + 'karma 11 20', + 'karma 12 20' + ].join(' ') + + expect(rendered).to have_text(expected_order_1) + + # decimal builds order + expected_order_2 = [ + 'test 1 a', + 'test 1.5.1', + 'test 1.10' + ].join(' ') + + expect(rendered).to have_text(expected_order_2) + end + private def create_build(stage, stage_idx, name, status) From 713e8cf2eb5f5b46a355f9d76a49076c3b459d1c Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 22 Dec 2016 14:39:39 -0600 Subject: [PATCH 3/6] add CHANGELOG.md entry for !8277 --- changelogs/unreleased/fix-build-sort-order.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/fix-build-sort-order.yml diff --git a/changelogs/unreleased/fix-build-sort-order.yml b/changelogs/unreleased/fix-build-sort-order.yml new file mode 100644 index 00000000000..a6d6371f69a --- /dev/null +++ b/changelogs/unreleased/fix-build-sort-order.yml @@ -0,0 +1,4 @@ +--- +title: Sort numbers in build names more intelligently +merge_request: 8277 +author: From 1c5a506588342bdb2a6390f14d333d41a5482f58 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Tue, 27 Dec 2016 09:59:42 -0600 Subject: [PATCH 4/6] rename sort method --- app/models/commit_status.rb | 2 +- app/views/projects/stage/_graph.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 897e53fc7bd..9547c57b2ae 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -138,7 +138,7 @@ class CommitStatus < ActiveRecord::Base .fabricate! end - def natsort_name + def sortable_name name.split(/(\d+)/).map do |v| v =~ /\d+/ ? v.to_i : v end diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index faadcfee30c..4ee30b023ac 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -1,6 +1,6 @@ - stage = local_assigns.fetch(:stage) - statuses = stage.statuses.latest -- status_groups = statuses.sort_by(&:natsort_name).group_by(&:group_name) +- status_groups = statuses.sort_by(&:sortable_name).group_by(&:group_name) %li.stage-column .stage-name %a{ name: stage.name } From dc3d40ff28e4087120d48a94db5bd3d3b32f164d Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Mon, 2 Jan 2017 11:42:08 -0600 Subject: [PATCH 5/6] prefer unit test on model over view test --- spec/models/commit_status_spec.rb | 22 ++++++++++++ .../projects/pipelines/show.html.haml_spec.rb | 35 ------------------- 2 files changed, 22 insertions(+), 35 deletions(-) diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 701f3323c0f..daabc804d16 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -243,4 +243,26 @@ describe CommitStatus, models: true do .to be_a Gitlab::Ci::Status::Success end end + + describe '#sortable_name' do + subject { commit_status.sortable_name } + + tests = { + 'karma' => ['karma'], + 'karma 0 20' => ['karma ', 0, ' ', 20], + 'karma 10 20' => ['karma ', 10, ' ', 20], + 'karma 50:100' => ['karma ', 50, ':', 100], + 'karma 1.10' => ['karma ', 1, '.', 10], + 'karma 1.5.1' => ['karma ', 1, '.', 5, '.', 1], + 'karma 1 a' => ['karma ', 1, ' a'] + } + + tests.each do |name, sortable_name| + it "'#{name}' sorts as '#{sortable_name}'" do + commit_status.name = name + + is_expected.to eq(sortable_name) + end + end + end end diff --git a/spec/views/projects/pipelines/show.html.haml_spec.rb b/spec/views/projects/pipelines/show.html.haml_spec.rb index afda286c089..a066ea078e6 100644 --- a/spec/views/projects/pipelines/show.html.haml_spec.rb +++ b/spec/views/projects/pipelines/show.html.haml_spec.rb @@ -45,41 +45,6 @@ describe 'projects/pipelines/show' do expect(rendered).to have_text('jenkins') end - it 'lists builds in the correct sort order' do - create_build('test', 1, 'karma 0 20', :created) - create_build('test', 1, 'karma 12 20', :created) - create_build('test', 1, 'karma 1 20', :created) - create_build('test', 1, 'karma 10 20', :created) - create_build('test', 1, 'karma 11 20', :created) - create_build('test', 1, 'karma 2 20', :created) - create_build('test', 1, 'test 1.10', :created) - create_build('test', 1, 'test 1.5.1', :created) - create_build('test', 1, 'test 1 a', :created) - - render - - # spaced builds order - expected_order_1 = [ - 'karma 0 20', - 'karma 1 20', - 'karma 2 20', - 'karma 10 20', - 'karma 11 20', - 'karma 12 20' - ].join(' ') - - expect(rendered).to have_text(expected_order_1) - - # decimal builds order - expected_order_2 = [ - 'test 1 a', - 'test 1.5.1', - 'test 1.10' - ].join(' ') - - expect(rendered).to have_text(expected_order_2) - end - private def create_build(stage, stage_idx, name, status) From 0bc48797c8559a1f9c042b74e2faadc820fc7257 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 13 Jan 2017 14:54:25 -0500 Subject: [PATCH 6/6] revise sortable_name test formatting --- spec/models/commit_status_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index daabc804d16..64ea607eb95 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -245,8 +245,6 @@ describe CommitStatus, models: true do end describe '#sortable_name' do - subject { commit_status.sortable_name } - tests = { 'karma' => ['karma'], 'karma 0 20' => ['karma ', 0, ' ', 20], @@ -260,8 +258,7 @@ describe CommitStatus, models: true do tests.each do |name, sortable_name| it "'#{name}' sorts as '#{sortable_name}'" do commit_status.name = name - - is_expected.to eq(sortable_name) + expect(commit_status.sortable_name).to eq(sortable_name) end end end