diff --git a/app/models/custom_field/order_statements.rb b/app/models/custom_field/order_statements.rb index 8192ec7760..569a47badf 100644 --- a/app/models/custom_field/order_statements.rb +++ b/app/models/custom_field/order_statements.rb @@ -61,6 +61,15 @@ module CustomField::OrderStatements end end + ## + # Returns the null handling for the given direction + def null_handling(asc) + return unless %w[int float].include?(field_format) + + null_direction = asc ? 'FIRST' : 'LAST' + Arel.sql("NULLS #{null_direction}") + end + # Returns the grouping result # which differ for multi-value select fields, # because in this case we do want the primary CV values diff --git a/app/models/queries/columns/base.rb b/app/models/queries/columns/base.rb index de3e872b14..02c1e4ed90 100644 --- a/app/models/queries/columns/base.rb +++ b/app/models/queries/columns/base.rb @@ -63,6 +63,10 @@ class Queries::Columns::Base raise NotImplementedError end + def null_handling(_asc) + @null_handling + end + def groupable=(value) @groupable = name_or_value_or_false(value) end diff --git a/app/models/queries/work_packages/columns/custom_field_column.rb b/app/models/queries/work_packages/columns/custom_field_column.rb index a9afce9054..8aa9eadd40 100644 --- a/app/models/queries/work_packages/columns/custom_field_column.rb +++ b/app/models/queries/work_packages/columns/custom_field_column.rb @@ -65,6 +65,10 @@ class Queries::WorkPackages::Columns::CustomFieldColumn < Queries::WorkPackages: @cf.name end + def null_handling(asc) + custom_field.null_handling(asc) + end + def custom_field @cf end diff --git a/app/models/query/group_by.rb b/app/models/query/group_by.rb index fdea6d3059..de58f83e13 100644 --- a/app/models/query/group_by.rb +++ b/app/models/query/group_by.rb @@ -180,10 +180,6 @@ module ::Query::GroupBy sort_entry = query.sort_criteria.detect { |column, _dir| column == query.group_by } order = sort_entry&.last || column.default_order - if column.null_handling - "#{order} #{column.null_handling}" - else - order - end + "#{order} #{column.null_handling(order == 'asc')}" end end diff --git a/app/models/query/sort_criteria.rb b/app/models/query/sort_criteria.rb index f511527fe9..42ddeb03e7 100644 --- a/app/models/query/sort_criteria.rb +++ b/app/models/query/sort_criteria.rb @@ -64,11 +64,7 @@ class ::Query::SortCriteria < ::SortHelper::SortCriteria def append_order(column, criterion, asc = true) ordered_criterion = append_direction(criterion, asc) - if column.null_handling - ordered_criterion.map { |statement| "#{statement} #{column.null_handling}" } - else - ordered_criterion - end + ordered_criterion.map { |statement| "#{statement} #{column.null_handling(asc)}" } end def execute_criterion(criteria) diff --git a/spec/models/query/results_cf_sorting_integration_spec.rb b/spec/models/query/results_cf_sorting_integration_spec.rb new file mode 100644 index 0000000000..e51bc01431 --- /dev/null +++ b/spec/models/query/results_cf_sorting_integration_spec.rb @@ -0,0 +1,99 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2020 the OpenProject GmbH +# +# 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. +#++ + +require 'spec_helper' + +describe ::Query::Results, 'Sorting of custom field floats', type: :model, with_mail: false do + let(:query_results) do + ::Query::Results.new query + end + let(:user) do + FactoryBot.create(:user, + firstname: 'user', + lastname: '1', + member_in_project: project, + member_with_permissions: [:view_work_packages]) + end + + let(:type) { FactoryBot.create(:type_standard, custom_fields: [custom_field]) } + let(:project) do + FactoryBot.create :project, + types: [type], + work_package_custom_fields: [custom_field] + end + let(:work_package_with_float) do + FactoryBot.create :work_package, + type: type, + project: project, + custom_values: {custom_field.id => "6.25"} + end + + let(:work_package_without_float) do + FactoryBot.create :work_package, + type: type, + project: project + end + + let(:custom_field) do + FactoryBot.create(:float_wp_custom_field, name: 'MyFloat') + end + + let(:query) do + FactoryBot.build(:query, + user: user, + show_hierarchies: false, + project: project).tap do |q| + q.filters.clear + q.sort_criteria = sort_criteria + end + end + + before do + login_as(user) + work_package_with_float + work_package_without_float + end + + describe 'sorting ASC by float cf' do + let(:sort_criteria) { [["cf_#{custom_field.id}", 'asc']] } + + it 'returns the correctly sorted result' do + expect(query_results.sorted_work_packages.pluck(:id)) + .to match [work_package_without_float, work_package_with_float].map(&:id) + end + end + + describe 'sorting DESC by float cf' do + let(:sort_criteria) { [["cf_#{custom_field.id}", 'desc']] } + + it 'returns the correctly sorted result' do + expect(query_results.sorted_work_packages.pluck(:id)) + .to match [work_package_with_float, work_package_without_float].map(&:id) + end + end +end