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

@@ -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]
 

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_my_queries), sidebar_queries.select(&:is_private?))
-    out << query_links(l(:label_query_plural), sidebar_queries.select(&:is_public?))
+    out << query_links(l(:label_query_plural), sidebar_queries.reject(&:is_private?))
     out
   end
 

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

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

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]

app/views/queries/_form.html.erb

@@ -6,15 +6,22 @@
 <%= text_field 'query', 'name', :size => 80 %></p>
 
 <% if User.current.admin? || User.current.allowed_to?(:manage_public_queries, @project) %>
-<p><label for="query_is_public"><%=l(:field_is_public)%></label>
+<p><label><%=l(:field_visible)%></label>
-<%= check_box 'query', 'is_public',
+  <label class="block"><%= radio_button 'query', 'visibility', Query::VISIBILITY_PRIVATE %> <%= l(:label_visibility_private) %></label>
-      :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");}') %></p>
+  <label class="block"><%= radio_button 'query', 'visibility', Query::VISIBILITY_ROLES %> <%= l(:label_visibility_roles) %>:</label>
+  <% Role.givable.sorted.each do |role| %>
+    <label class="block role-visibility"><%= check_box_tag 'query[role_ids][]', role.id, @query.roles.include?(role), :id => nil %> <%= role.name %></label>
+  <% end %>
+  <label class="block"><%= radio_button 'query', 'visibility', Query::VISIBILITY_PUBLIC %> <%= l(:label_visibility_public) %></label>
+  <%= hidden_field_tag 'query[role_ids][]', '' %>
+</p>
 <% end %>
 
 <p><label for="query_is_for_all"><%=l(:field_is_for_all)%></label>
 <%= check_box_tag 'query_is_for_all', 1, @query.project.nil?,
       :disabled => (!@query.new_record? && (@query.project.nil? || (@query.is_public? && !User.current.admin?))) %></p>
 
+<fieldset><legend><%= l(:label_options) %></legend>
 <p><label for="query_default_columns"><%=l(:label_default_columns)%></label>
 <%= check_box_tag 'default_columns', 1, @query.has_default_columns?, :id => 'query_default_columns',
       :onclick => 'if (this.checked) {$("#columns").hide();} else {$("#columns").show();}' %></p>
@@ -24,6 +31,7 @@
 
 <p><label><%= l(:button_show) %></label>
 <%= available_block_columns_tags(@query) %></p>
+</fieldset>
 </div>
 
 <fieldset id="filters"><legend><%= l(:label_filter_plural) %></legend>
@@ -53,3 +61,12 @@
 <% end %>
 
 </div>
+
+<%= 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 %>

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

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

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

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

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

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;

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: |
     --- 

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

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}
 

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',
+    assert_select 'input[name=?][value=0][checked=checked]', 'query[visibility]'
-                                                 :name => 'query[is_public]',
-                                                 :checked => nil }
     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',
+    assert_select 'input[name=?]', 'query[visibility]', 0
-                                                    :name => 'query[is_public]' }
     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',
+    assert_select 'input[name=?][value=2][checked=checked]', 'query[visibility]'
-                                                 :name => 'query[is_public]',
-                                                 :checked => 'checked' }
     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',
+    assert_select 'input[name=?]', 'query[visibility]', 0
-                                                    :name => 'query[is_public]' }
     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',
+    assert_select 'input[name=?]', 'query[visibility]', 0
-                                                    :name => 'query[is_public]' }
     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',
+    assert_select 'input[name=?][value=2][checked=checked]', 'query[visibility]'
-                                                 :name => 'query[is_public]',
-                                                 :checked => 'checked'
-                                                  }
     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')

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

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!