Commit 631eed02 authored by Sean McGivern's avatar Sean McGivern

Remove default scope from todos

This was causing todo priority sorting to fail.
parent fc9955ce
...@@ -150,9 +150,7 @@ class TodosFinder ...@@ -150,9 +150,7 @@ class TodosFinder
if project? if project?
items.where(project: project) items.where(project: project)
else else
projects = Project projects = Project.public_or_visible_to_user(current_user)
.public_or_visible_to_user(current_user)
.order_id_desc
items.joins(:project).merge(projects) items.joins(:project).merge(projects)
end end
......
...@@ -32,8 +32,6 @@ class Todo < ActiveRecord::Base ...@@ -32,8 +32,6 @@ class Todo < ActiveRecord::Base
validates :target_id, presence: true, unless: :for_commit? validates :target_id, presence: true, unless: :for_commit?
validates :commit_id, presence: true, if: :for_commit? validates :commit_id, presence: true, if: :for_commit?
default_scope { reorder(id: :desc) }
scope :pending, -> { with_state(:pending) } scope :pending, -> { with_state(:pending) }
scope :done, -> { with_state(:done) } scope :done, -> { with_state(:done) }
...@@ -53,10 +51,14 @@ class Todo < ActiveRecord::Base ...@@ -53,10 +51,14 @@ class Todo < ActiveRecord::Base
# milestones, but still show something if the user has a URL with that # milestones, but still show something if the user has a URL with that
# selected. # selected.
def sort(method) def sort(method)
sorted =
case method.to_s case method.to_s
when 'priority', 'label_priority' then order_by_labels_priority when 'priority', 'label_priority' then order_by_labels_priority
else order_by(method) else order_by(method)
end end
# Break ties with the ID column for pagination
sorted.order(id: :desc)
end end
# Order by priority depending on which issue/merge request the Todo belongs to # Order by priority depending on which issue/merge request the Todo belongs to
......
...@@ -3,11 +3,12 @@ require 'spec_helper' ...@@ -3,11 +3,12 @@ require 'spec_helper'
describe TodosFinder do describe TodosFinder do
describe '#execute' do describe '#execute' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project) } let(:group) { create(:group) }
let(:project) { create(:project, namespace: group) }
let(:finder) { described_class } let(:finder) { described_class }
before do before do
project.add_developer(user) group.add_developer(user)
end end
describe '#sort' do describe '#sort' do
...@@ -34,17 +35,20 @@ describe TodosFinder do ...@@ -34,17 +35,20 @@ describe TodosFinder do
end end
it "sorts by priority" do it "sorts by priority" do
project_2 = create(:project)
label_1 = create(:label, title: 'label_1', project: project, priority: 1) label_1 = create(:label, title: 'label_1', project: project, priority: 1)
label_2 = create(:label, title: 'label_2', project: project, priority: 2) label_2 = create(:label, title: 'label_2', project: project, priority: 2)
label_3 = create(:label, title: 'label_3', project: project, priority: 3) label_3 = create(:label, title: 'label_3', project: project, priority: 3)
label_1_2 = create(:label, title: 'label_1', project: project_2, priority: 1)
issue_1 = create(:issue, title: 'issue_1', project: project) issue_1 = create(:issue, title: 'issue_1', project: project)
issue_2 = create(:issue, title: 'issue_2', project: project) issue_2 = create(:issue, title: 'issue_2', project: project)
issue_3 = create(:issue, title: 'issue_3', project: project) issue_3 = create(:issue, title: 'issue_3', project: project)
issue_4 = create(:issue, title: 'issue_4', project: project) issue_4 = create(:issue, title: 'issue_4', project: project)
merge_request_1 = create(:merge_request, source_project: project) merge_request_1 = create(:merge_request, source_project: project_2)
merge_request_1.labels << label_1 merge_request_1.labels << label_1_2
# Covers the case where Todo has more than one label # Covers the case where Todo has more than one label
issue_3.labels << label_1 issue_3.labels << label_1
...@@ -57,15 +61,14 @@ describe TodosFinder do ...@@ -57,15 +61,14 @@ describe TodosFinder do
todo_2 = create(:todo, user: user, project: project, target: issue_2) todo_2 = create(:todo, user: user, project: project, target: issue_2)
todo_3 = create(:todo, user: user, project: project, target: issue_3, created_at: 2.hours.ago) todo_3 = create(:todo, user: user, project: project, target: issue_3, created_at: 2.hours.ago)
todo_4 = create(:todo, user: user, project: project, target: issue_1) todo_4 = create(:todo, user: user, project: project, target: issue_1)
todo_5 = create(:todo, user: user, project: project, target: merge_request_1, created_at: 1.hour.ago) todo_5 = create(:todo, user: user, project: project_2, target: merge_request_1, created_at: 1.hour.ago)
project_2.add_developer(user)
todos = finder.new(user, { sort: 'priority' }).execute todos = finder.new(user, { sort: 'priority' }).execute
expect(todos.first).to eq(todo_3) puts todos.to_sql
expect(todos.second).to eq(todo_5) expect(todos).to eq([todo_3, todo_5, todo_4, todo_2, todo_1])
expect(todos.third).to eq(todo_4)
expect(todos.fourth).to eq(todo_2)
expect(todos.fifth).to eq(todo_1)
end end
end end
end end
......
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