From 888c3581eb0fbfc5ede87a24f7f03bfa4f7d810b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Thu, 11 Jul 2013 17:45:10 +0000 Subject: [PATCH] Role based custom queries (#1019). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11994 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/queries_controller.rb | 6 +- app/helpers/issues_helper.rb | 4 +- app/models/issue_query.rb | 43 +++++++++++- app/models/query.rb | 19 +++++- app/models/user.rb | 2 +- app/views/queries/_form.html.erb | 23 ++++++- app/views/queries/index.api.rsb | 2 +- config/locales/en.yml | 3 + config/locales/fr.yml | 3 + .../20130602092539_create_queries_roles.rb | 13 ++++ .../20130710182539_add_queries_visibility.rb | 13 ++++ public/stylesheets/application.css | 2 + test/fixtures/queries.yml | 18 ++--- test/functional/calendars_controller_test.rb | 2 +- test/functional/issues_controller_test.rb | 12 ++-- test/functional/queries_controller_test.rb | 34 ++++------ test/unit/query_test.rb | 66 +++++++++++++++++++ test/unit/user_test.rb | 4 +- 18 files changed, 214 insertions(+), 55 deletions(-) create mode 100644 db/migrate/20130602092539_create_queries_roles.rb create mode 100644 db/migrate/20130710182539_add_queries_visibility.rb diff --git a/app/controllers/queries_controller.rb b/app/controllers/queries_controller.rb index 639255125..cd1ec87a2 100644 --- a/app/controllers/queries_controller.rb +++ b/app/controllers/queries_controller.rb @@ -45,7 +45,7 @@ class QueriesController < ApplicationController @query = IssueQuery.new @query.user = User.current @query.project = @project - @query.is_public = false unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin? + @query.visibility = IssueQuery::VISIBILITY_PRIVATE unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin? @query.build_from_params(params) end @@ -53,7 +53,7 @@ class QueriesController < ApplicationController @query = IssueQuery.new(params[:query]) @query.user = User.current @query.project = params[:query_is_for_all] ? nil : @project - @query.is_public = false unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin? + @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] @@ -71,7 +71,7 @@ class QueriesController < ApplicationController def update @query.attributes = params[:query] @query.project = nil if params[:query_is_for_all] - @query.is_public = false unless User.current.allowed_to?(:manage_public_queries, @project) || User.current.admin? + @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] diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 05090c7b3..f88a61e21 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -225,8 +225,8 @@ module IssuesHelper def render_sidebar_queries out = ''.html_safe - out << query_links(l(:label_my_queries), sidebar_queries.reject(&:is_public?)) - out << query_links(l(:label_query_plural), sidebar_queries.select(&:is_public?)) + out << query_links(l(:label_my_queries), sidebar_queries.select(&:is_private?)) + out << query_links(l(:label_query_plural), sidebar_queries.reject(&:is_private?)) out end diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index f9adf4697..b2e470d7a 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -45,9 +45,25 @@ class IssueQuery < Query scope :visible, lambda {|*args| user = args.shift || User.current base = Project.allowed_to_condition(user, :view_issues, *args) - user_id = user.logged? ? user.id : 0 + scope = includes(:project).where("#{table_name}.project_id IS NULL OR (#{base})") - includes(:project).where("(#{table_name}.project_id IS NULL OR (#{base})) AND (#{table_name}.is_public = ? OR #{table_name}.user_id = ?)", true, user_id) + if user.admin? + scope.where("#{table_name}.visibility <> ? OR #{table_name}.user_id = ?", VISIBILITY_PRIVATE, user.id) + elsif user.memberships.any? + scope.where("#{table_name}.visibility = ?" + + " OR (#{table_name}.visibility = ? AND #{table_name}.id IN (" + + "SELECT DISTINCT q.id FROM #{table_name} q" + + " INNER JOIN #{table_name_prefix}queries_roles#{table_name_suffix} qr on qr.query_id = q.id" + + " INNER JOIN #{MemberRole.table_name} mr ON mr.role_id = qr.role_id" + + " INNER JOIN #{Member.table_name} m ON m.id = mr.member_id AND m.user_id = ?" + + " WHERE q.project_id IS NULL OR q.project_id = m.project_id))" + + " OR #{table_name}.user_id = ?", + VISIBILITY_PUBLIC, VISIBILITY_ROLES, user.id, user.id) + elsif user.logged? + scope.where("#{table_name}.visibility = ? OR #{table_name}.user_id = ?", VISIBILITY_PUBLIC, user.id) + else + scope.where("#{table_name}.visibility = ?", VISIBILITY_PUBLIC) + end } def initialize(attributes=nil, *args) @@ -57,7 +73,28 @@ class IssueQuery < Query # Returns true if the query is visible to +user+ or the current user. def visible?(user=User.current) - (project.nil? || user.allowed_to?(:view_issues, project)) && (self.is_public? || self.user_id == user.id) + return true if user.admin? + return false unless project.nil? || user.allowed_to?(:view_issues, project) + case visibility + when VISIBILITY_PUBLIC + true + when VISIBILITY_ROLES + if project + (user.roles_for_project(project) & roles).any? + else + Member.where(:user_id => user.id).joins(:roles).where(:member_roles => {:role_id => roles.map(&:id)}).any? + end + else + user == self.user + end + end + + def is_private? + visibility == VISIBILITY_PRIVATE + end + + def is_public? + !is_private? end def initialize_available_filters diff --git a/app/models/query.rb b/app/models/query.rb index aa24857bd..2753c2768 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -116,8 +116,13 @@ class Query < ActiveRecord::Base class StatementInvalid < ::ActiveRecord::StatementInvalid end + VISIBILITY_PRIVATE = 0 + VISIBILITY_ROLES = 1 + VISIBILITY_PUBLIC = 2 + belongs_to :project belongs_to :user + has_and_belongs_to_many :roles, :join_table => "#{table_name_prefix}queries_roles#{table_name_suffix}", :foreign_key => "query_id" serialize :filters serialize :column_names serialize :sort_criteria, Array @@ -126,7 +131,17 @@ class Query < ActiveRecord::Base validates_presence_of :name validates_length_of :name, :maximum => 255 + validates :visibility, :inclusion => { :in => [VISIBILITY_PUBLIC, VISIBILITY_ROLES, VISIBILITY_PRIVATE] } validate :validate_query_filters + validate do |query| + errors.add(:base, l(:label_role_plural) + ' ' + l('activerecord.errors.messages.blank')) if query.visibility == VISIBILITY_ROLES && roles.blank? + end + + after_save do |query| + if query.visibility_changed? && query.visibility != VISIBILITY_ROLES + query.roles.clear + end + end class_attribute :operators self.operators = { @@ -245,9 +260,9 @@ class Query < ActiveRecord::Base def editable_by?(user) return false unless user # 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) + return true if user.admin? || (is_private? && 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) + is_public? && !@is_for_all && user.allowed_to?(:manage_public_queries, project) end def trackers diff --git a/app/models/user.rb b/app/models/user.rb index 40a7120b4..441e6b5b8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -669,7 +669,7 @@ class User < Principal Message.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] News.update_all ['author_id = ?', substitute.id], ['author_id = ?', id] # Remove private queries and keep public ones - ::Query.delete_all ['user_id = ? AND is_public = ?', id, false] + ::Query.delete_all ['user_id = ? AND visibility = ?', id, ::Query::VISIBILITY_PRIVATE] ::Query.update_all ['user_id = ?', substitute.id], ['user_id = ?', id] TimeEntry.update_all ['user_id = ?', substitute.id], ['user_id = ?', id] Token.delete_all ['user_id = ?', id] diff --git a/app/views/queries/_form.html.erb b/app/views/queries/_form.html.erb index 1070b4716..116821529 100644 --- a/app/views/queries/_form.html.erb +++ b/app/views/queries/_form.html.erb @@ -6,15 +6,22 @@ <%= text_field 'query', 'name', :size => 80 %>

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

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

+

+ + + <% 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?))) %>

+
<%= l(:label_options) %>

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

@@ -24,6 +31,7 @@

<%= available_block_columns_tags(@query) %>

+
<%= l(:label_filter_plural) %> @@ -53,3 +61,12 @@ <% end %> + +<%= 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); + }).trigger('change'); +}); +<% end %> diff --git a/app/views/queries/index.api.rsb b/app/views/queries/index.api.rsb index 800b4964e..cdf8e4020 100644 --- a/app/views/queries/index.api.rsb +++ b/app/views/queries/index.api.rsb @@ -3,7 +3,7 @@ api.array :queries, api_meta(:total_count => @query_count, :offset => @offset, : api.query do api.id query.id api.name query.name - api.is_public query.is_public + api.is_public query.is_public? api.project_id query.project_id end end diff --git a/config/locales/en.yml b/config/locales/en.yml index eebc34500..7099de8c4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -896,6 +896,9 @@ en: label_cross_project_hierarchy: With project hierarchy label_cross_project_system: With all projects label_gantt_progress_line: Progress line + label_visibility_private: to me only + label_visibility_roles: to these roles only + label_visibility_public: to any users button_login: Login button_submit: Submit diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 0847308b0..510913a17 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -872,6 +872,9 @@ fr: label_cross_project_hierarchy: Avec toute la hiérarchie label_cross_project_system: Avec tous les projets label_gantt_progress_line: Ligne de progression + label_visibility_private: par moi uniquement + label_visibility_roles: par ces roles uniquement + label_visibility_public: par tout le monde button_login: Connexion button_submit: Soumettre diff --git a/db/migrate/20130602092539_create_queries_roles.rb b/db/migrate/20130602092539_create_queries_roles.rb new file mode 100644 index 000000000..7713a7a36 --- /dev/null +++ b/db/migrate/20130602092539_create_queries_roles.rb @@ -0,0 +1,13 @@ +class CreateQueriesRoles < ActiveRecord::Migration + def self.up + create_table :queries_roles, :id => false do |t| + t.column :query_id, :integer, :null => false + t.column :role_id, :integer, :null => false + end + add_index :queries_roles, [:query_id, :role_id], :unique => true, :name => :queries_roles_ids + end + + def self.down + drop_table :queries_roles + end +end diff --git a/db/migrate/20130710182539_add_queries_visibility.rb b/db/migrate/20130710182539_add_queries_visibility.rb new file mode 100644 index 000000000..d6cd1a7b3 --- /dev/null +++ b/db/migrate/20130710182539_add_queries_visibility.rb @@ -0,0 +1,13 @@ +class AddQueriesVisibility < ActiveRecord::Migration + def up + add_column :queries, :visibility, :integer, :default => 0 + Query.where(:is_public => true).update_all(:visibility => 2) + remove_column :queries, :is_public + end + + def down + add_column :queries, :is_public, :boolean, :default => true, :null => false + Query.where('visibility <> ?', 2).update_all(:is_public => false) + remove_column :queries, :visibility + end +end diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 31e56b88e..514e33145 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -584,6 +584,8 @@ input.autocomplete.ajax-loading { background-image: url(../images/loading.gif); } +.role-visibility {padding-left:2em;} + /***** Flash & error messages ****/ #errorExplanation, div.flash, .nodata, .warning, .conflict { padding: 4px 4px 4px 30px; diff --git a/test/fixtures/queries.yml b/test/fixtures/queries.yml index f88da94d1..b92079aeb 100644 --- a/test/fixtures/queries.yml +++ b/test/fixtures/queries.yml @@ -3,7 +3,7 @@ queries_001: id: 1 type: IssueQuery project_id: 1 - is_public: true + visibility: 2 name: Multiple custom fields query filters: | --- @@ -26,7 +26,7 @@ queries_002: id: 2 type: IssueQuery project_id: 1 - is_public: false + visibility: 0 name: Private query for cookbook filters: | --- @@ -45,7 +45,7 @@ queries_003: id: 3 type: IssueQuery project_id: - is_public: false + visibility: 0 name: Private query for all projects filters: | --- @@ -60,7 +60,7 @@ queries_004: id: 4 type: IssueQuery project_id: - is_public: true + visibility: 2 name: Public query for all projects filters: | --- @@ -75,7 +75,7 @@ queries_005: id: 5 type: IssueQuery project_id: - is_public: true + visibility: 2 name: Open issues by priority and tracker filters: | --- @@ -96,7 +96,7 @@ queries_006: id: 6 type: IssueQuery project_id: - is_public: true + visibility: 2 name: Open issues grouped by tracker filters: | --- @@ -116,7 +116,7 @@ queries_007: id: 7 type: IssueQuery project_id: 2 - is_public: true + visibility: 2 name: Public query for project 2 filters: | --- @@ -131,7 +131,7 @@ queries_008: id: 8 type: IssueQuery project_id: 2 - is_public: false + visibility: 0 name: Private query for project 2 filters: | --- @@ -146,7 +146,7 @@ queries_009: id: 9 type: IssueQuery project_id: - is_public: true + visibility: 2 name: Open issues grouped by list custom field filters: | --- diff --git a/test/functional/calendars_controller_test.rb b/test/functional/calendars_controller_test.rb index dc302f456..5848e7601 100644 --- a/test/functional/calendars_controller_test.rb +++ b/test/functional/calendars_controller_test.rb @@ -34,7 +34,7 @@ class CalendarsControllerTest < ActionController::TestCase end def test_show_should_run_custom_queries - @query = IssueQuery.create!(:name => 'Calendar', :is_public => true) + @query = IssueQuery.create!(:name => 'Calendar', :visibility => IssueQuery::VISIBILITY_PUBLIC) get :show, :query_id => @query.id assert_response :success diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 707b57438..ec10af116 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -327,7 +327,7 @@ class IssuesControllerTest < ActionController::TestCase end def test_index_with_cross_project_query_in_session_should_show_project_issues - q = IssueQuery.create!(:name => "test", :user_id => 2, :is_public => false, :project => nil) + q = IssueQuery.create!(:name => "test", :user_id => 2, :visibility => IssueQuery::VISIBILITY_PRIVATE, :project => nil) @request.session[:query] = {:id => q.id, :project_id => 1} with_settings :display_subprojects_issues => '0' do @@ -341,7 +341,7 @@ class IssuesControllerTest < ActionController::TestCase end def test_private_query_should_not_be_available_to_other_users - q = IssueQuery.create!(:name => "private", :user => User.find(2), :is_public => false, :project => nil) + q = IssueQuery.create!(:name => "private", :user => User.find(2), :visibility => IssueQuery::VISIBILITY_PRIVATE, :project => nil) @request.session[:user_id] = 3 get :index, :query_id => q.id @@ -349,7 +349,7 @@ class IssuesControllerTest < ActionController::TestCase end def test_private_query_should_be_available_to_its_user - q = IssueQuery.create!(:name => "private", :user => User.find(2), :is_public => false, :project => nil) + q = IssueQuery.create!(:name => "private", :user => User.find(2), :visibility => IssueQuery::VISIBILITY_PRIVATE, :project => nil) @request.session[:user_id] = 2 get :index, :query_id => q.id @@ -357,7 +357,7 @@ class IssuesControllerTest < ActionController::TestCase end def test_public_query_should_be_available_to_other_users - q = IssueQuery.create!(:name => "private", :user => User.find(2), :is_public => true, :project => nil) + q = IssueQuery.create!(:name => "private", :user => User.find(2), :visibility => IssueQuery::VISIBILITY_PUBLIC, :project => nil) @request.session[:user_id] = 3 get :index, :query_id => q.id @@ -1151,7 +1151,7 @@ class IssuesControllerTest < ActionController::TestCase end def test_show_should_display_prev_next_links_with_saved_query_in_session - query = IssueQuery.create!(:name => 'test', :is_public => true, :user_id => 1, + query = IssueQuery.create!(:name => 'test', :visibility => IssueQuery::VISIBILITY_PUBLIC, :user_id => 1, :filters => {'status_id' => {:values => ['5'], :operator => '='}}, :sort_criteria => [['id', 'asc']]) @request.session[:query] = {:id => query.id, :project_id => nil} @@ -1243,7 +1243,7 @@ class IssuesControllerTest < ActionController::TestCase CustomValue.create!(:custom_field => cf, :customized => Issue.find(3), :value => '3') CustomValue.create!(:custom_field => cf, :customized => Issue.find(5), :value => '') - query = IssueQuery.create!(:name => 'test', :is_public => true, :user_id => 1, :filters => {}, + query = IssueQuery.create!(:name => 'test', :visibility => IssueQuery::VISIBILITY_PUBLIC, :user_id => 1, :filters => {}, :sort_criteria => [["cf_#{cf.id}", 'asc'], ['id', 'asc']]) @request.session[:query] = {:id => query.id, :project_id => nil} diff --git a/test/functional/queries_controller_test.rb b/test/functional/queries_controller_test.rb index a232fec2e..b2bbb22fd 100644 --- a/test/functional/queries_controller_test.rb +++ b/test/functional/queries_controller_test.rb @@ -35,9 +35,7 @@ class QueriesControllerTest < ActionController::TestCase 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_select 'input[name=?][value=0][checked=checked]', 'query[visibility]' assert_tag :tag => 'input', :attributes => { :type => 'checkbox', :name => 'query_is_for_all', :checked => nil, @@ -53,8 +51,7 @@ class QueriesControllerTest < ActionController::TestCase get :new assert_response :success assert_template 'new' - assert_no_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'query[is_public]' } + assert_select 'input[name=?]', 'query[visibility]', 0 assert_tag :tag => 'input', :attributes => { :type => 'checkbox', :name => 'query_is_for_all', :checked => 'checked', @@ -75,7 +72,7 @@ class QueriesControllerTest < ActionController::TestCase :f => ["status_id", "assigned_to_id"], :op => {"assigned_to_id" => "=", "status_id" => "o"}, :v => { "assigned_to_id" => ["1"], "status_id" => ["1"]}, - :query => {"name" => "test_new_project_public_query", "is_public" => "1"} + :query => {"name" => "test_new_project_public_query", "visibility" => "2"} q = Query.find_by_name('test_new_project_public_query') assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook', :query_id => q @@ -92,7 +89,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", "is_public" => "1"} + :query => {"name" => "test_new_project_private_query", "visibility" => "2"} q = Query.find_by_name('test_new_project_private_query') assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook', :query_id => q @@ -107,7 +104,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", "is_public" => "1"}, + :query => {"name" => "test_new_global_private_query", "visibility" => "2"}, :c => ["", "tracker", "subject", "priority", "category"] q = Query.find_by_name('test_new_global_private_query') @@ -140,7 +137,7 @@ class QueriesControllerTest < ActionController::TestCase :operators => {"status_id" => "o"}, :values => {"status_id" => ["1"]}, :query => {:name => "test_new_with_sort", - :is_public => "1", + :visibility => "2", :sort_criteria => {"0" => ["due_date", "desc"], "1" => ["tracker", ""]}} query = Query.find_by_name("test_new_with_sort") @@ -163,9 +160,7 @@ class QueriesControllerTest < ActionController::TestCase get :edit, :id => 4 assert_response :success assert_template 'edit' - assert_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'query[is_public]', - :checked => 'checked' } + assert_select 'input[name=?][value=2][checked=checked]', 'query[visibility]' assert_tag :tag => 'input', :attributes => { :type => 'checkbox', :name => 'query_is_for_all', :checked => 'checked', @@ -177,8 +172,7 @@ class QueriesControllerTest < ActionController::TestCase get :edit, :id => 3 assert_response :success assert_template 'edit' - assert_no_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'query[is_public]' } + assert_select 'input[name=?]', 'query[visibility]', 0 assert_tag :tag => 'input', :attributes => { :type => 'checkbox', :name => 'query_is_for_all', :checked => 'checked', @@ -190,8 +184,7 @@ class QueriesControllerTest < ActionController::TestCase get :edit, :id => 2 assert_response :success assert_template 'edit' - assert_no_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'query[is_public]' } + assert_select 'input[name=?]', 'query[visibility]', 0 assert_tag :tag => 'input', :attributes => { :type => 'checkbox', :name => 'query_is_for_all', :checked => nil, @@ -203,10 +196,7 @@ class QueriesControllerTest < ActionController::TestCase get :edit, :id => 1 assert_response :success assert_template 'edit' - assert_tag :tag => 'input', :attributes => { :type => 'checkbox', - :name => 'query[is_public]', - :checked => 'checked' - } + assert_select 'input[name=?][value=2][checked=checked]', 'query[visibility]' assert_tag :tag => 'input', :attributes => { :type => 'checkbox', :name => 'query_is_for_all', :checked => nil, @@ -240,7 +230,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_edit_global_private_query", "is_public" => "1"} + :query => {"name" => "test_edit_global_private_query", "visibility" => "2"} assert_redirected_to :controller => 'issues', :action => 'index', :query_id => 3 q = Query.find_by_name('test_edit_global_private_query') @@ -257,7 +247,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_edit_global_public_query", "is_public" => "1"} + :query => {"name" => "test_edit_global_public_query", "visibility" => "2"} assert_redirected_to :controller => 'issues', :action => 'index', :query_id => 4 q = Query.find_by_name('test_edit_global_public_query') diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 59639384f..15f1cf21c 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -28,6 +28,24 @@ class QueryTest < ActiveSupport::TestCase :projects_trackers, :custom_fields_trackers + def test_query_with_roles_visibility_should_validate_roles + set_language_if_valid 'en' + query = IssueQuery.new(:name => 'Query', :visibility => IssueQuery::VISIBILITY_ROLES) + assert !query.save + assert_include "Roles can't be blank", query.errors.full_messages + query.role_ids = [1, 2] + assert query.save + end + + def test_changing_roles_visibility_should_clear_roles + query = IssueQuery.create!(:name => 'Query', :visibility => IssueQuery::VISIBILITY_ROLES, :role_ids => [1, 2]) + assert_equal 2, query.roles.count + + query.visibility = IssueQuery::VISIBILITY_PUBLIC + query.save! + assert_equal 0, query.roles.count + end + def test_available_filters_should_be_ordered set_language_if_valid 'en' query = IssueQuery.new @@ -1089,6 +1107,54 @@ class QueryTest < ActiveSupport::TestCase assert !query_ids.include?(7), 'public query on private project was visible' end + def test_query_with_public_visibility_should_be_visible_to_anyone + q = IssueQuery.create!(:name => 'Query', :visibility => IssueQuery::VISIBILITY_PUBLIC) + + assert q.visible?(User.anonymous) + assert IssueQuery.visible(User.anonymous).find_by_id(q.id) + + assert q.visible?(User.find(7)) + assert IssueQuery.visible(User.find(7)).find_by_id(q.id) + + assert q.visible?(User.find(2)) + assert IssueQuery.visible(User.find(2)).find_by_id(q.id) + + assert q.visible?(User.find(1)) + assert IssueQuery.visible(User.find(1)).find_by_id(q.id) + end + + def test_query_with_roles_visibility_should_be_visible_to_user_with_role + q = IssueQuery.create!(:name => 'Query', :visibility => IssueQuery::VISIBILITY_ROLES, :role_ids => [1,2]) + + assert !q.visible?(User.anonymous) + assert_nil IssueQuery.visible(User.anonymous).find_by_id(q.id) + + assert !q.visible?(User.find(7)) + assert_nil IssueQuery.visible(User.find(7)).find_by_id(q.id) + + assert q.visible?(User.find(2)) + assert IssueQuery.visible(User.find(2)).find_by_id(q.id) + + assert q.visible?(User.find(1)) + assert IssueQuery.visible(User.find(1)).find_by_id(q.id) + end + + def test_query_with_private_visibility_should_be_visible_to_owner + q = IssueQuery.create!(:name => 'Query', :visibility => IssueQuery::VISIBILITY_PRIVATE, :user => User.find(7)) + + assert !q.visible?(User.anonymous) + assert_nil IssueQuery.visible(User.anonymous).find_by_id(q.id) + + assert q.visible?(User.find(7)) + assert IssueQuery.visible(User.find(7)).find_by_id(q.id) + + assert !q.visible?(User.find(2)) + assert_nil IssueQuery.visible(User.find(2)).find_by_id(q.id) + + assert q.visible?(User.find(1)) + assert_nil IssueQuery.visible(User.find(1)).find_by_id(q.id) + end + test "#available_filters should include users of visible projects in cross-project view" do users = IssueQuery.new.available_filters["assigned_to_id"] assert_not_nil users diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 4594d8e1a..52a7d64b7 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -291,7 +291,7 @@ class UserTest < ActiveSupport::TestCase end def test_destroy_should_delete_private_queries - query = Query.new(:name => 'foo', :is_public => false) + query = Query.new(:name => 'foo', :visibility => Query::VISIBILITY_PRIVATE) query.project_id = 1 query.user_id = 2 query.save! @@ -302,7 +302,7 @@ class UserTest < ActiveSupport::TestCase end def test_destroy_should_update_public_queries - query = Query.new(:name => 'foo', :is_public => true) + query = Query.new(:name => 'foo', :visibility => Query::VISIBILITY_PUBLIC) query.project_id = 1 query.user_id = 2 query.save!