diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | app/models/concerns/sortable.rb | 11 | ||||
| -rw-r--r-- | app/models/todo.rb | 2 | ||||
| -rw-r--r-- | spec/features/todos/todos_sorting_spec.rb | 126 | ||||
| -rw-r--r-- | spec/models/concerns/issuable_spec.rb | 14 |
5 files changed, 102 insertions, 52 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9151a27e100..6af5d716be6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Please view this file on the master branch, on stable branches it's out of date. ## 8.13.2 - Fix builds dropdown overlapping bug !7124 + - Fix and improve `Sortable.highest_label_priority` ## 8.13.1 (2016-10-25) - Fix branch protection API. !6215 diff --git a/app/models/concerns/sortable.rb b/app/models/concerns/sortable.rb index 12b23f00769..7edb0acd56c 100644 --- a/app/models/concerns/sortable.rb +++ b/app/models/concerns/sortable.rb @@ -38,16 +38,21 @@ module Sortable private - def highest_label_priority(target_type:, target_column:, project_column:, excluded_labels: []) + def highest_label_priority(target_type_column: nil, target_type: nil, target_column:, project_column:, excluded_labels: []) query = Label.select(LabelPriority.arel_table[:priority].minimum). left_join_priorities. joins(:label_links). where("label_priorities.project_id = #{project_column}"). - where(label_links: { target_type: target_type }). where("label_links.target_id = #{target_column}"). reorder(nil) - query.where.not(title: excluded_labels) if excluded_labels.present? + if target_type_column + query = query.where("label_links.target_type = #{target_type_column}") + else + query = query.where(label_links: { target_type: target_type }) + end + + query = query.where.not(title: excluded_labels) if excluded_labels.present? query end diff --git a/app/models/todo.rb b/app/models/todo.rb index 11c072dd000..f5ade1cc293 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -53,7 +53,7 @@ class Todo < ActiveRecord::Base # Need to order by created_at last because of differences on Mysql and Postgres when joining by type "Merge_request/Issue" def order_by_labels_priority params = { - target_type: ['Issue', 'MergeRequest'], + target_type_column: "todos.target_type", target_column: "todos.target_id", project_column: "todos.project_id" } diff --git a/spec/features/todos/todos_sorting_spec.rb b/spec/features/todos/todos_sorting_spec.rb index e74a51acede..fec28c55d30 100644 --- a/spec/features/todos/todos_sorting_spec.rb +++ b/spec/features/todos/todos_sorting_spec.rb @@ -8,60 +8,90 @@ describe "Dashboard > User sorts todos", feature: true do let(:label_2) { create(:label, title: 'label_2', project: project, priority: 2) } let(:label_3) { create(:label, title: 'label_3', project: project, priority: 3) } - let(:issue_1) { create(:issue, title: 'issue_1', project: project) } - let(:issue_2) { create(:issue, title: 'issue_2', project: project) } - let(:issue_3) { create(:issue, title: 'issue_3', project: project) } - let(:issue_4) { create(:issue, title: 'issue_4', project: project) } - - let!(:merge_request_1) { create(:merge_request, source_project: project, title: "merge_request_1") } - - before do - create(:todo, user: user, project: project, target: issue_4, created_at: 5.hours.ago) - create(:todo, user: user, project: project, target: issue_2, created_at: 4.hours.ago) - create(:todo, user: user, project: project, target: issue_3, created_at: 3.hours.ago) - create(:todo, user: user, project: project, target: issue_1, created_at: 2.hours.ago) - create(:todo, user: user, project: project, target: merge_request_1, created_at: 1.hour.ago) - - merge_request_1.labels << label_1 - issue_3.labels << label_1 - issue_2.labels << label_3 - issue_1.labels << label_2 - - project.team << [user, :developer] - login_as(user) - visit dashboard_todos_path - end + before { project.team << [user, :developer] } - it "sorts with oldest created todos first" do - click_link "Last created" + context 'sort options' do + let(:issue_1) { create(:issue, title: 'issue_1', project: project) } + let(:issue_2) { create(:issue, title: 'issue_2', project: project) } + let(:issue_3) { create(:issue, title: 'issue_3', project: project) } + let(:issue_4) { create(:issue, title: 'issue_4', project: project) } - results_list = page.find('.todos-list') - expect(results_list.all('p')[0]).to have_content("merge_request_1") - expect(results_list.all('p')[1]).to have_content("issue_1") - expect(results_list.all('p')[2]).to have_content("issue_3") - expect(results_list.all('p')[3]).to have_content("issue_2") - expect(results_list.all('p')[4]).to have_content("issue_4") - end + let!(:merge_request_1) { create(:merge_request, source_project: project, title: "merge_request_1") } + + before do + create(:todo, user: user, project: project, target: issue_4, created_at: 5.hours.ago) + create(:todo, user: user, project: project, target: issue_2, created_at: 4.hours.ago) + create(:todo, user: user, project: project, target: issue_3, created_at: 3.hours.ago) + create(:todo, user: user, project: project, target: issue_1, created_at: 2.hours.ago) + create(:todo, user: user, project: project, target: merge_request_1, created_at: 1.hour.ago) + + merge_request_1.labels << label_1 + issue_3.labels << label_1 + issue_2.labels << label_3 + issue_1.labels << label_2 + + login_as(user) + visit dashboard_todos_path + end + + it "sorts with oldest created todos first" do + click_link "Last created" + + results_list = page.find('.todos-list') + expect(results_list.all('p')[0]).to have_content("merge_request_1") + expect(results_list.all('p')[1]).to have_content("issue_1") + expect(results_list.all('p')[2]).to have_content("issue_3") + expect(results_list.all('p')[3]).to have_content("issue_2") + expect(results_list.all('p')[4]).to have_content("issue_4") + end - it "sorts with newest created todos first" do - click_link "Oldest created" + it "sorts with newest created todos first" do + click_link "Oldest created" - results_list = page.find('.todos-list') - expect(results_list.all('p')[0]).to have_content("issue_4") - expect(results_list.all('p')[1]).to have_content("issue_2") - expect(results_list.all('p')[2]).to have_content("issue_3") - expect(results_list.all('p')[3]).to have_content("issue_1") - expect(results_list.all('p')[4]).to have_content("merge_request_1") + results_list = page.find('.todos-list') + expect(results_list.all('p')[0]).to have_content("issue_4") + expect(results_list.all('p')[1]).to have_content("issue_2") + expect(results_list.all('p')[2]).to have_content("issue_3") + expect(results_list.all('p')[3]).to have_content("issue_1") + expect(results_list.all('p')[4]).to have_content("merge_request_1") + end + + it "sorts by priority" do + click_link "Priority" + + results_list = page.find('.todos-list') + expect(results_list.all('p')[0]).to have_content("issue_3") + expect(results_list.all('p')[1]).to have_content("merge_request_1") + expect(results_list.all('p')[2]).to have_content("issue_1") + expect(results_list.all('p')[3]).to have_content("issue_2") + expect(results_list.all('p')[4]).to have_content("issue_4") + end end - it "sorts by priority" do - click_link "Priority" + context 'issues and merge requests' do + let(:issue_1) { create(:issue, id: 10000, title: 'issue_1', project: project) } + let(:issue_2) { create(:issue, id: 10001, title: 'issue_2', project: project) } + let(:merge_request_1) { create(:merge_request, id: 10000, title: 'merge_request_1', source_project: project) } + + before do + issue_1.labels << label_1 + issue_2.labels << label_2 + + create(:todo, user: user, project: project, target: issue_1) + create(:todo, user: user, project: project, target: issue_2) + create(:todo, user: user, project: project, target: merge_request_1) + + login_as(user) + visit dashboard_todos_path + end + + it "doesn't mix issues and merge requests priorities" do + click_link "Priority" - results_list = page.find('.todos-list') - expect(results_list.all('p')[0]).to have_content("issue_3") - expect(results_list.all('p')[1]).to have_content("merge_request_1") - expect(results_list.all('p')[2]).to have_content("issue_1") - expect(results_list.all('p')[3]).to have_content("issue_2") - expect(results_list.all('p')[4]).to have_content("issue_4") + results_list = page.find('.todos-list') + expect(results_list.all('p')[0]).to have_content("issue_1") + expect(results_list.all('p')[1]).to have_content("issue_2") + expect(results_list.all('p')[2]).to have_content("merge_request_1") + end end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 60e4bbc8564..a59d30687f6 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -298,6 +298,20 @@ describe Issue, "Issuable" do end end + describe '.order_labels_priority' do + let(:label_1) { create(:label, title: 'label_1', project: issue.project, priority: 1) } + let(:label_2) { create(:label, title: 'label_2', project: issue.project, priority: 2) } + + subject { Issue.order_labels_priority(excluded_labels: ['label_1']).first.highest_priority } + + before do + issue.labels << label_1 + issue.labels << label_2 + end + + it { is_expected.to eq(2) } + end + describe ".with_label" do let(:project) { create(:project, :public) } let(:bug) { create(:label, project: project, title: 'bug') } |