From 56f989e53e80fe7545a3400ba7ee98a0cd2cf259 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 10 Feb 2014 15:04:52 +0200 Subject: Fix wrong issues appears at Dashboard#issues page Filtering service used klass instead of passed items. Because of this you see list of all issues intead of authorized ones. This commit fixes it so people see only issues they are authorized to see. Signed-off-by: Dmitriy Zaporozhets --- app/controllers/dashboard_controller.rb | 4 ++-- app/models/concerns/issuable.rb | 10 +++++----- app/services/filtering_service.rb | 6 +++--- 3 files changed, 10 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 -- cgit v1.2.1 From ec437ad999858db18627174ff1cf9f272e4ddefb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 10 Feb 2014 15:23:19 +0200 Subject: Add more tests for FilteringService Signed-off-by: Dmitriy Zaporozhets --- spec/services/filtering_service_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 -- cgit v1.2.1 From 5e30f4d54c61f6d9cfee731c8cce4fa1ef58d628 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 10 Feb 2014 17:32:36 +0200 Subject: Fix dashboard atom feed Signed-off-by: Dmitriy Zaporozhets --- spec/features/atom/dashboard_issues_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) 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) -- cgit v1.2.1