Don't preload all query filters (#24787).

git-svn-id: http://svn.redmine.org/redmine/trunk@16170 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/queries_controller.rb

@@ -17,7 +17,7 @@
 
 class QueriesController < ApplicationController
   menu_item :issues
-  before_action :find_query, :except => [:new, :create, :index]
+  before_action :find_query, :only => [:edit, :update, :destroy]
   before_action :find_optional_project, :only => [:new, :create]
 
   accept_api_auth :index
@@ -85,6 +85,25 @@ class QueriesController < ApplicationController
     redirect_to_items(:set_filter => 1)
   end
 
+  # Returns the values for a query filter
+  def filter
+    q = query_class.new
+    if params[:project_id].present?
+      q.project = Project.find(params[:project_id])
+    end
+
+    unless User.current.allowed_to?(q.class.view_permission, q.project, :global => true)
+      raise Unauthorized
+    end
+
+    filter = q.available_filters[params[:name].to_s]
+    values = filter ? filter.values : []
+
+    render :json => values
+  rescue ActiveRecord::RecordNotFound
+    render_404
+  end
+
   private
 
   def find_query

app/models/issue_query.rb

@@ -78,80 +78,37 @@ class IssueQuery < Query
   end
 
   def initialize_available_filters
-    principals = []
-    subprojects = []
-    versions = []
-    categories = []
-    issue_custom_fields = []
-
-    if project
-      principals += project.principals.visible
-      unless project.leaf?
-        subprojects = project.descendants.visible.to_a
-        principals += Principal.member_of(subprojects).visible
-      end
-      versions = project.shared_versions.to_a
-      categories = project.issue_categories.to_a
-      issue_custom_fields = project.all_issue_custom_fields
-    else
-      if all_projects.any?
-        principals += Principal.member_of(all_projects).visible
-      end
-      versions = Version.visible.where(:sharing => 'system').to_a
-      issue_custom_fields = IssueCustomField.where(:is_for_all => true)
-    end
-    principals.uniq!
-    principals.sort!
-    principals.reject! {|p| p.is_a?(GroupBuiltin)}
-    users = principals.select {|p| p.is_a?(User)}
-
     add_available_filter "status_id",
-      :type => :list_status, :values => IssueStatus.sorted.collect{|s| [s.name, s.id.to_s] }
+      :type => :list_status, :values => lambda { IssueStatus.sorted.collect{|s| [s.name, s.id.to_s] } }
 
-    if project.nil?
-      project_values = []
-      if User.current.logged? && User.current.memberships.any?
-        project_values << ["<< #{l(:label_my_projects).downcase} >>", "mine"]
-      end
-      project_values += all_projects_values
-      add_available_filter("project_id",
-        :type => :list, :values => project_values
-      ) unless project_values.empty?
-    end
+    add_available_filter("project_id",
+      :type => :list, :values => lambda { project_values }
+    ) if project.nil?
 
     add_available_filter "tracker_id",
       :type => :list, :values => trackers.collect{|s| [s.name, s.id.to_s] }
+
     add_available_filter "priority_id",
       :type => :list, :values => IssuePriority.all.collect{|s| [s.name, s.id.to_s] }
 
-    author_values = []
-    author_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged?
-    author_values += users.collect{|s| [s.name, s.id.to_s] }
     add_available_filter("author_id",
-      :type => :list, :values => author_values
-    ) unless author_values.empty?
+      :type => :list, :values => lambda { author_values }
+    )
 
-    assigned_to_values = []
-    assigned_to_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged?
-    assigned_to_values += (Setting.issue_group_assignment? ?
-                              principals : users).collect{|s| [s.name, s.id.to_s] }
     add_available_filter("assigned_to_id",
-      :type => :list_optional, :values => assigned_to_values
-    ) unless assigned_to_values.empty?
+      :type => :list_optional, :values => lambda { assigned_to_values }
+    )
 
-    group_values = Group.givable.visible.collect {|g| [g.name, g.id.to_s] }
     add_available_filter("member_of_group",
-      :type => :list_optional, :values => group_values
-    ) unless group_values.empty?
+      :type => :list_optional, :values => lambda { Group.givable.visible.collect {|g| [g.name, g.id.to_s] } }
+    )
 
-    role_values = Role.givable.collect {|r| [r.name, r.id.to_s] }
     add_available_filter("assigned_to_role",
-      :type => :list_optional, :values => role_values
-    ) unless role_values.empty?
+      :type => :list_optional, :values => lambda { Role.givable.collect {|r| [r.name, r.id.to_s] } }
+    )
 
     add_available_filter "fixed_version_id",
-      :type => :list_optional,
-      :values => Version.sort_by_status(versions).collect{|s| ["#{s.project.name} - #{s.name}", s.id.to_s, l("version_status_#{s.status}")] }
+      :type => :list_optional, :values => lambda { fixed_version_values }
 
     add_available_filter "fixed_version.due_date",
       :type => :date,
@@ -164,7 +121,7 @@ class IssueQuery < Query
 
     add_available_filter "category_id",
       :type => :list_optional,
-      :values => categories.collect{|s| [s.name, s.id.to_s] }
+      :values => lambda { project.issue_categories.collect{|s| [s.name, s.id.to_s] } } if project
 
     add_available_filter "subject", :type => :text
     add_available_filter "description", :type => :text
@@ -188,18 +145,20 @@ class IssueQuery < Query
         :type => :list, :values => [["<< #{l(:label_me)} >>", "me"]]
     end
 
-    if subprojects.any?
+    if project && !project.leaf?
       add_available_filter "subproject_id",
         :type => :list_subprojects,
-        :values => subprojects.collect{|s| [s.name, s.id.to_s] }
+        :values => lambda { subproject_values }
     end
 
+
+    issue_custom_fields = project ? project.all_issue_custom_fields : IssueCustomField.where(:is_for_all => true)
     add_custom_fields_filters(issue_custom_fields)
 
     add_associations_custom_fields_filters :project, :author, :assigned_to, :fixed_version
 
     IssueRelation::TYPES.each do |relation_type, options|
-      add_available_filter relation_type, :type => :relation, :label => options[:name]
+      add_available_filter relation_type, :type => :relation, :label => options[:name], :values => lambda {all_projects_values}
     end
     add_available_filter "parent_id", :type => :tree, :label => :field_parent_issue
     add_available_filter "child_id", :type => :tree, :label => :label_subtask_plural

app/models/query.rb

@@ -160,6 +160,40 @@ class QueryAssociationCustomFieldColumn < QueryCustomFieldColumn
   end
 end
 
+class QueryFilter
+  include Redmine::I18n
+
+  def initialize(field, options)
+    @field = field.to_s
+    @options = options
+    @options[:name] ||= l(options[:label] || "field_#{field}".gsub(/_id$/, ''))
+    # Consider filters with a Proc for values as remote by default
+    @remote = options.key?(:remote) ? options[:remote] : options[:values].is_a?(Proc)
+  end
+
+  def [](arg)
+    if arg == :values
+      values
+    else
+      @options[arg]
+    end
+  end
+
+  def values
+    @values ||= begin
+      values = @options[:values]
+      if values.is_a?(Proc)
+        values = values.call
+      end
+      values
+    end
+  end
+
+  def remote
+    @remote
+  end
+end
+
 class Query < ActiveRecord::Base
   class StatementInvalid < ::ActiveRecord::StatementInvalid
   end
@@ -404,12 +438,17 @@ class Query < ActiveRecord::Base
   # Returns a representation of the available filters for JSON serialization
   def available_filters_as_json
     json = {}
-    available_filters.each do |field, options|
-      options = options.slice(:type, :name, :values)
-      if options[:values] && values_for(field)
-        missing = Array(values_for(field)).select(&:present?) - options[:values].map(&:last)
-        if missing.any? && respond_to?(method = "find_#{field}_filter_values")
-          options[:values] += send(method, missing)
+    available_filters.each do |field, filter|
+      options = {:type => filter[:type], :name => filter[:name]}
+      options[:remote] = true if filter.remote
+
+      if has_filter?(field) || !filter.remote
+        options[:values] = filter.values
+        if options[:values] && values_for(field)
+          missing = Array(values_for(field)).select(&:present?) - options[:values].map(&:last)
+          if missing.any? && respond_to?(method = "find_#{field}_filter_values")
+            options[:values] += send(method, missing)
+          end
         end
       end
       json[field] = options.stringify_keys
@@ -432,6 +471,65 @@ class Query < ActiveRecord::Base
     @all_projects_values = values
   end
 
+  def project_values
+    project_values = []
+    if User.current.logged? && User.current.memberships.any?
+      project_values << ["<< #{l(:label_my_projects).downcase} >>", "mine"]
+    end
+    project_values += all_projects_values
+    project_values
+  end
+
+  def subproject_values
+    project.descendants.visible.collect{|s| [s.name, s.id.to_s] }
+  end
+
+  def principals
+    @principal ||= begin
+      principals = []
+      if project
+        principals += project.principals.visible
+        unless project.leaf?
+          principals += Principal.member_of(project.descendants.visible).visible
+        end
+      else
+        principals += Principal.member_of(all_projects).visible
+      end
+      principals.uniq!
+      principals.sort!
+      principals.reject! {|p| p.is_a?(GroupBuiltin)}
+      principals
+    end
+  end
+
+  def users
+    principals.select {|p| p.is_a?(User)}
+  end
+
+  def author_values
+    author_values = []
+    author_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged?
+    author_values += users.collect{|s| [s.name, s.id.to_s] }
+    author_values
+  end
+
+  def assigned_to_values
+    assigned_to_values = []
+    assigned_to_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged?
+    assigned_to_values += (Setting.issue_group_assignment? ? principals : users).collect{|s| [s.name, s.id.to_s] }
+    assigned_to_values
+  end
+
+  def fixed_version_values
+    versions = []
+    if project
+      versions = project.shared_versions.to_a
+    else
+      versions = Version.visible.where(:sharing => 'system').to_a
+    end
+    Version.sort_by_status(versions).collect{|s| ["#{s.project.name} - #{s.name}", s.id.to_s, l("version_status_#{s.status}")] }
+  end
+
   # Adds available filters
   def initialize_available_filters
     # implemented by sub-classes
@@ -441,7 +539,7 @@ class Query < ActiveRecord::Base
   # Adds an available filter
   def add_available_filter(field, options)
     @available_filters ||= ActiveSupport::OrderedHash.new
-    @available_filters[field] = options
+    @available_filters[field] = QueryFilter.new(field, options)
     @available_filters
   end
 
@@ -457,9 +555,6 @@ class Query < ActiveRecord::Base
     unless @available_filters
       initialize_available_filters
       @available_filters ||= {}
-      @available_filters.each do |field, options|
-        options[:name] ||= l(options[:label] || "field_#{field}".gsub(/_id$/, ''))
-      end
     end
     @available_filters
   end

app/models/time_entry_query.rb

@@ -42,65 +42,38 @@ class TimeEntryQuery < Query
   def initialize_available_filters
     add_available_filter "spent_on", :type => :date_past
 
-    principals = []
-    versions = []
-    if project
-      principals += project.principals.visible.sort
-      unless project.leaf?
-        subprojects = project.descendants.visible.to_a
-        if subprojects.any?
-          add_available_filter "subproject_id",
-            :type => :list_subprojects,
-            :values => subprojects.collect{|s| [s.name, s.id.to_s] }
-          principals += Principal.member_of(subprojects).visible
-        end
-      end
-      versions = project.shared_versions.to_a
-    else
-      if all_projects.any?
-        # members of visible projects
-        principals += Principal.member_of(all_projects).visible
-        # project filter
-        project_values = []
-        if User.current.logged? && User.current.memberships.any?
-          project_values << ["<< #{l(:label_my_projects).downcase} >>", "mine"]
-        end
-        project_values += all_projects_values
-        add_available_filter("project_id",
-          :type => :list, :values => project_values
-        ) unless project_values.empty?
-      end
+    add_available_filter("project_id",
+      :type => :list, :values => lambda { project_values }
+    ) if project.nil?
+
+    if project && !project.leaf?
+      add_available_filter "subproject_id",
+        :type => :list_subprojects,
+        :values => lambda { subproject_values }
     end
 
     add_available_filter("issue_id", :type => :tree, :label => :label_issue)
     add_available_filter("issue.tracker_id",
       :type => :list,
       :name => l("label_attribute_of_issue", :name => l(:field_tracker)),
-      :values => Tracker.sorted.map {|t| [t.name, t.id.to_s]})
+      :values => lambda { Tracker.sorted.map {|t| [t.name, t.id.to_s]} })
     add_available_filter("issue.status_id",
       :type => :list,
       :name => l("label_attribute_of_issue", :name => l(:field_status)),
-      :values => IssueStatus.sorted.map {|s| [s.name, s.id.to_s]})
+      :values => lambda { IssueStatus.sorted.map {|s| [s.name, s.id.to_s]} })
     add_available_filter("issue.fixed_version_id",
       :type => :list,
       :name => l("label_attribute_of_issue", :name => l(:field_fixed_version)),
-      :values => Version.sort_by_status(versions).collect{|s| ["#{s.project.name} - #{s.name}", s.id.to_s, l("version_status_#{s.status}")] })
+      :values => lambda { fixed_version_values }) if project
 
-    principals.uniq!
-    principals.sort!
-    users = principals.select {|p| p.is_a?(User)}
-
-    users_values = []
-    users_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged?
-    users_values += users.collect{|s| [s.name, s.id.to_s] }
     add_available_filter("user_id",
-      :type => :list_optional, :values => users_values
-    ) unless users_values.empty?
+      :type => :list_optional, :values => lambda { author_values }
+    )
 
     activities = (project ? project.activities : TimeEntryActivity.shared)
     add_available_filter("activity_id",
       :type => :list, :values => activities.map {|a| [a.name, a.id.to_s]}
-    ) unless activities.empty?
+    )
 
     add_available_filter "comments", :type => :text
     add_available_filter "hours", :type => :float

app/views/queries/_filters.html.erb

@@ -3,7 +3,9 @@ var operatorLabels = <%= raw_json Query.operators_labels %>;
 var operatorByType = <%= raw_json Query.operators_by_filter_type %>;
 var availableFilters = <%= raw_json query.available_filters_as_json %>;
 var labelDayPlural = <%= raw_json l(:label_day_plural) %>;
-var allProjects = <%= raw_json query.all_projects_values %>;
+
+var filtersUrl = <%= raw_json queries_filter_path(:project_id => @query.project.try(:id), :type => @query.type) %>;
+
 $(document).ready(function(){
   initFilters();
   <% query.filters.each do |field, options| %>

config/routes.rb

@@ -199,6 +199,7 @@ Rails.application.routes.draw do
   match '/issues', :controller => 'issues', :action => 'destroy', :via => :delete
 
   resources :queries, :except => [:show]
+  get '/queries/filter', :to => 'queries#filter', :as => 'queries_filter'
 
   resources :news, :only => [:index, :show, :edit, :update, :destroy]
   match '/news/:id/comments', :to => 'comments#create', :via => :post

public/javascripts/application.js

@@ -120,6 +120,18 @@ function initFilters() {
 function addFilter(field, operator, values) {
   var fieldId = field.replace('.', '_');
   var tr = $('#tr_'+fieldId);
+  
+  var filterOptions = availableFilters[field];
+  if (!filterOptions) return;
+
+  if (filterOptions['remote'] && filterOptions['values'] == null) {
+    $.getJSON(filtersUrl, {'name': field}).done(function(data) {
+      filterOptions['values'] = data;
+      addFilter(field, operator, values) ;
+    });
+    return;
+  }
+
   if (tr.length > 0) {
     tr.show();
   } else {
@@ -134,7 +146,7 @@ function addFilter(field, operator, values) {
   });
 }
 
-function buildFilterRow(field, operator, values) {
+function buildFilterRow(field, operator, values, loadedValues) {
   var fieldId = field.replace('.', '_');
   var filterTable = $("#filters-table");
   var filterOptions = availableFilters[field];
@@ -212,8 +224,8 @@ function buildFilterRow(field, operator, values) {
     );
     $('#values_'+fieldId).val(values[0]);
     select = tr.find('td.values select');
-    for (i = 0; i < allProjects.length; i++) {
-      var filterValue = allProjects[i];
+    for (i = 0; i < filterValues.length; i++) {
+      var filterValue = filterValues[i];
       var option = $('<option>');
       option.val(filterValue[1]).text(filterValue[0]);
       if (values[0] == filterValue[1]) { option.attr('selected', true); }

test/functional/queries_controller_test.rb

@@ -18,7 +18,12 @@
 require File.expand_path('../../test_helper', __FILE__)
 
 class QueriesControllerTest < Redmine::ControllerTest
-  fixtures :projects, :users, :members, :member_roles, :roles, :trackers, :issue_statuses, :issue_categories, :enumerations, :issues, :custom_fields, :custom_values, :queries, :enabled_modules
+  fixtures :projects, :enabled_modules,
+           :users, :email_addresses,
+           :members, :member_roles, :roles,
+           :trackers, :issue_statuses, :issue_categories, :enumerations, :versions,
+           :issues, :custom_fields, :custom_values,
+           :queries
 
   def setup
     User.current = nil
@@ -397,4 +402,24 @@ class QueriesControllerTest < Redmine::ControllerTest
     assert_response :success
     assert_include 'addFilter("subject", "=", ["foo\/bar"]);', response.body
   end
+
+  def test_filter_with_project_id_should_return_filter_values
+    @request.session[:user_id] = 2
+    get :filter, :project_id => 1, :name => 'fixed_version_id'
+
+    assert_response :success
+    assert_equal 'application/json', response.content_type
+    json = ActiveSupport::JSON.decode(response.body)
+    assert_include ["eCookbook - 2.0", "3", "open"], json
+  end
+
+  def test_filter_without_project_id_should_return_filter_values
+    @request.session[:user_id] = 2
+    get :filter, :name => 'fixed_version_id'
+
+    assert_response :success
+    assert_equal 'application/json', response.content_type
+    json = ActiveSupport::JSON.decode(response.body)
+    assert_include ["OnlineStore - Systemwide visible version", "7", "open"], json
+  end
 end

test/integration/routing/queries_test.rb

@@ -21,6 +21,7 @@ class RoutingQueriesTest < Redmine::RoutingTest
   def test_queries
     should_route 'GET /queries/new' => 'queries#new'
     should_route 'POST /queries' => 'queries#create'
+    should_route 'GET /queries/filter' => 'queries#filter'
 
     should_route 'GET /queries/1/edit' => 'queries#edit', :id => '1'
     should_route 'PUT /queries/1' => 'queries#update', :id => '1'

test/unit/query_test.rb

@@ -118,6 +118,28 @@ class QueryTest < ActiveSupport::TestCase
     assert_not_include 'start_date', query.available_filters
   end
 
+  def test_filter_values_without_project_should_be_arrays
+    q = IssueQuery.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_filter_values_with_project_should_be_arrays
+    q = IssueQuery.new(:project => Project.find(1))
+    assert_not_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 find_issues_with_query(query)
     Issue.joins(:status, :tracker, :project, :priority).where(
          query.statement

test/unit/time_entry_query_test.rb

@@ -27,6 +27,28 @@ class TimeEntryQueryTest < ActiveSupport::TestCase
            :groups_users,
            :enabled_modules
 
+  def test_filter_values_without_project_should_be_arrays
+    q = TimeEntryQuery.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_filter_values_with_project_should_be_arrays
+    q = TimeEntryQuery.new(:project => Project.find(1))
+    assert_not_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_cross_project_activity_filter_should_propose_non_active_activities
     activity = TimeEntryActivity.create!(:name => 'Disabled', :active => false)
     assert !activity.active?