Merge branch 'bug/filtering_service' into 'master'

Fix wrong issues appears at Dashboard#issues page

Fixes #1028

5 files changed, 31 insertions, 10 deletions

diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index 656eda9dec2..b95233c0e91 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -54,12 +54,12 @@ class DashboardController < ApplicationController
 
   def merge_requests
     @merge_requests = FilteringService.new.execute(MergeRequest, current_user, params)
-    @merge_requests = @merge_requests.recent.page(params[:page]).per(20)
+    @merge_requests = @merge_requests.page(params[:page]).per(20)
   end
 
   def issues
     @issues = FilteringService.new.execute(Issue, current_user, params)
-    @issues = @issues.recent.page(params[:page]).per(20)
+    @issues = @issues.page(params[:page]).per(20)
     @issues = @issues.includes(:author, :project)
 
     respond_to do |format|
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 0f1dad4ef16..bf2c2157d38 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -48,13 +48,13 @@ module Issuable
 
     def sort(method)
       case method.to_s
-      when 'newest' then reorder('created_at DESC')
-      when 'oldest' then reorder('created_at ASC')
-      when 'recently_updated' then reorder('updated_at DESC')
-      when 'last_updated' then reorder('updated_at ASC')
+      when 'newest' then reorder("#{table_name}.created_at DESC")
+      when 'oldest' then reorder("#{table_name}.created_at ASC")
+      when 'recently_updated' then reorder("#{table_name}.updated_at DESC")
+      when 'last_updated' then reorder("#{table_name}.updated_at ASC")
       when 'milestone_due_soon' then joins(:milestone).reorder("milestones.due_date ASC")
       when 'milestone_due_later' then joins(:milestone).reorder("milestones.due_date DESC")
-      else reorder('created_at DESC')
+      else reorder("#{table_name}.created_at DESC")
       end
     end
   end
diff --git a/app/services/filtering_service.rb b/app/services/filtering_service.rb
index ebd394ee758..52537f7ba4f 100644
--- a/app/services/filtering_service.rb
+++ b/app/services/filtering_service.rb
@@ -57,11 +57,11 @@ class FilteringService
   def by_scope(items)
     case params[:scope]
     when 'created-by-me', 'authored' then
-      klass.where(author_id: current_user.id)
+      items.where(author_id: current_user.id)
     when 'all' then
-      klass
+      items
     when 'assigned-to-me' then
-      klass.where(assignee_id: current_user.id)
+      items.where(assignee_id: current_user.id)
     else
       raise 'You must specify default scope'
     end
diff --git a/spec/features/atom/dashboard_issues_spec.rb b/spec/features/atom/dashboard_issues_spec.rb
index 6f5d51d16b6..62f44690349 100644
--- a/spec/features/atom/dashboard_issues_spec.rb
+++ b/spec/features/atom/dashboard_issues_spec.rb
@@ -8,6 +8,11 @@ describe "Dashboard Issues Feed" do
     let!(:issue1)   { create(:issue, author: user, assignee: user, project: project1) }
     let!(:issue2)   { create(:issue, author: user, assignee: user, project: project2) }
 
+    before do
+      project1.team << [user, :master]
+      project2.team << [user, :master]
+    end
+
     describe "atom feed" do
       it "should render atom feed via private token" do
         visit issues_dashboard_path(:atom, private_token: user.private_token)
diff --git a/spec/services/filtering_service_spec.rb b/spec/services/filtering_service_spec.rb
index 596601264b3..92971d0be12 100644
--- a/spec/services/filtering_service_spec.rb
+++ b/spec/services/filtering_service_spec.rb
@@ -15,6 +15,7 @@ describe FilteringService do
   before do
     project1.team << [user, :master]
     project2.team << [user, :developer]
+    project2.team << [user2, :developer]
   end
 
   describe 'merge requests' do
@@ -61,5 +62,20 @@ describe FilteringService do
       issues = FilteringService.new.execute(Issue, user, params)
       issues.size.should == 1
     end
+
+    it 'should be empty for unauthorized user' do
+      params = { scope: "all", state: 'opened' }
+      issues = FilteringService.new.execute(Issue, nil, params)
+      issues.size.should be_zero
+    end
+
+    it 'should not include unauthorized issues' do
+      params = { scope: "all", state: 'opened' }
+      issues = FilteringService.new.execute(Issue, user2, params)
+      issues.size.should == 2
+      issues.should_not include(issue1)
+      issues.should include(issue2)
+      issues.should include(issue3)
+    end
   end
 end