From: Jean-Philippe Lang Date: Mon, 29 Jun 2015 16:06:37 +0000 (+0000) Subject: Fixed that user with "Manage public queries" permission, can create global public... X-Git-Tag: 3.2.0~352 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=004fc8b84b1cebc521664ca3830f969a91244e67;p=redmine.git Fixed that user with "Manage public queries" permission, can create global public query (#19842). git-svn-id: http://svn.redmine.org/redmine/trunk@14388 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/controllers/queries_controller.rb b/app/controllers/queries_controller.rb index ac91b1a15..e09790b94 100644 --- a/app/controllers/queries_controller.rb +++ b/app/controllers/queries_controller.rb @@ -48,17 +48,14 @@ class QueriesController < ApplicationController @query = IssueQuery.new @query.user = User.current @query.project = @project - @query.visibility = IssueQuery::VISIBILITY_PRIVATE unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin? @query.build_from_params(params) end def create - @query = IssueQuery.new(params[:query]) + @query = IssueQuery.new @query.user = User.current - @query.project = params[:query_is_for_all] ? nil : @project - @query.visibility = IssueQuery::VISIBILITY_PRIVATE unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin? - @query.build_from_params(params) - @query.column_names = nil if params[:default_columns] + @query.project = @project + update_query_from_params if @query.save flash[:notice] = l(:notice_successful_create) @@ -72,11 +69,7 @@ class QueriesController < ApplicationController end def update - @query.attributes = params[:query] - @query.project = nil if params[:query_is_for_all] - @query.visibility = IssueQuery::VISIBILITY_PRIVATE unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin? - @query.build_from_params(params) - @query.column_names = nil if params[:default_columns] + update_query_from_params if @query.save flash[:notice] = l(:notice_successful_update) @@ -107,6 +100,20 @@ private render_404 end + def update_query_from_params + @query.project = params[:query_is_for_all] ? nil : @project + @query.build_from_params(params) + @query.column_names = nil if params[:default_columns] + @query.sort_criteria = params[:query] && params[:query][:sort_criteria] + @query.name = params[:query] && params[:query][:name] + if User.current.allowed_to?(:manage_public_queries, @query.project) || User.current.admin? + @query.visibility = (params[:query] && params[:query][:visibility]) || IssueQuery::VISIBILITY_PRIVATE + else + @query.visibility = IssueQuery::VISIBILITY_PRIVATE + end + @query + end + def redirect_to_issues(options) if params[:gantt] if @project diff --git a/app/models/query.rb b/app/models/query.rb index cac43a4e4..b77f76c2e 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -487,7 +487,9 @@ class Query < ActiveRecord::Base if arg.is_a?(Hash) arg = arg.keys.sort.collect {|k| arg[k]} end - c = arg.select {|k,o| !k.to_s.blank?}.slice(0,3).collect {|k,o| [k.to_s, (o == 'desc' || o == false) ? 'desc' : 'asc']} + if arg + c = arg.select {|k,o| !k.to_s.blank?}.slice(0,3).collect {|k,o| [k.to_s, (o == 'desc' || o == false) ? 'desc' : 'asc']} + end write_attribute(:sort_criteria, c) end diff --git a/app/views/queries/_form.html.erb b/app/views/queries/_form.html.erb index fc03b857d..753eaedab 100644 --- a/app/views/queries/_form.html.erb +++ b/app/views/queries/_form.html.erb @@ -7,21 +7,20 @@

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

-<% if User.current.admin? || User.current.allowed_to?(:manage_public_queries, @project) %> +<% if User.current.admin? || User.current.allowed_to?(:manage_public_queries, @query.project) %>

+ <% Role.givable.sorted.each do |role| %> <% end %> - <%= hidden_field_tag 'query[role_ids][]', '' %>

<% 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 'query_is_for_all', 1, @query.project.nil?, :class => (User.current.admin? ? '' : 'disable-unless-private') %>

<% unless params[:gantt] %>
<%= l(:label_options) %> @@ -80,8 +79,11 @@ <%= javascript_tag do %> $(document).ready(function(){ $("input[name='query[visibility]']").change(function(){ - var checked = $('#query_visibility_1').is(':checked'); - $("input[name='query[role_ids][]'][type=checkbox]").attr('disabled', !checked); + var roles_checked = $('#query_visibility_1').is(':checked'); + var private_checked = $('#query_visibility_0').is(':checked'); + $("input[name='query[role_ids][]'][type=checkbox]").attr('disabled', !roles_checked); + if (!private_checked) $("input.disable-unless-private").attr('checked', false); + $("input.disable-unless-private").attr('disabled', !private_checked); }).trigger('change'); }); <% end %> diff --git a/test/functional/queries_controller_test.rb b/test/functional/queries_controller_test.rb index ab2321bc5..dd93de4bf 100644 --- a/test/functional/queries_controller_test.rb +++ b/test/functional/queries_controller_test.rb @@ -83,7 +83,7 @@ class QueriesControllerTest < ActionController::TestCase :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", "visibility" => "2"} + :query => {"name" => "test_new_project_private_query", "visibility" => "0"} q = Query.find_by_name('test_new_project_private_query') assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook', :query_id => q @@ -98,7 +98,7 @@ class QueriesControllerTest < ActionController::TestCase :fields => ["status_id", "assigned_to_id"], :operators => {"assigned_to_id" => "=", "status_id" => "o"}, :values => { "assigned_to_id" => ["me"], "status_id" => ["1"]}, - :query => {"name" => "test_new_global_private_query", "visibility" => "2"}, + :query => {"name" => "test_new_global_private_query", "visibility" => "0"}, :c => ["", "tracker", "subject", "priority", "category"] q = Query.find_by_name('test_new_global_private_query') @@ -119,6 +119,7 @@ class QueriesControllerTest < ActionController::TestCase q = Query.find_by_name('test_new_global_query') assert_redirected_to :controller => 'issues', :action => 'index', :project_id => nil, :query_id => q + assert !q.is_public? assert !q.has_filter?(:status_id) assert_equal ['assigned_to_id'], q.filters.keys assert q.valid? @@ -186,13 +187,73 @@ class QueriesControllerTest < ActionController::TestCase assert_equal false, query.draw_progress_line end + def test_create_project_public_query_should_force_private_without_manage_public_queries_permission + @request.session[:user_id] = 3 + query = new_record(Query) do + post :create, + :project_id => 'ecookbook', + :query => {"name" => "name", "visibility" => "2"} + assert_response 302 + end + assert_not_nil query.project + assert_equal Query::VISIBILITY_PRIVATE, query.visibility + end + + def test_create_global_public_query_should_force_private_without_manage_public_queries_permission + @request.session[:user_id] = 3 + query = new_record(Query) do + post :create, + :project_id => 'ecookbook', :query_is_for_all => '1', + :query => {"name" => "name", "visibility" => "2"} + assert_response 302 + end + assert_nil query.project + assert_equal Query::VISIBILITY_PRIVATE, query.visibility + end + + def test_create_project_public_query_with_manage_public_queries_permission + @request.session[:user_id] = 2 + query = new_record(Query) do + post :create, + :project_id => 'ecookbook', + :query => {"name" => "name", "visibility" => "2"} + assert_response 302 + end + assert_not_nil query.project + assert_equal Query::VISIBILITY_PUBLIC, query.visibility + end + + def test_create_global_public_query_should_force_private_with_manage_public_queries_permission + @request.session[:user_id] = 2 + query = new_record(Query) do + post :create, + :project_id => 'ecookbook', :query_is_for_all => '1', + :query => {"name" => "name", "visibility" => "2"} + assert_response 302 + end + assert_nil query.project + assert_equal Query::VISIBILITY_PRIVATE, query.visibility + end + + def test_create_global_public_query_by_admin + @request.session[:user_id] = 1 + query = new_record(Query) do + post :create, + :project_id => 'ecookbook', :query_is_for_all => '1', + :query => {"name" => "name", "visibility" => "2"} + assert_response 302 + end + assert_nil query.project + assert_equal Query::VISIBILITY_PUBLIC, query.visibility + end + def test_edit_global_public_query @request.session[:user_id] = 1 get :edit, :id => 4 assert_response :success assert_template 'edit' assert_select 'input[name=?][value="2"][checked=checked]', 'query[visibility]' - assert_select 'input[name=query_is_for_all][type=checkbox][checked=checked][disabled=disabled]' + assert_select 'input[name=query_is_for_all][type=checkbox][checked=checked]' end def test_edit_global_private_query @@ -201,7 +262,7 @@ class QueriesControllerTest < ActionController::TestCase assert_response :success assert_template 'edit' assert_select 'input[name=?]', 'query[visibility]', 0 - assert_select 'input[name=query_is_for_all][type=checkbox][checked=checked][disabled=disabled]' + assert_select 'input[name=query_is_for_all][type=checkbox][checked=checked]' end def test_edit_project_private_query @@ -210,7 +271,7 @@ class QueriesControllerTest < ActionController::TestCase assert_response :success assert_template 'edit' assert_select 'input[name=?]', 'query[visibility]', 0 - assert_select 'input[name=query_is_for_all][type=checkbox]:not([checked]):not([disabled])' + assert_select 'input[name=query_is_for_all][type=checkbox]:not([checked])' end def test_edit_project_public_query @@ -219,7 +280,7 @@ class QueriesControllerTest < ActionController::TestCase assert_response :success assert_template 'edit' assert_select 'input[name=?][value="2"][checked=checked]', 'query[visibility]' - assert_select 'input[name=query_is_for_all][type=checkbox][disabled=disabled]:not([checked])' + assert_select 'input[name=query_is_for_all][type=checkbox]:not([checked])' end def test_edit_sort_criteria diff --git a/test/test_helper.rb b/test/test_helper.rb index be4e68e18..73987a592 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -180,6 +180,15 @@ class ActiveSupport::TestCase ActiveRecord::Base.connection.quoted_date(date) end + # Asserts that a new record for the given class is created + # and returns it + def new_record(klass, &block) + assert_difference "#{klass}.count" do + yield + end + klass.order(:id => :desc).first + end + def assert_save(object) saved = object.save message = "#{object.class} could not be saved"