From 0be82ea2c45eb2d95e5cde32365dea8a5b333123 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Thu, 14 Feb 2013 20:37:17 +0000 Subject: [PATCH] Refactor: use an ordered hash to store available filters and remove :order option (#13154). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11372 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/helpers/queries_helper.rb | 23 +-- app/models/issue_query.rb | 190 +++++++++++------------ app/models/query.rb | 52 +++++-- app/models/time_entry_query.rb | 40 ++--- test/unit/helpers/queries_helper_test.rb | 15 +- test/unit/query_test.rb | 5 + 6 files changed, 158 insertions(+), 167 deletions(-) diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index 9c403ab8f..6c8ba1a7f 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -24,28 +24,7 @@ module QueriesHelper def filters_options(query) options = [[]] - sorted_options = query.available_filters.sort do |a, b| - ord = 0 - if !(a[1][:order] == 20 && b[1][:order] == 20) - ord = a[1][:order] <=> b[1][:order] - else - cn = (CustomField::CUSTOM_FIELDS_NAMES.index(a[1][:field].class.name) <=> - CustomField::CUSTOM_FIELDS_NAMES.index(b[1][:field].class.name)) - if cn != 0 - ord = cn - else - f = (a[1][:field] <=> b[1][:field]) - if f != 0 - ord = f - else - # assigned_to or author - ord = (a[0] <=> b[0]) - end - end - end - ord - end - options += sorted_options.map do |field, field_options| + options += query.available_filters.map do |field, field_options| [field_options[:name], field] end end diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index 17c6962ff..1959c3ea3 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -58,141 +58,127 @@ class IssueQuery < Query (project.nil? || user.allowed_to?(:view_issues, project)) && (self.is_public? || self.user_id == user.id) end - def available_filters - return @available_filters if @available_filters - @available_filters = { - "status_id" => { - :type => :list_status, :order => 0, - :values => IssueStatus.sorted.all.collect{|s| [s.name, s.id.to_s] } - }, - "tracker_id" => { - :type => :list, :order => 2, :values => trackers.collect{|s| [s.name, s.id.to_s] } - }, - "priority_id" => { - :type => :list, :order => 3, :values => IssuePriority.all.collect{|s| [s.name, s.id.to_s] } - }, - "subject" => { :type => :text, :order => 8 }, - "created_on" => { :type => :date_past, :order => 9 }, - "updated_on" => { :type => :date_past, :order => 10 }, - "start_date" => { :type => :date, :order => 11 }, - "due_date" => { :type => :date, :order => 12 }, - "estimated_hours" => { :type => :float, :order => 13 }, - "done_ratio" => { :type => :integer, :order => 14 } - } - IssueRelation::TYPES.each do |relation_type, options| - @available_filters[relation_type] = { - :type => :relation, :order => @available_filters.size + 100, - :label => options[:name] - } - end + def initialize_available_filters principals = [] + subprojects = [] + versions = [] + categories = [] + issue_custom_fields = [] + if project principals += project.principals.sort unless project.leaf? subprojects = project.descendants.visible.all - if subprojects.any? - @available_filters["subproject_id"] = { - :type => :list_subprojects, :order => 13, - :values => subprojects.collect{|s| [s.name, s.id.to_s] } - } - principals += Principal.member_of(subprojects) - end + principals += Principal.member_of(subprojects) end + versions = project.shared_versions.all + categories = project.issue_categories.all + issue_custom_fields = project.all_issue_custom_fields else if all_projects.any? - # members of visible projects principals += Principal.member_of(all_projects) - # 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 - @available_filters["project_id"] = { - :type => :list, :order => 1, :values => project_values - } unless project_values.empty? end + versions = Version.visible.find_all_by_sharing('system') + issue_custom_fields = IssueCustomField.where(:is_filter => true, :is_for_all => true).all end principals.uniq! principals.sort! users = principals.select {|p| p.is_a?(User)} - 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] } - @available_filters["assigned_to_id"] = { - :type => :list_optional, :order => 4, :values => assigned_to_values - } unless assigned_to_values.empty? + + add_available_filter "status_id", + :type => :list_status, :values => IssueStatus.sorted.all.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 "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] } - @available_filters["author_id"] = { - :type => :list, :order => 5, :values => author_values - } unless author_values.empty? + add_available_filter("author_id", + :type => :list, :values => author_values + ) unless author_values.empty? + + 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? group_values = Group.all.collect {|g| [g.name, g.id.to_s] } - @available_filters["member_of_group"] = { - :type => :list_optional, :order => 6, :values => group_values - } unless group_values.empty? + add_available_filter("member_of_group", + :type => :list_optional, :values => group_values + ) unless group_values.empty? role_values = Role.givable.collect {|r| [r.name, r.id.to_s] } - @available_filters["assigned_to_role"] = { - :type => :list_optional, :order => 7, :values => role_values - } unless role_values.empty? + add_available_filter("assigned_to_role", + :type => :list_optional, :values => role_values + ) unless role_values.empty? - if User.current.logged? - @available_filters["watcher_id"] = { - :type => :list, :order => 15, :values => [["<< #{l(:label_me)} >>", "me"]] - } + if versions.any? + add_available_filter "fixed_version_id", + :type => :list_optional, + :values => versions.sort.collect{|s| ["#{s.project.name} - #{s.name}", s.id.to_s] } end - if project - # project specific filters - categories = project.issue_categories.all - unless categories.empty? - @available_filters["category_id"] = { - :type => :list_optional, :order => 6, - :values => categories.collect{|s| [s.name, s.id.to_s] } - } - end - versions = project.shared_versions.all - unless versions.empty? - @available_filters["fixed_version_id"] = { - :type => :list_optional, :order => 7, - :values => versions.sort.collect{|s| ["#{s.project.name} - #{s.name}", s.id.to_s] } - } - end - add_custom_fields_filters(project.all_issue_custom_fields) - else - # global filters for cross project issue list - system_shared_versions = Version.visible.find_all_by_sharing('system') - unless system_shared_versions.empty? - @available_filters["fixed_version_id"] = { - :type => :list_optional, :order => 7, - :values => system_shared_versions.sort.collect{|s| - ["#{s.project.name} - #{s.name}", s.id.to_s] - } - } - end - add_custom_fields_filters(IssueCustomField.where(:is_filter => true, :is_for_all => true).all) + if categories.any? + add_available_filter "category_id", + :type => :list_optional, + :values => categories.collect{|s| [s.name, s.id.to_s] } end - add_associations_custom_fields_filters :project, :author, :assigned_to, :fixed_version + + add_available_filter "subject", :type => :text + add_available_filter "created_on", :type => :date_past + add_available_filter "updated_on", :type => :date_past + add_available_filter "start_date", :type => :date + add_available_filter "due_date", :type => :date + add_available_filter "estimated_hours", :type => :float + add_available_filter "done_ratio", :type => :integer + if User.current.allowed_to?(:set_issues_private, nil, :global => true) || User.current.allowed_to?(:set_own_issues_private, nil, :global => true) - @available_filters["is_private"] = { - :type => :list, :order => 16, + add_available_filter "is_private", + :type => :list, :values => [[l(:general_text_yes), "1"], [l(:general_text_no), "0"]] - } end + + if User.current.logged? + add_available_filter "watcher_id", + :type => :list, :values => [["<< #{l(:label_me)} >>", "me"]] + end + + if subprojects.any? + add_available_filter "subproject_id", + :type => :list_subprojects, + :values => subprojects.collect{|s| [s.name, s.id.to_s] } + end + + 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] + end + Tracker.disabled_core_fields(trackers).each {|field| - @available_filters.delete field + delete_available_filter field } - @available_filters.each do |field, options| - options[:name] ||= l(options[:label] || "field_#{field}".gsub(/_id$/, '')) - end - @available_filters end def available_columns diff --git a/app/models/query.rb b/app/models/query.rb index 6a77508df..cd1b9c386 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -278,6 +278,37 @@ class Query < ActiveRecord::Base @all_projects_values = values end + # Adds available filters + def initialize_available_filters + # implemented by sub-classes + end + protected :initialize_available_filters + + # Adds an available filter + def add_available_filter(field, options) + @available_filters ||= ActiveSupport::OrderedHash.new + @available_filters[field] = options + @available_filters + end + + # Removes an available filter + def delete_available_filter(field) + if @available_filters + @available_filters.delete(field) + end + end + + # Return a hash of available filters + def available_filters + unless @available_filters + initialize_available_filters + @available_filters.each do |field, options| + options[:name] ||= l(options[:label] || "field_#{field}".gsub(/_id$/, '')) + end + end + @available_filters + end + def add_filter(field, operator, values=nil) # values must be an array return unless values.nil? || values.is_a?(Array) @@ -692,31 +723,30 @@ class Query < ActiveRecord::Base def add_custom_fields_filters(custom_fields, assoc=nil) return unless custom_fields.present? - @available_filters ||= {} - custom_fields.select(&:is_filter?).each do |field| + custom_fields.select(&:is_filter?).sort.each do |field| case field.field_format when "text" - options = { :type => :text, :order => 20 } + options = { :type => :text } when "list" - options = { :type => :list_optional, :values => field.possible_values, :order => 20} + options = { :type => :list_optional, :values => field.possible_values } when "date" - options = { :type => :date, :order => 20 } + options = { :type => :date } when "bool" - options = { :type => :list, :values => [[l(:general_text_yes), "1"], [l(:general_text_no), "0"]], :order => 20 } + options = { :type => :list, :values => [[l(:general_text_yes), "1"], [l(:general_text_no), "0"]] } when "int" - options = { :type => :integer, :order => 20 } + options = { :type => :integer } when "float" - options = { :type => :float, :order => 20 } + options = { :type => :float } when "user", "version" next unless project values = field.possible_values_options(project) if User.current.logged? && field.field_format == 'user' values.unshift ["<< #{l(:label_me)} >>", "me"] end - options = { :type => :list_optional, :values => values, :order => 20} + options = { :type => :list_optional, :values => values } else - options = { :type => :string, :order => 20 } + options = { :type => :string } end filter_id = "cf_#{field.id}" filter_name = field.name @@ -724,7 +754,7 @@ class Query < ActiveRecord::Base filter_id = "#{assoc}.#{filter_id}" filter_name = l("label_attribute_of_#{assoc}", :name => filter_name) end - @available_filters[filter_id] = options.merge({ + add_available_filter filter_id, options.merge({ :name => filter_name, :format => field.field_format, :field => field diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb index 7283fbd2f..f0301ce32 100644 --- a/app/models/time_entry_query.rb +++ b/app/models/time_entry_query.rb @@ -35,13 +35,8 @@ class TimeEntryQuery < Query add_filter('spent_on', '*') unless filters.present? end - def available_filters - return @available_filters if @available_filters - @available_filters = { - "spent_on" => { :type => :date_past, :order => 0 }, - "comments" => { :type => :text, :order => 5 }, - "hours" => { :type => :float, :order => 6 } - } + def initialize_available_filters + add_available_filter "spent_on", :type => :date_past principals = [] if project @@ -49,10 +44,9 @@ class TimeEntryQuery < Query unless project.leaf? subprojects = project.descendants.visible.all if subprojects.any? - @available_filters["subproject_id"] = { - :type => :list_subprojects, :order => 1, + add_available_filter "subproject_id", + :type => :list_subprojects, :values => subprojects.collect{|s| [s.name, s.id.to_s] } - } principals += Principal.member_of(subprojects) end end @@ -66,9 +60,9 @@ class TimeEntryQuery < Query project_values << ["<< #{l(:label_my_projects).downcase} >>", "mine"] end project_values += all_projects_values - @available_filters["project_id"] = { - :type => :list, :order => 1, :values => project_values - } unless project_values.empty? + add_available_filter("project_id", + :type => :list, :values => project_values + ) unless project_values.empty? end end principals.uniq! @@ -78,22 +72,20 @@ class TimeEntryQuery < Query users_values = [] users_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged? users_values += users.collect{|s| [s.name, s.id.to_s] } - @available_filters["user_id"] = { - :type => :list_optional, :order => 2, :values => users_values - } unless users_values.empty? + add_available_filter("user_id", + :type => :list_optional, :values => users_values + ) unless users_values.empty? activities = (project ? project.activities : TimeEntryActivity.shared.active) - @available_filters["activity_id"] = { - :type => :list, :order => 3, :values => activities.map {|a| [a.name, a.id.to_s]} - } unless activities.empty? + 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 add_custom_fields_filters(TimeEntryCustomField.where(:is_filter => true).all) add_associations_custom_fields_filters :project, :issue, :user - - @available_filters.each do |field, options| - options[:name] ||= l(options[:label] || "field_#{field}".gsub(/_id$/, '')) - end - @available_filters end def available_columns diff --git a/test/unit/helpers/queries_helper_test.rb b/test/unit/helpers/queries_helper_test.rb index d32959efe..81604a38b 100644 --- a/test/unit/helpers/queries_helper_test.rb +++ b/test/unit/helpers/queries_helper_test.rb @@ -29,7 +29,7 @@ class QueriesHelperTest < ActionView::TestCase :projects_trackers, :custom_fields_trackers - def test_order + def test_filters_options_should_be_ordered User.current = User.find_by_login('admin') query = IssueQuery.new(:project => nil, :name => '_') assert_equal 30, query.available_filters.size @@ -40,17 +40,16 @@ class QueriesHelperTest < ActionView::TestCase assert_equal "project_id", fo[2][1] assert_equal "tracker_id", fo[3][1] assert_equal "priority_id", fo[4][1] - assert_equal "watcher_id", fo[17][1] - assert_equal "is_private", fo[18][1] + assert_equal "is_private", fo[17][1] + assert_equal "watcher_id", fo[18][1] end - def test_order_custom_fields + def test_filters_options_should_be_ordered_with_custom_fields set_language_if_valid 'en' - field = UserCustomField.new( + field = UserCustomField.create!( :name => 'order test', :field_format => 'string', :is_for_all => true, :is_filter => true ) - assert field.save User.current = User.find_by_login('admin') query = IssueQuery.new(:project => nil, :name => '_') assert_equal 32, query.available_filters.size @@ -59,7 +58,7 @@ class QueriesHelperTest < ActionView::TestCase assert_equal "Searchable field", fo[19][0] assert_equal "Database", fo[20][0] assert_equal "Project's Development status", fo[21][0] - assert_equal "Assignee's order test", fo[22][0] - assert_equal "Author's order test", fo[23][0] + assert_equal "Author's order test", fo[22][0] + assert_equal "Assignee's order test", fo[23][0] end end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index dfc291d7e..0ee76a477 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -28,6 +28,11 @@ class QueryTest < ActiveSupport::TestCase :projects_trackers, :custom_fields_trackers + def test_available_filters_should_be_ordered + query = IssueQuery.new + assert_equal 0, query.available_filters.keys.index('status_id') + end + def test_custom_fields_for_all_projects_should_be_available_in_global_queries query = IssueQuery.new(:project => nil, :name => '_') assert query.available_filters.has_key?('cf_1')