Merged r23529-r23533 from trunk to 6.0-stable (#42352).

git-svn-id: https://svn.redmine.org/redmine/branches/6.0-stable@23534 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/admin_controller.rb

@@ -36,9 +36,7 @@ class AdminController < ApplicationController
   end
 
   def projects
-    retrieve_query(ProjectQuery, false, :defaults => @default_columns_names)
-    @query.admin_projects = 1
-
+    retrieve_query(ProjectAdminQuery, false, :defaults => @default_columns_names)
     @entry_count = @query.result_count
     @entry_pages = Paginator.new @entry_count, per_page_option, params['page']
     @projects = @query.results_scope(:limit => @entry_pages.per_page, :offset => @entry_pages.offset).to_a

app/controllers/queries_controller.rb

@@ -19,6 +19,8 @@
 
 class QueriesController < ApplicationController
   menu_item :issues
+  layout :query_layout
+
   before_action :find_query, :only => [:edit, :update, :destroy]
   before_action :find_optional_project, :only => [:new, :create]
 
@@ -52,7 +54,6 @@ class QueriesController < ApplicationController
     @query.user = User.current
     @query.project = @project
     @query.build_from_params(params)
-    render :layout => 'admin' if params[:admin_projects]
   end
 
   def create
@@ -63,14 +64,13 @@ class QueriesController < ApplicationController
 
     if @query.save
       flash[:notice] = l(:notice_successful_create)
-      redirect_to_items(:query_id => @query, :admin_projects => params[:admin_projects])
+      redirect_to_items(:query_id => @query)
     else
       render :action => 'new', :layout => !request.xhr?
     end
   end
 
   def edit
-    render :layout => 'admin' if params[:admin_projects]
   end
 
   def update
@@ -78,7 +78,7 @@ class QueriesController < ApplicationController
 
     if @query.save
       flash[:notice] = l(:notice_successful_update)
-      redirect_to_items(:query_id => @query, :admin_projects => params[:admin_projects])
+      redirect_to_items(:query_id => @query)
     else
       render :action => 'edit'
     end
@@ -109,18 +109,20 @@ class QueriesController < ApplicationController
   end
 
   def current_menu_item
-    @query ? @query.queried_class.to_s.underscore.pluralize.to_sym : nil
+    return unless @query
+    return if query_layout == 'admin'
+
+    @query.queried_class.to_s.underscore.pluralize.to_sym
   end
 
   def current_menu(project)
-    super if params[:admin_projects].nil?
+    super unless query_layout == 'admin'
   end
 
   private
 
   def find_query
     @query = Query.find(params[:id])
-    @query.admin_projects = params[:admin_projects] if @query.is_a?(ProjectQuery)
     @project = @query.project
     render_403 unless @query.editable_by?(User.current)
   rescue ActiveRecord::RecordNotFound
@@ -171,17 +173,25 @@ class QueriesController < ApplicationController
   end
 
   def redirect_to_project_query(options)
-    if params[:admin_projects]
-      redirect_to admin_projects_path(options)
-    else
-      redirect_to projects_path(options)
-    end
+    redirect_to projects_path(options)
+  end
+
+  def redirect_to_project_admin_query(options)
+    redirect_to admin_projects_path(options)
   end
 
   def redirect_to_user_query(options)
     redirect_to users_path(options)
   end
 
+  def query_layout
+    @query&.layout || 'base'
+  end
+
+  def menu_items
+    {self.controller_name.to_sym => {:actions => {}, :default => current_menu_item}}
+  end
+
   # Returns the Query subclass, IssueQuery by default
   # for compatibility with previous behaviour
   def query_class

app/helpers/queries_helper.rb

@@ -480,8 +480,6 @@ module QueriesHelper
     url_params =
       if controller_name == 'issues'
         {:controller => 'issues', :action => 'index', :project_id => @project}
-      elsif controller_name == 'admin' && action_name == 'projects'
-        {:admin_projects => '1'}
       else
         {}
       end

app/models/project_admin_query.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 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.
+
+class ProjectAdminQuery < ProjectQuery
+  self.layout = 'admin'
+
+  def self.default(project: nil, user: User.current)
+    nil
+  end
+
+  def self.visible(*args)
+    user = args.shift || User.current
+    if user.admin?
+      where('1=1')
+    else
+      where('1=0')
+    end
+  end
+
+  def visible?(user=User.current)
+    user&.admin?
+  end
+
+  def editable_by?(user)
+    user&.admin?
+  end
+
+  def available_display_types
+    ['list']
+  end
+
+  def display_type
+    'list'
+  end
+
+  def project_statuses_values
+    values = super
+
+    values << [l(:project_status_archived), Project::STATUS_ARCHIVED.to_s]
+    values << [l(:project_status_scheduled_for_deletion), Project::STATUS_SCHEDULED_FOR_DELETION.to_s]
+    values
+  end
+
+  def base_scope
+    Project.where(statement)
+  end
+end

app/models/project_query.rb

@@ -18,8 +18,6 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class ProjectQuery < Query
-  attr_accessor :admin_projects
-
   self.queried_class = Project
   self.view_permission = :search_project
 
@@ -28,6 +26,13 @@ class ProjectQuery < Query
     errors.add(:project_id, :exclusion) if query.project_id.present?
   end
 
+  # Inheriting ProjectAdminQuery from ProjectQuery introduces the problem that
+  # ProjectQuery.visible also yields ProjectAdminQueries, as
+  # well. We fix that by adding a condition on the actual class name.
+  def self.visible(*)
+    super.where type: name
+  end
+
   self.available_columns = [
     QueryColumn.new(:name, :sortable => "#{Project.table_name}.name"),
     QueryColumn.new(:status, :sortable => "#{Project.table_name}.status"),
@@ -83,12 +88,6 @@ class ProjectQuery < Query
     add_custom_fields_filters(project_custom_fields)
   end
 
-  def build_from_params(params, defaults={})
-    query = super
-    query.admin_projects = params[:admin_projects]
-    query
-  end
-
   def available_columns
     return @available_columns if @available_columns
 
@@ -99,28 +98,7 @@ class ProjectQuery < Query
   end
 
   def available_display_types
-    if self.admin_projects
-      ['list']
-    else
-      ['board', 'list']
-    end
-  end
-
-  def display_type
-    if self.admin_projects
-      'list'
-    else
-      super
-    end
-  end
-
-  def project_statuses_values
-    values = super
-    if self.admin_projects
-      values << [l(:project_status_archived), Project::STATUS_ARCHIVED.to_s]
-      values << [l(:project_status_scheduled_for_deletion), Project::STATUS_SCHEDULED_FOR_DELETION.to_s]
-    end
-    values
+    ['board', 'list']
   end
 
   def default_columns_names
@@ -136,11 +114,7 @@ class ProjectQuery < Query
   end
 
   def base_scope
-    if self.admin_projects
-      Project.where(statement)
-    else
-      Project.visible.where(statement)
-    end
+    Project.visible.where(statement)
   end
 
   # Returns the project count

app/models/query.rb

@@ -355,6 +355,8 @@ class Query < ApplicationRecord
   # Permission required to view the queries, set on subclasses.
   class_attribute :view_permission
 
+  class_attribute :layout, default: 'base'
+
   # Scope of queries that are global or on the given project
   scope :global_or_on_project, (lambda do |project|
     where(:project_id => (project.nil? ? nil : [nil, project.id]))
@@ -1004,7 +1006,7 @@ class Query < ApplicationRecord
         end
       end
 
-      if field == 'project_id' || (self.type == 'ProjectQuery' && %w[id parent_id].include?(field))
+      if field == 'project_id' || (is_a?(ProjectQuery) && %w[id parent_id].include?(field))
         if v.delete('mine')
           v += User.current.memberships.pluck(:project_id).map(&:to_s)
         end

app/models/user_query.rb

@@ -17,6 +17,7 @@
 # along with this program; if not, write to the Free Software
 
 class UserQuery < Query
+  self.layout = 'admin'
   self.queried_class = Principal # must be Principal (not User) for custom field filters to work
 
   self.available_columns = [
@@ -33,6 +34,15 @@ class UserQuery < Query
     QueryAssociationColumn.new(:auth_source, :name, caption: :field_auth_source, sortable: "#{AuthSource.table_name}.name")
   ]
 
+  def self.visible(*args)
+    user = args.shift || User.current
+    if user.admin?
+      where('1=1')
+    else
+      where('1=0')
+    end
+  end
+
   def initialize(attributes=nil, *args)
     super(attributes)
     self.filters ||= { 'status' => {operator: "=", values: [User::STATUS_ACTIVE]} }
@@ -64,6 +74,14 @@ class UserQuery < Query
     add_custom_fields_filters(user_custom_fields)
   end
 
+  def visible?(user=User.current)
+    user&.admin?
+  end
+
+  def editable_by?(user)
+    user&.admin?
+  end
+
   def auth_sources_values
     AuthSource.order(name: :asc).pluck(:name, :id)
   end

app/views/admin/projects.html.erb

@@ -6,7 +6,6 @@
 <%= @query.persisted? && @query.description.present? ? content_tag('p', @query.description, class: 'subtitle') : '' %>
 
 <%= form_tag(admin_projects_path(@project, nil), :method => :get, :id => 'query_form') do %>
-<%= hidden_field_tag 'admin_projects', '1' %>
 <%= render :partial => 'queries/query_form' %>
 <% end %>
 
@@ -19,5 +18,6 @@
 <% end %>
 
 <% content_for :sidebar do %>
-  <%= render :partial => 'projects/sidebar' %>
+  <%= render_sidebar_queries(ProjectAdminQuery, @project) %>
+  <%= call_hook(:view_admin_projects_sidebar_queries_bottom) %>
 <% end %>

app/views/queries/_form.html.erb

@@ -4,7 +4,6 @@
 <div class="tabular">
 <%= hidden_field_tag 'gantt', '1' if params[:gantt] %>
 <%= hidden_field_tag 'calendar', '1' if params[:calendar] %>
-<%= hidden_field_tag 'admin_projects', '1' if params[:admin_projects] %>
 
 <p>
   <label for="query_name">
@@ -21,7 +20,7 @@
 <p><label><%=l(:field_visible)%></label>
   <label class="block"><%= radio_button 'query', 'visibility', Query::VISIBILITY_PRIVATE %> <%= l(:label_visibility_private) %></label>
   <label class="block"><%= radio_button 'query', 'visibility', Query::VISIBILITY_PUBLIC %> <%= l(:label_visibility_public) %></label>
-  <% unless @query.type == 'ProjectQuery' %>
+  <% unless @query.is_a?(ProjectQuery) %>
     <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>
@@ -31,7 +30,7 @@
 </p>
 <% end %>
 
-<% unless @query.type == 'ProjectQuery' %>
+<% unless @query.is_a?(ProjectQuery) %>
   <p><label for="query_is_for_all"><%=l(:field_is_for_all)%></label>
   <%= check_box_tag 'query_is_for_all', 1, @query.project.nil?, :class => (User.current.admin? ? '' : 'disable-unless-private') %></p>
 <% end %>

app/views/queries/_query_form.html.erb

@@ -69,9 +69,8 @@
     <% end %>
   <% else %>
     <% if @query.editable_by?(User.current) %>
-      <% redirect_params = (controller_name == 'admin' && action_name == 'projects') ? {:admin_projects => 1} : {} %>
-      <%= link_to sprite_icon('edit', l(:button_edit_object, object_name: l(:label_query)).capitalize), edit_query_path(@query, redirect_params), :class => 'icon icon-edit' %>
-      <%= delete_link query_path(@query, redirect_params), {}, l(:button_delete_object, object_name: l(:label_query)).capitalize %>
+      <%= link_to sprite_icon('edit', l(:button_edit_object, object_name: l(:label_query)).capitalize), edit_query_path(@query), :class => 'icon icon-edit' %>
+      <%= delete_link query_path(@query), {}, l(:button_delete_object, object_name: l(:label_query)).capitalize %>
     <% end %>
   <% end %>
 </p>

test/functional/queries_controller_test.rb

@@ -600,11 +600,11 @@ class QueriesControllerTest < Redmine::ControllerTest
   def test_create_admin_projects_query_should_redirect_to_admin_projects
     @request.session[:user_id] = 1
 
-    q = new_record(ProjectQuery) do
+    q = new_record(ProjectAdminQuery) do
       post(
         :create,
         :params => {
-          :type => 'ProjectQuery',
+          :type => 'ProjectAdminQuery',
           :default_columns => '1',
           :f => ["status"],
           :op => {
@@ -615,13 +615,12 @@ class QueriesControllerTest < Redmine::ControllerTest
           },
           :query => {
             "name" => "test_new_project_public_query", "visibility" => "2"
-          },
-          :admin_projects => 1
+          }
         }
       )
     end
 
-    assert_redirected_to :controller => 'admin', :action => 'projects', :query_id => q.id, :admin_projects => 1
+    assert_redirected_to :controller => 'admin', :action => 'projects', :query_id => q.id
   end
 
   def test_edit_global_public_query
@@ -738,7 +737,7 @@ class QueriesControllerTest < Redmine::ControllerTest
   end
 
   def test_update_admin_projects_query
-    q = ProjectQuery.create(:name => 'project_query')
+    q = ProjectAdminQuery.create(:name => 'project_query')
     @request.session[:user_id] = 1
 
     put(
@@ -755,12 +754,11 @@ class QueriesControllerTest < Redmine::ControllerTest
         },
         :query => {
           "name" => "test_project_query_updated", "visibility" => "2"
-        },
-        :admin_projects => 1
+        }
       }
     )
 
-    assert_redirected_to :controller => 'admin', :action => 'projects', :query_id => q.id, :admin_projects => 1
+    assert_redirected_to :controller => 'admin', :action => 'projects', :query_id => q.id
     assert Query.find_by_name('test_project_query_updated')
   end
 

test/unit/project_admin_query_test.rb

@@ -0,0 +1,137 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 2006-  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_relative '../test_helper'
+
+class ProjectAdminQueryTest < ActiveSupport::TestCase
+  include Redmine::I18n
+
+  def test_filter_values_be_arrays
+    q = ProjectAdminQuery.new
+    assert_nil q.project
+
+    q.available_filters.each do |name, filter|
+      values = filter.values
+      assert (values.nil? || values.is_a?(Array)),
+             "#values for #{name} filter returned a #{values.class.name}"
+    end
+  end
+
+  def test_project_statuses_filter_should_return_project_statuses
+    set_language_if_valid 'en'
+    query = ProjectAdminQuery.new(:name => '_')
+    query.filters = {'status' => {:operator => '=', :values => []}}
+    values = query.available_filters['status'][:values]
+    assert_equal ['active', 'closed', 'archived', 'scheduled for deletion'], values.map(&:first)
+    assert_equal ['1', '5', '9', '10'], values.map(&:second)
+  end
+
+  def test_default_columns
+    q = ProjectAdminQuery.new
+    assert q.columns.any?
+    assert q.inline_columns.any?
+    assert q.block_columns.empty?
+  end
+
+  def test_available_columns_should_include_project_custom_fields
+    query = ProjectAdminQuery.new
+    assert_include :cf_3, query.available_columns.map(&:name)
+  end
+
+  def test_available_display_types_should_always_returns_list
+    query = ProjectAdminQuery.new
+    assert_equal ['list'], query.available_display_types
+  end
+
+  def test_display_type_should_returns_list
+    ProjectAdminQuery.new.available_display_types.each do |t|
+      with_settings :project_list_display_type => t do
+        q = ProjectAdminQuery.new
+        assert_equal 'list', q.display_type
+      end
+    end
+  end
+
+  def test_no_default_project_admin_query
+    user = User.find(1)
+    query = ProjectQuery.find(11)
+    user_query = ProjectQuery.find(12)
+    user_query.update(visibility: Query::VISIBILITY_PUBLIC)
+
+    [nil, user, User.anonymous].each do |u|
+      assert_nil ProjectAdminQuery.default(user: u)
+    end
+
+    # ignore the default_project_query for admin queries
+    with_settings :default_project_query => query.id do
+      [nil, user, User.anonymous].each do |u|
+        assert_nil ProjectAdminQuery.default(user: u)
+      end
+    end
+
+    # user default, overrides global default
+    user.pref.default_project_query = user_query.id
+    user.pref.save
+
+    with_settings :default_project_query => query.id do
+      assert_nil ProjectAdminQuery.default(user: user)
+    end
+  end
+
+  def test_project_statuses_values_should_return_all_statuses
+    q = ProjectAdminQuery.new
+    assert_equal [
+      ["active", "1"],
+      ["closed", "5"],
+      ["archived", "9"],
+      ["scheduled for deletion", "10"]
+    ], q.project_statuses_values
+  end
+
+  def test_base_scope_should_return_all_projects
+    q = ProjectAdminQuery.new
+    assert_equal Project.all, q.base_scope
+  end
+
+  def test_results_scope_has_last_activity_date
+    q = ProjectAdminQuery.generate!(column_names: [:last_activity_date])
+    result_projects = q.results_scope({})
+
+    assert_kind_of ActiveRecord::Relation, result_projects
+    assert_equal Project, result_projects.klass
+
+    last_activitiy_date = result_projects.find{|p| p.id == 1}.instance_variable_get(:@last_activity_date)
+    assert_not_nil last_activitiy_date
+    assert_equal Redmine::Activity::Fetcher.new(User.current).events(nil, nil, :project => Project.find(1)).first.updated_on, last_activitiy_date
+  end
+
+  def test_results_scope_with_offset_and_limit
+    q = ProjectAdminQuery.new
+
+    ((q.results_scope.count / 2) + 1).times do |i|
+      limit = 2
+      offset = i * 2
+
+      scope_without = q.results_scope.offset(offset).limit(limit).ids
+      scope_with = q.results_scope(:offset => offset, :limit => limit).ids
+
+      assert_equal scope_without, scope_with
+    end
+  end
+end

test/unit/project_query_test.rb

@@ -56,16 +56,9 @@ class ProjectQueryTest < ActiveSupport::TestCase
 
   def test_available_display_types_should_returns_bord_and_list
     query = ProjectQuery.new
-    query.admin_projects = nil
     assert_equal ['board', 'list'], query.available_display_types
   end
 
-  def test_available_display_types_should_always_returns_list_when_admin_projects_is_set
-    query = ProjectQuery.new
-    query.admin_projects = 1
-    assert_equal ['list'], query.available_display_types
-  end
-
   def test_display_type_default_should_equal_with_setting_project_list_display_type
     ProjectQuery.new.available_display_types.each do |t|
       with_settings :project_list_display_type => t do
@@ -81,8 +74,10 @@ class ProjectQueryTest < ActiveSupport::TestCase
     user_query = ProjectQuery.find(12)
     user_query.update(visibility: Query::VISIBILITY_PUBLIC)
 
-    [nil, user, User.anonymous].each do |u|
-      assert_nil IssueQuery.default(user: u)
+    with_settings :default_project_query => nil do
+      [nil, user, User.anonymous].each do |u|
+        assert_nil ProjectQuery.default(user: u)
+      end
     end
 
     # only global default is set
@@ -110,38 +105,17 @@ class ProjectQueryTest < ActiveSupport::TestCase
     assert_nil ProjectQuery.default
   end
 
-  def test_display_type_should_returns_list_when_admin_projects_is_set
-    q = ProjectQuery.new
-    q.admin_projects = 1
-    assert_equal 'list', q.display_type
-  end
-
   def test_project_statuses_values_should_equal_ancestors_return
     ancestor = Query.new
     q = ProjectQuery.new
     assert_equal ancestor.project_statuses_values, q.project_statuses_values
   end
 
-  def test_project_statuses_values_should_includes_project_status_archeved_when_admin_projects_is_set
-    q = ProjectQuery.new
-    q.admin_projects = 1
-    assert_includes q.project_statuses_values, [l(:project_status_archived), Project::STATUS_ARCHIVED.to_s]
-    Query.new.project_statuses_values.each do |status|
-      assert_includes q.project_statuses_values, status
-    end
-  end
-
   def test_base_scope_should_return_visible_projects
     q = ProjectQuery.new
     assert_equal Project.visible, q.base_scope
   end
 
-  def test_base_scope_should_return_all_projects_when_admin_projects_is_set
-    q = ProjectQuery.new
-    q.admin_projects = 1
-    assert_equal Project.all, q.base_scope
-  end
-
   def test_results_scope_has_last_activity_date
     q = ProjectQuery.generate!(column_names: [:last_activity_date])
     result_projects = q.results_scope({})

test/unit/user_query_test.rb

@@ -209,6 +209,30 @@ class UserQueryTest < ActiveSupport::TestCase
     assert_equal [2, 1], users.pluck(:id)
   end
 
+  def test_user_query_is_only_visible_to_admins
+    q = UserQuery.new(name: '_')
+    assert q.save
+
+    admin = User.admin(true).first
+    user = User.admin(false).first
+
+    assert q.visible?(admin)
+    assert_include q, UserQuery.visible(admin).to_a
+
+    assert_not q.visible?(user)
+    assert_not_include q, UserQuery.visible(user)
+  end
+
+  def test_user_query_is_only_editable_by_admins
+    q = UserQuery.new(name: '_')
+
+    admin = User.admin(true).first
+    user = User.admin(false).first
+
+    assert q.editable_by?(admin)
+    assert_not q.editable_by?(user)
+  end
+
   def find_users_with_query(query)
     User.where(query.statement).to_a
   end