diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index dbc3161d7..7d4212095 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -73,6 +73,8 @@ class IssuesController < ApplicationController # Send html if the query is not valid render(:template => 'issues/index.rhtml', :layout => !request.xhr?) end + rescue ActiveRecord::RecordNotFound + render_404 end def changes @@ -87,6 +89,8 @@ class IssuesController < ApplicationController end @title = (@project ? @project.name : Setting.app_title) + ": " + (@query.new_record? ? l(:label_changes_details) : @query.name) render :layout => false, :content_type => 'application/atom+xml' + rescue ActiveRecord::RecordNotFound + render_404 end def show @@ -384,7 +388,10 @@ private # Retrieve query from session or build a new query def retrieve_query if !params[:query_id].blank? - @query = Query.find(params[:query_id], :conditions => {:project_id => (@project ? @project.id : nil)}) + cond = "project_id IS NULL" + cond << " OR project_id = #{@project.id}" if @project + @query = Query.find(params[:query_id], :conditions => cond) + @query.project = @project session[:query] = {:id => @query.id, :project_id => @query.project_id} else if params[:set_filter] || session[:query].nil? || session[:query][:project_id] != (@project ? @project.id : nil) diff --git a/app/controllers/queries_controller.rb b/app/controllers/queries_controller.rb index 0a762eee0..194b1df57 100644 --- a/app/controllers/queries_controller.rb +++ b/app/controllers/queries_controller.rb @@ -18,19 +18,14 @@ class QueriesController < ApplicationController layout 'base' menu_item :issues - before_filter :find_project, :authorize - - def index - @queries = @project.queries.find(:all, - :order => "name ASC", - :conditions => ["is_public = ? or user_id = ?", true, (User.current.logged? ? User.current.id : 0)]) - end + before_filter :find_query, :except => :new + before_filter :find_project, :authorize, :only => :new def new @query = Query.new(params[:query]) - @query.project = @project + @query.project = params[:query_is_for_all] ? nil : @project @query.user = User.current - @query.is_public = false unless current_role.allowed_to?(:manage_public_queries) + @query.is_public = false unless (@query.project && current_role.allowed_to?(:manage_public_queries)) || User.current.admin? @query.column_names = nil if params[:default_columns] params[:fields].each do |field| @@ -52,7 +47,8 @@ class QueriesController < ApplicationController @query.add_filter(field, params[:operators][field], params[:values][field]) end if params[:fields] @query.attributes = params[:query] - @query.is_public = false unless current_role.allowed_to?(:manage_public_queries) + @query.project = nil if params[:query_is_for_all] + @query.is_public = false unless (@query.project && current_role.allowed_to?(:manage_public_queries)) || User.current.admin? @query.column_names = nil if params[:default_columns] if @query.save @@ -64,18 +60,20 @@ class QueriesController < ApplicationController def destroy @query.destroy if request.post? - redirect_to :controller => 'queries', :project_id => @project + redirect_to :controller => 'issues', :action => 'index', :project_id => @project, :set_filter => 1 end private + def find_query + @query = Query.find(params[:id]) + @project = @query.project + render_403 unless @query.editable_by?(User.current) + rescue ActiveRecord::RecordNotFound + render_404 + end + def find_project - if params[:id] - @query = Query.find(params[:id]) - @project = @query.project - render_403 unless @query.editable_by?(User.current) - else - @project = Project.find(params[:project_id]) - end + @project = Project.find(params[:project_id]) rescue ActiveRecord::RecordNotFound render_404 end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 17889fadd..6013f1ec8 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -32,6 +32,19 @@ module IssuesHelper "#{@cached_label_assigned_to}: #{issue.assigned_to}
" + "#{@cached_label_priority}: #{issue.priority.name}" end + + def sidebar_queries + unless @sidebar_queries + # User can see public queries and his own queries + visible = ARCondition.new(["is_public = ? OR user_id = ?", true, (User.current.logged? ? User.current.id : 0)]) + # Project specific queries and global queries + visible << (@project.nil? ? ["project_id IS NULL"] : ["project_id IS NULL OR project_id = ?", @project.id]) + @sidebar_queries = Query.find(:all, + :order => "name ASC", + :conditions => visible.conditions) + end + @sidebar_queries + end def show_detail(detail, no_html=false) case detail.property diff --git a/app/models/query.rb b/app/models/query.rb index dfdfa909b..c54c143e2 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -116,6 +116,11 @@ class Query < ActiveRecord::Base set_language_if_valid(User.current.language) end + def after_initialize + # Store the fact that project is nil (used in #editable_by?) + @is_for_all = project.nil? + end + def validate filters.each_key do |field| errors.add label_for(field), :activerecord_error_blank unless @@ -128,8 +133,10 @@ class Query < ActiveRecord::Base def editable_by?(user) return false unless user - return true if !is_public && self.user_id == user.id - is_public && user.allowed_to?(:manage_public_queries, project) + # Admin can edit them all and regular users can edit their private queries + return true if user.admin? || (!is_public && self.user_id == user.id) + # Members can not edit public queries that are for all project (only admin is allowed to) + is_public && !@is_for_all && user.allowed_to?(:manage_public_queries, project) end def available_filters diff --git a/app/views/issues/_sidebar.rhtml b/app/views/issues/_sidebar.rhtml index 4a1b7e9bc..c269eee06 100644 --- a/app/views/issues/_sidebar.rhtml +++ b/app/views/issues/_sidebar.rhtml @@ -1,13 +1,14 @@ +<% if @project %>

<%= l(:label_issue_plural) %>

<%= link_to l(:label_issue_view_all), { :controller => 'issues', :action => 'index', :project_id => @project, :set_filter => 1 } %>
<%= link_to l(:field_summary), :controller => 'reports', :action => 'issue_report', :id => @project %>
<%= link_to l(:label_change_log), :controller => 'projects', :action => 'changelog', :id => @project %> +<% end %> +<% unless sidebar_queries.empty? -%>

<%= l(:label_query_plural) %>

-<% queries = @project.queries.find(:all, - :order => "name ASC", - :conditions => ["is_public = ? or user_id = ?", true, (User.current.logged? ? User.current.id : 0)]) - queries.each do |query| %> +<% sidebar_queries.each do |query| -%> <%= link_to query.name, :controller => 'issues', :action => 'index', :project_id => @project, :query_id => query %>
-<% end %> +<% end -%> +<% end -%> diff --git a/app/views/issues/index.rhtml b/app/views/issues/index.rhtml index 094c49ff2..0123099f2 100644 --- a/app/views/issues/index.rhtml +++ b/app/views/issues/index.rhtml @@ -54,7 +54,7 @@ <% content_for :sidebar do %> <%= render :partial => 'issues/sidebar' %> -<% end if @project%> +<% end %> <% content_for :header_tags do %> <%= auto_discovery_link_tag(:atom, {:query_id => @query, :format => 'atom', :page => nil, :key => User.current.rss_key}, :title => l(:label_issue_plural)) %> diff --git a/app/views/queries/_form.rhtml b/app/views/queries/_form.rhtml index 2d4b96fd1..8da264032 100644 --- a/app/views/queries/_form.rhtml +++ b/app/views/queries/_form.rhtml @@ -6,11 +6,16 @@

<%= text_field 'query', 'name', :size => 80 %>

-<% if current_role.allowed_to?(:manage_public_queries) %> -

- <%= check_box 'query', 'is_public' %>

+<% if User.current.admin? || (@project && current_role.allowed_to?(:manage_public_queries)) %> +

+<%= check_box 'query', 'is_public', + :onchange => (User.current.admin? ? nil : 'if (this.checked) {$("query_is_for_all").checked = false; $("query_is_for_all").disabled = true;} else {$("query_is_for_all").disabled = false;}') %>

<% end %> +

+<%= check_box_tag 'query_is_for_all', 1, @query.project.nil?, + :disabled => (!@query.new_record? && (@query.project.nil? || (@query.is_public? && !User.current.admin?))) %>

+

<%= check_box_tag 'default_columns', 1, @query.has_default_columns?, :id => 'query_default_columns', :onclick => 'if (this.checked) {Element.hide("columns")} else {Element.show("columns")}' %>

diff --git a/test/fixtures/queries.yml b/test/fixtures/queries.yml index a4c045b15..f12022729 100644 --- a/test/fixtures/queries.yml +++ b/test/fixtures/queries.yml @@ -1,7 +1,9 @@ --- queries_001: - name: Multiple custom fields query + id: 1 project_id: 1 + is_public: true + name: Multiple custom fields query filters: | --- cf_1: @@ -17,6 +19,51 @@ queries_001: - "125" :operator: "=" - id: 1 - is_public: true user_id: 1 + column_names: +queries_002: + id: 2 + project_id: 1 + is_public: false + name: Private query for cookbook + filters: | + --- + tracker_id: + :values: + - "3" + :operator: "=" + status_id: + :values: + - "1" + :operator: o + + user_id: 3 + column_names: +queries_003: + id: 3 + project_id: + is_public: false + name: Private query for all projects + filters: | + --- + tracker_id: + :values: + - "3" + :operator: "=" + + user_id: 3 + column_names: +queries_004: + id: 4 + project_id: + is_public: true + name: Public query for all projects + filters: | + --- + tracker_id: + :values: + - "3" + :operator: "=" + + user_id: 2 + column_names: diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml index 0aabbb619..1ede6fca9 100644 --- a/test/fixtures/roles.yml +++ b/test/fixtures/roles.yml @@ -57,7 +57,6 @@ roles_002: - :add_issue_notes - :move_issues - :delete_issues - - :manage_public_queries - :save_queries - :view_gantt - :view_calendar @@ -94,7 +93,6 @@ roles_003: - :manage_issue_relations - :add_issue_notes - :move_issues - - :manage_public_queries - :save_queries - :view_gantt - :view_calendar diff --git a/test/functional/queries_controller_test.rb b/test/functional/queries_controller_test.rb new file mode 100644 index 000000000..86af8edd7 --- /dev/null +++ b/test/functional/queries_controller_test.rb @@ -0,0 +1,185 @@ +# redMine - project management software +# Copyright (C) 2006-2008 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.dirname(__FILE__) + '/../test_helper' +require 'queries_controller' + +# Re-raise errors caught by the controller. +class QueriesController; def rescue_action(e) raise e end; end + +class QueriesControllerTest < Test::Unit::TestCase + fixtures :projects, :users, :members, :roles, :trackers, :issue_statuses, :issue_categories, :enumerations, :issues, :custom_fields, :custom_values, :queries + + def setup + @controller = QueriesController.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + User.current = nil + end + + def test_get_new + @request.session[:user_id] = 2 + get :new, :project_id => 1 + assert_response :success + assert_template 'new' + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query[is_public]', + :checked => nil } + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query_is_for_all', + :checked => nil, + :disabled => nil } + end + + def test_new_project_public_query + @request.session[:user_id] = 2 + post :new, + :project_id => 'ecookbook', + :confirm => '1', + :default_columns => '1', + :fields => ["status_id", "assigned_to_id"], + :operators => {"assigned_to_id" => "=", "status_id" => "o"}, + :values => { "assigned_to_id" => ["1"], "status_id" => ["1"]}, + :query => {"name" => "test_new_project_public_query", "is_public" => "1"}, + :column_names => ["", "tracker", "status", "priority", "subject", "updated_on", "category"] + + q = Query.find_by_name('test_new_project_public_query') + assert_redirected_to :controller => 'issues', :action => 'index', :query_id => q + assert q.is_public? + assert q.has_default_columns? + assert q.valid? + end + + def test_new_project_private_query + @request.session[:user_id] = 3 + post :new, + :project_id => 'ecookbook', + :confirm => '1', + :default_columns => '1', + :fields => ["status_id", "assigned_to_id"], + :operators => {"assigned_to_id" => "=", "status_id" => "o"}, + :values => { "assigned_to_id" => ["1"], "status_id" => ["1"]}, + :query => {"name" => "test_new_project_private_query", "is_public" => "1"}, + :column_names => ["", "tracker", "status", "priority", "subject", "updated_on", "category"] + + q = Query.find_by_name('test_new_project_private_query') + assert_redirected_to :controller => 'issues', :action => 'index', :query_id => q + assert !q.is_public? + assert q.has_default_columns? + assert q.valid? + end + + def test_get_edit_global_public_query + @request.session[:user_id] = 1 + get :edit, :id => 4 + assert_response :success + assert_template 'edit' + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query[is_public]', + :checked => 'checked' } + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query_is_for_all', + :checked => 'checked', + :disabled => 'disabled' } + end + + def test_edit_global_public_query + @request.session[:user_id] = 1 + post :edit, + :id => 4, + :confirm => '1', + :default_columns => '1', + :fields => ["status_id", "assigned_to_id"], + :operators => {"assigned_to_id" => "=", "status_id" => "o"}, + :values => { "assigned_to_id" => ["1"], "status_id" => ["1"]}, + :query => {"name" => "test_edit_global_public_query", "is_public" => "1"}, + :column_names => ["", "tracker", "status", "priority", "subject", "updated_on", "category"] + + assert_redirected_to :controller => 'issues', :action => 'index', :query_id => 4 + q = Query.find_by_name('test_edit_global_public_query') + assert q.is_public? + assert q.has_default_columns? + assert q.valid? + end + + def test_get_edit_global_private_query + @request.session[:user_id] = 3 + get :edit, :id => 3 + assert_response :success + assert_template 'edit' + assert_no_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query[is_public]' } + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query_is_for_all', + :checked => 'checked', + :disabled => 'disabled' } + end + + def test_edit_global_private_query + @request.session[:user_id] = 3 + post :edit, + :id => 3, + :confirm => '1', + :default_columns => '1', + :fields => ["status_id", "assigned_to_id"], + :operators => {"assigned_to_id" => "=", "status_id" => "o"}, + :values => { "assigned_to_id" => ["me"], "status_id" => ["1"]}, + :query => {"name" => "test_edit_global_private_query", "is_public" => "1"}, + :column_names => ["", "tracker", "status", "priority", "subject", "updated_on", "category"] + + assert_redirected_to :controller => 'issues', :action => 'index', :query_id => 3 + q = Query.find_by_name('test_edit_global_private_query') + assert !q.is_public? + assert q.has_default_columns? + assert q.valid? + end + + def test_get_edit_project_private_query + @request.session[:user_id] = 3 + get :edit, :id => 2 + assert_response :success + assert_template 'edit' + assert_no_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query[is_public]' } + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query_is_for_all', + :checked => nil, + :disabled => nil } + end + + def test_get_edit_project_public_query + @request.session[:user_id] = 2 + get :edit, :id => 1 + assert_response :success + assert_template 'edit' + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query[is_public]', + :checked => 'checked' + } + assert_tag :tag => 'input', :attributes => { :type => 'checkbox', + :name => 'query_is_for_all', + :checked => nil, + :disabled => 'disabled' } + end + + def test_destroy + @request.session[:user_id] = 2 + post :destroy, :id => 1 + assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook', :set_filter => 1, :query_id => nil + assert_nil Query.find_by_id(1) + end +end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index c00f47e5d..3a9d19112 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -1,5 +1,5 @@ # redMine - project management software -# Copyright (C) 2006-2007 Jean-Philippe Lang +# Copyright (C) 2006-2008 Jean-Philippe Lang # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License @@ -18,7 +18,7 @@ require File.dirname(__FILE__) + '/../test_helper' class QueryTest < Test::Unit::TestCase - fixtures :projects, :users, :trackers, :issue_statuses, :issue_categories, :enumerations, :issues, :custom_fields, :custom_values, :queries + fixtures :projects, :users, :members, :roles, :trackers, :issue_statuses, :issue_categories, :enumerations, :issues, :custom_fields, :custom_values, :queries def test_query_with_multiple_custom_fields query = Query.find(1) @@ -41,4 +41,34 @@ class QueryTest < Test::Unit::TestCase c = q.columns.first assert q.has_column?(c) end + + def test_editable_by + admin = User.find(1) + manager = User.find(2) + developer = User.find(3) + + # Public query on project 1 + q = Query.find(1) + assert q.editable_by?(admin) + assert q.editable_by?(manager) + assert !q.editable_by?(developer) + + # Private query on project 1 + q = Query.find(2) + assert q.editable_by?(admin) + assert !q.editable_by?(manager) + assert q.editable_by?(developer) + + # Private query for all projects + q = Query.find(3) + assert q.editable_by?(admin) + assert !q.editable_by?(manager) + assert q.editable_by?(developer) + + # Public query for all projects + q = Query.find(4) + assert q.editable_by?(admin) + assert !q.editable_by?(manager) + assert !q.editable_by?(developer) + end end