Commit 657eb7ba93b1aae22d927f02d54a25ef2704ad5a
Exists in
spb-stable
and in
3 other branches
Merge branch 'bug/filtering_service' into 'master'
Fix wrong issues appears at Dashboard#issues page Fixes #1028
Showing
5 changed files
with
31 additions
and
10 deletions
Show diff stats
app/controllers/dashboard_controller.rb
| @@ -54,12 +54,12 @@ class DashboardController < ApplicationController | @@ -54,12 +54,12 @@ class DashboardController < ApplicationController | ||
| 54 | 54 | ||
| 55 | def merge_requests | 55 | def merge_requests |
| 56 | @merge_requests = FilteringService.new.execute(MergeRequest, current_user, params) | 56 | @merge_requests = FilteringService.new.execute(MergeRequest, current_user, params) |
| 57 | - @merge_requests = @merge_requests.recent.page(params[:page]).per(20) | 57 | + @merge_requests = @merge_requests.page(params[:page]).per(20) |
| 58 | end | 58 | end |
| 59 | 59 | ||
| 60 | def issues | 60 | def issues |
| 61 | @issues = FilteringService.new.execute(Issue, current_user, params) | 61 | @issues = FilteringService.new.execute(Issue, current_user, params) |
| 62 | - @issues = @issues.recent.page(params[:page]).per(20) | 62 | + @issues = @issues.page(params[:page]).per(20) |
| 63 | @issues = @issues.includes(:author, :project) | 63 | @issues = @issues.includes(:author, :project) |
| 64 | 64 | ||
| 65 | respond_to do |format| | 65 | respond_to do |format| |
app/models/concerns/issuable.rb
| @@ -48,13 +48,13 @@ module Issuable | @@ -48,13 +48,13 @@ module Issuable | ||
| 48 | 48 | ||
| 49 | def sort(method) | 49 | def sort(method) |
| 50 | case method.to_s | 50 | case method.to_s |
| 51 | - when 'newest' then reorder('created_at DESC') | ||
| 52 | - when 'oldest' then reorder('created_at ASC') | ||
| 53 | - when 'recently_updated' then reorder('updated_at DESC') | ||
| 54 | - when 'last_updated' then reorder('updated_at ASC') | 51 | + when 'newest' then reorder("#{table_name}.created_at DESC") |
| 52 | + when 'oldest' then reorder("#{table_name}.created_at ASC") | ||
| 53 | + when 'recently_updated' then reorder("#{table_name}.updated_at DESC") | ||
| 54 | + when 'last_updated' then reorder("#{table_name}.updated_at ASC") | ||
| 55 | when 'milestone_due_soon' then joins(:milestone).reorder("milestones.due_date ASC") | 55 | when 'milestone_due_soon' then joins(:milestone).reorder("milestones.due_date ASC") |
| 56 | when 'milestone_due_later' then joins(:milestone).reorder("milestones.due_date DESC") | 56 | when 'milestone_due_later' then joins(:milestone).reorder("milestones.due_date DESC") |
| 57 | - else reorder('created_at DESC') | 57 | + else reorder("#{table_name}.created_at DESC") |
| 58 | end | 58 | end |
| 59 | end | 59 | end |
| 60 | end | 60 | end |
app/services/filtering_service.rb
| @@ -57,11 +57,11 @@ class FilteringService | @@ -57,11 +57,11 @@ class FilteringService | ||
| 57 | def by_scope(items) | 57 | def by_scope(items) |
| 58 | case params[:scope] | 58 | case params[:scope] |
| 59 | when 'created-by-me', 'authored' then | 59 | when 'created-by-me', 'authored' then |
| 60 | - klass.where(author_id: current_user.id) | 60 | + items.where(author_id: current_user.id) |
| 61 | when 'all' then | 61 | when 'all' then |
| 62 | - klass | 62 | + items |
| 63 | when 'assigned-to-me' then | 63 | when 'assigned-to-me' then |
| 64 | - klass.where(assignee_id: current_user.id) | 64 | + items.where(assignee_id: current_user.id) |
| 65 | else | 65 | else |
| 66 | raise 'You must specify default scope' | 66 | raise 'You must specify default scope' |
| 67 | end | 67 | end |
spec/features/atom/dashboard_issues_spec.rb
| @@ -8,6 +8,11 @@ describe "Dashboard Issues Feed" do | @@ -8,6 +8,11 @@ describe "Dashboard Issues Feed" do | ||
| 8 | let!(:issue1) { create(:issue, author: user, assignee: user, project: project1) } | 8 | let!(:issue1) { create(:issue, author: user, assignee: user, project: project1) } |
| 9 | let!(:issue2) { create(:issue, author: user, assignee: user, project: project2) } | 9 | let!(:issue2) { create(:issue, author: user, assignee: user, project: project2) } |
| 10 | 10 | ||
| 11 | + before do | ||
| 12 | + project1.team << [user, :master] | ||
| 13 | + project2.team << [user, :master] | ||
| 14 | + end | ||
| 15 | + | ||
| 11 | describe "atom feed" do | 16 | describe "atom feed" do |
| 12 | it "should render atom feed via private token" do | 17 | it "should render atom feed via private token" do |
| 13 | visit issues_dashboard_path(:atom, private_token: user.private_token) | 18 | visit issues_dashboard_path(:atom, private_token: user.private_token) |
spec/services/filtering_service_spec.rb
| @@ -15,6 +15,7 @@ describe FilteringService do | @@ -15,6 +15,7 @@ describe FilteringService do | ||
| 15 | before do | 15 | before do |
| 16 | project1.team << [user, :master] | 16 | project1.team << [user, :master] |
| 17 | project2.team << [user, :developer] | 17 | project2.team << [user, :developer] |
| 18 | + project2.team << [user2, :developer] | ||
| 18 | end | 19 | end |
| 19 | 20 | ||
| 20 | describe 'merge requests' do | 21 | describe 'merge requests' do |
| @@ -61,5 +62,20 @@ describe FilteringService do | @@ -61,5 +62,20 @@ describe FilteringService do | ||
| 61 | issues = FilteringService.new.execute(Issue, user, params) | 62 | issues = FilteringService.new.execute(Issue, user, params) |
| 62 | issues.size.should == 1 | 63 | issues.size.should == 1 |
| 63 | end | 64 | end |
| 65 | + | ||
| 66 | + it 'should be empty for unauthorized user' do | ||
| 67 | + params = { scope: "all", state: 'opened' } | ||
| 68 | + issues = FilteringService.new.execute(Issue, nil, params) | ||
| 69 | + issues.size.should be_zero | ||
| 70 | + end | ||
| 71 | + | ||
| 72 | + it 'should not include unauthorized issues' do | ||
| 73 | + params = { scope: "all", state: 'opened' } | ||
| 74 | + issues = FilteringService.new.execute(Issue, user2, params) | ||
| 75 | + issues.size.should == 2 | ||
| 76 | + issues.should_not include(issue1) | ||
| 77 | + issues.should include(issue2) | ||
| 78 | + issues.should include(issue3) | ||
| 79 | + end | ||
| 64 | end | 80 | end |
| 65 | end | 81 | end |