Commit d6bf691d authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '23928-sortable-highest_label_priority-is-bugged' into 'master'

Fix and improve `Sortable.highest_label_priority`

Closes #23928

See merge request !7165
parents 5c5f34dd 7bd6ff03
...@@ -36,6 +36,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -36,6 +36,7 @@ Please view this file on the master branch, on stable branches it's out of date.
## 8.13.2 ## 8.13.2
- Fix builds dropdown overlapping bug !7124 - Fix builds dropdown overlapping bug !7124
- Fix and improve `Sortable.highest_label_priority`
## 8.13.1 (2016-10-25) ## 8.13.1 (2016-10-25)
- Fix branch protection API. !6215 - Fix branch protection API. !6215
......
...@@ -38,16 +38,21 @@ module Sortable ...@@ -38,16 +38,21 @@ module Sortable
private 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). query = Label.select(LabelPriority.arel_table[:priority].minimum).
left_join_priorities. left_join_priorities.
joins(:label_links). joins(:label_links).
where("label_priorities.project_id = #{project_column}"). where("label_priorities.project_id = #{project_column}").
where(label_links: { target_type: target_type }).
where("label_links.target_id = #{target_column}"). where("label_links.target_id = #{target_column}").
reorder(nil) 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 query
end end
......
...@@ -53,7 +53,7 @@ class Todo < ActiveRecord::Base ...@@ -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" # 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 def order_by_labels_priority
params = { params = {
target_type: ['Issue', 'MergeRequest'], target_type_column: "todos.target_type",
target_column: "todos.target_id", target_column: "todos.target_id",
project_column: "todos.project_id" project_column: "todos.project_id"
} }
......
...@@ -8,60 +8,90 @@ describe "Dashboard > User sorts todos", feature: true do ...@@ -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_2) { create(:label, title: 'label_2', project: project, priority: 2) }
let(:label_3) { create(:label, title: 'label_3', project: project, priority: 3) } let(:label_3) { create(:label, title: 'label_3', project: project, priority: 3) }
let(:issue_1) { create(:issue, title: 'issue_1', project: project) } before { project.team << [user, :developer] }
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
it "sorts with oldest created todos first" do context 'sort options' do
click_link "Last created" 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') let!(:merge_request_1) { create(:merge_request, source_project: project, title: "merge_request_1") }
expect(results_list.all('p')[0]).to have_content("merge_request_1")
expect(results_list.all('p')[1]).to have_content("issue_1") before do
expect(results_list.all('p')[2]).to have_content("issue_3") create(:todo, user: user, project: project, target: issue_4, created_at: 5.hours.ago)
expect(results_list.all('p')[3]).to have_content("issue_2") create(:todo, user: user, project: project, target: issue_2, created_at: 4.hours.ago)
expect(results_list.all('p')[4]).to have_content("issue_4") create(:todo, user: user, project: project, target: issue_3, created_at: 3.hours.ago)
end 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 it "sorts with newest created todos first" do
click_link "Oldest created" click_link "Oldest created"
results_list = page.find('.todos-list') results_list = page.find('.todos-list')
expect(results_list.all('p')[0]).to have_content("issue_4") 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')[1]).to have_content("issue_2")
expect(results_list.all('p')[2]).to have_content("issue_3") 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')[3]).to have_content("issue_1")
expect(results_list.all('p')[4]).to have_content("merge_request_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 end
it "sorts by priority" do context 'issues and merge requests' do
click_link "Priority" 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') results_list = page.find('.todos-list')
expect(results_list.all('p')[0]).to have_content("issue_3") expect(results_list.all('p')[0]).to have_content("issue_1")
expect(results_list.all('p')[1]).to have_content("merge_request_1") expect(results_list.all('p')[1]).to have_content("issue_2")
expect(results_list.all('p')[2]).to have_content("issue_1") expect(results_list.all('p')[2]).to have_content("merge_request_1")
expect(results_list.all('p')[3]).to have_content("issue_2") end
expect(results_list.all('p')[4]).to have_content("issue_4")
end end
end end
...@@ -298,6 +298,20 @@ describe Issue, "Issuable" do ...@@ -298,6 +298,20 @@ describe Issue, "Issuable" do
end end
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 describe ".with_label" do
let(:project) { create(:project, :public) } let(:project) { create(:project, :public) }
let(:bug) { create(:label, project: project, title: 'bug') } let(:bug) { create(:label, project: project, title: 'bug') }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment