From 9020120e4452212a160f26a002fcbcf8f2aaa07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 18 Mar 2019 13:42:52 +0100 Subject: [PATCH] [29689] Allow setting the sort dir for group_by column https://community.openproject.com/wp/29689 --- app/models/query/group_by.rb | 128 ++++++++++++++++++++++++++++++ app/models/query/results.rb | 126 ++--------------------------- app/models/query/sums.rb | 29 +++++++ spec/models/query/results_spec.rb | 28 +++++++ 4 files changed, 190 insertions(+), 121 deletions(-) create mode 100644 app/models/query/group_by.rb diff --git a/app/models/query/group_by.rb b/app/models/query/group_by.rb new file mode 100644 index 0000000000..c92ce540e2 --- /dev/null +++ b/app/models/query/group_by.rb @@ -0,0 +1,128 @@ +#-- encoding: UTF-8 +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# 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. +# +# See docs/COPYRIGHT.rdoc for more details. +#++ + +module ::Query::Grouping + # Returns the work package count by group or nil if query is not grouped + def work_package_count_by_group + @work_package_count_by_group ||= begin + if query.grouped? + r = groups_grouped_by_column + + transform_group_keys(r) + end + end + rescue ::ActiveRecord::StatementInvalid => e + raise ::Query::StatementInvalid.new(e.message) + end + + def work_package_count_for(group) + work_package_count_by_group[group] + end + + def groups_grouped_by_column + # Rails will raise an (unexpected) RecordNotFound if there's only a nil group value + WorkPackage + .group(query.group_by_statement) + .visible + .includes(all_includes) + .joins(all_filter_joins) + .references(:statuses, :projects) + .where(query.statement) + .count + rescue ActiveRecord::RecordNotFound + { nil => work_package_count } + end + + def transform_group_keys(groups) + column = query.group_by_column + + if column.is_a?(Queries::WorkPackages::Columns::CustomFieldColumn) && column.custom_field.list? + transform_list_group_by_keys(column.custom_field, groups) + elsif column.is_a?(Queries::WorkPackages::Columns::CustomFieldColumn) + transform_custom_field_keys(column.custom_field, groups) + else + groups + end + end + + def transform_list_group_by_keys(custom_field, groups) + options = custom_options_for_keys(custom_field, groups) + + groups.transform_keys do |key| + if custom_field.multi_value? + key.split('.').map do |subkey| + options[subkey].first + end + else + options[key] ? options[key].first : nil + end + end + end + + def custom_options_for_keys(custom_field, groups) + keys = groups.keys.map { |k| k.split('.') } + # Because of multi select cfs we might end up having overlapping groups + # (e.g group "1" and group "1.3" and group "3" which represent concatenated ids). + # This can result in us having ids in the keys array multiple times (e.g. ["1", "1", "3", "3"]). + # If we were to use the keys array with duplicates to find the actual custom options, + # AR would throw an error as the number of records returned does not match the number + # of ids searched for. + custom_field.custom_options.find(keys.flatten.uniq).group_by { |o| o.id.to_s } + end + + def transform_custom_field_keys(custom_field, groups) + groups.transform_keys { |key| custom_field.cast_value(key) } + end + # Returns the SQL sort order that should be prepended for grouping + def group_by_sort_order + if query.grouped? && (column = query.group_by_column) + aliases = include_aliases + + Array(column.sortable).map do |s| + aliased_group_by_sort_order(s, order_for_group_by(column), aliases[column.name]) + end.join(',') + end + end + + def aliased_group_by_sort_order(sortable, order, alias_name) + if alias_name + "#{alias_name}.#{sortable} #{order}" + else + "#{sortable} #{order}" + end + end + + ## + # Retrieve the defined order for the group by + # IF it occurs in the sort criteria + def order_for_group_by(column) + sort_entry = query.sort_criteria.detect { |column, _dir| column == query.group_by } + sort_entry&.last || column.default_order + end +end diff --git a/app/models/query/results.rb b/app/models/query/results.rb index 65b2d75533..d2acfd51d7 100644 --- a/app/models/query/results.rb +++ b/app/models/query/results.rb @@ -28,8 +28,12 @@ # See docs/COPYRIGHT.rdoc for more details. #++ +require_dependency 'query/group_by' +require_dependency 'query/sums' + class ::Query::Results - include Sums + include ::Query::Grouping + include ::Query::Sums include Redmine::I18n attr_accessor :options, @@ -53,23 +57,6 @@ class ::Query::Results raise ::Query::StatementInvalid.new(e.message) end - # Returns the work package count by group or nil if query is not grouped - def work_package_count_by_group - @work_package_count_by_group ||= begin - if query.grouped? - r = groups_grouped_by_column - - transform_group_keys(r) - end - end - rescue ::ActiveRecord::StatementInvalid => e - raise ::Query::StatementInvalid.new(e.message) - end - - def work_package_count_for(group) - work_package_count_by_group[group] - end - def work_packages WorkPackage .visible @@ -101,35 +88,6 @@ class ::Query::Results scope end - def column_total_sums - query.columns.map { |column| total_sum_of(column) } - end - - def all_total_sums - query.available_columns.select { |column| - column.summable? && Setting.work_package_list_summable_columns.include?(column.name.to_s) - }.inject({}) { |result, column| - sum = total_sum_of(column) - result[column] = sum unless sum.nil? - result - } - end - - def all_sums_for_group(group) - return nil unless query.grouped? - - group_work_packages = work_packages.select { |wp| query.group_by_column.value(wp) == group } - query.available_columns.inject({}) do |result, column| - sum = sum_of(column, group_work_packages) - result[column] = sum unless sum.nil? - result - end - end - - def column_group_sums - query.group_by_column && query.columns.map { |column| grouped_sums(column) } - end - def order_option order_option = [group_by_sort_order].reject(&:blank?).join(', ') @@ -161,61 +119,6 @@ class ::Query::Results name.to_s =~ /\Acf_\d+\z/ end - def groups_grouped_by_column - # Rails will raise an (unexpected) RecordNotFound if there's only a nil group value - WorkPackage - .group(query.group_by_statement) - .visible - .includes(all_includes) - .joins(all_filter_joins) - .references(:statuses, :projects) - .where(query.statement) - .count - rescue ActiveRecord::RecordNotFound - { nil => work_package_count } - end - - def transform_group_keys(groups) - column = query.group_by_column - - if column.is_a?(Queries::WorkPackages::Columns::CustomFieldColumn) && column.custom_field.list? - transform_list_group_by_keys(column.custom_field, groups) - elsif column.is_a?(Queries::WorkPackages::Columns::CustomFieldColumn) - transform_custom_field_keys(column.custom_field, groups) - else - groups - end - end - - def transform_list_group_by_keys(custom_field, groups) - options = custom_options_for_keys(custom_field, groups) - - groups.transform_keys do |key| - if custom_field.multi_value? - key.split('.').map do |subkey| - options[subkey].first - end - else - options[key] ? options[key].first : nil - end - end - end - - def custom_options_for_keys(custom_field, groups) - keys = groups.keys.map { |k| k.split('.') } - # Because of multi select cfs we might end up having overlapping groups - # (e.g group "1" and group "1.3" and group "3" which represent concatenated ids). - # This can result in us having ids in the keys array multiple times (e.g. ["1", "1", "3", "3"]). - # If we were to use the keys array with duplicates to find the actual custom options, - # AR would throw an error as the number of records returned does not match the number - # of ids searched for. - custom_field.custom_options.find(keys.flatten.uniq).group_by { |o| o.id.to_s } - end - - def transform_custom_field_keys(custom_field, groups) - groups.transform_keys { |key| custom_field.cast_value(key) } - end - ## # Returns the columns that need to be included to allow: # * sorting @@ -258,25 +161,6 @@ class ::Query::Results sorting_by_column_name end - # Returns the SQL sort order that should be prepended for grouping - def group_by_sort_order - if query.grouped? && (column = query.group_by_column) - aliases = include_aliases - - Array(column.sortable).map do |s| - aliased_group_by_sort_order(s, column, aliases[column.name]) - end.join(',') - end - end - - def aliased_group_by_sort_order(sortable, column, alias_name) - if alias_name - "#{alias_name}.#{sortable} #{column.default_order}" - else - "#{sortable} #{column.default_order}" - end - end - # To avoid naming conflicts, joined tables are aliased if they are joined # more than once. Here, joining tables that are referenced by multiple # columns are of particular interest. diff --git a/app/models/query/sums.rb b/app/models/query/sums.rb index 86fe6be28e..d9b3b0df4f 100644 --- a/app/models/query/sums.rb +++ b/app/models/query/sums.rb @@ -114,4 +114,33 @@ module ::Query::Sums def should_be_summed_up?(column) column.summable? && Setting.work_package_list_summable_columns.include?(column.name.to_s) end + + def column_total_sums + query.columns.map { |column| total_sum_of(column) } + end + + def all_total_sums + query.available_columns.select { |column| + column.summable? && Setting.work_package_list_summable_columns.include?(column.name.to_s) + }.inject({}) { |result, column| + sum = total_sum_of(column) + result[column] = sum unless sum.nil? + result + } + end + + def all_sums_for_group(group) + return nil unless query.grouped? + + group_work_packages = work_packages.select { |wp| query.group_by_column.value(wp) == group } + query.available_columns.inject({}) do |result, column| + sum = sum_of(column, group_work_packages) + result[column] = sum unless sum.nil? + result + end + end + + def column_group_sums + query.group_by_column && query.columns.map { |column| grouped_sums(column) } + end end diff --git a/spec/models/query/results_spec.rb b/spec/models/query/results_spec.rb index f7dbd6455e..240a4e02bb 100644 --- a/spec/models/query/results_spec.rb +++ b/spec/models/query/results_spec.rb @@ -369,6 +369,34 @@ describe ::Query::Results, type: :model do end end + context 'sorting and grouping by priority' do + let(:prio_low) { FactoryBot.create :issue_priority, position: 1 } + let(:prio_high) { FactoryBot.create :issue_priority, position: 0 } + let(:group_by) { 'priority' } + + before do + allow(User).to receive(:current).and_return(user_1) + + work_package1.priority = prio_low + work_package2.priority = prio_high + + work_package1.save(validate: false) + work_package2.save(validate: false) + end + + it 'respects the sorting (Regression #29689)' do + query.sort_criteria = [['priority', 'asc']] + + expect(query_results.sorted_work_packages) + .to match [work_package1, work_package2] + + query.sort_criteria = [['priority', 'desc']] + + expect(query_results.sorted_work_packages) + .to match [work_package2, work_package1] + end + end + context 'sorting by author and responsible, grouping by assigned_to' do let(:group_by) { 'assigned_to' } let(:sort_by) { [['author', 'asc'], ['responsible', 'desc']] }