diff --git a/app/models/custom_field/order_statements.rb b/app/models/custom_field/order_statements.rb index 98c46c8c8167..a6811620cc02 100644 --- a/app/models/custom_field/order_statements.rb +++ b/app/models/custom_field/order_statements.rb @@ -27,152 +27,144 @@ #++ module CustomField::OrderStatements - # Returns a ORDER BY clause that can used to sort customized - # objects by their value of the custom field. - # Returns false, if the custom field can not be used for sorting. - def order_statements + # Returns the expression to use in ORDER BY clause to sort objects by their + # value of the custom field. + def order_statement + case field_format + when "string", "date", "bool", "link", "int", "float", "list", "user", "version" + "cf_order_#{id}.value" + end + end + + # Returns the join statement that is required to sort objects by their value + # of the custom field. + def order_join_statement case field_format - when "list" - [order_by_list_sql] when "string", "date", "bool", "link" - [coalesce_select_custom_value_as_string] - when "int", "float" - # Make the database cast values into numeric - # Postgresql will raise an error if a value can not be casted! - # CustomValue validations should ensure that it doesn't occur - [select_custom_value_as_decimal] + join_for_order_by_string_sql + when "int" + join_for_order_by_int_sql + when "float" + join_for_order_by_float_sql + when "list" + join_for_order_by_list_sql when "user" - [order_by_user_sql] + join_for_order_by_user_sql when "version" - [order_by_version_sql] + join_for_order_by_version_sql end end - ## - # Returns the null handling for the given direction - def null_handling(asc) - return unless %w[int float].include?(field_format) - + # Returns the ORDER BY option defining order of objects without value for the + # custom field. + def order_null_handling(asc) 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 + # Returns the expression to use in GROUP BY (and ORDER BY) clause to group + # objects by their value of the custom field. def group_by_statement - return order_statements unless field_format == "list" + return unless can_be_used_for_grouping? - if multi_value? - # We want to return the internal IDs in the case of grouping - select_custom_values_as_group - else - coalesce_select_custom_value_as_string - end + order_statement end - private - - def coalesce_select_custom_value_as_string - # COALESCE is here to make sure that blank and NULL values are sorted equally - <<-SQL.squish - COALESCE(#{select_custom_value_as_string}, '') - SQL - end + # Returns the expression to use in SELECT clause if it differs from one used + # to group by + def group_by_select_statement + return unless field_format == "list" - def select_custom_value_as_string - <<-SQL.squish - ( - SELECT cv_sort.value - FROM #{CustomValue.quoted_table_name} cv_sort - WHERE #{cv_sort_only_custom_field_condition_sql} - LIMIT 1 - ) - SQL + # MIN needed to not add this column to group by, ANY_VALUE can be used when + # minimum required PostgreSQL becomes 16 + "MIN(cf_order_#{id}.ids)" end - def select_custom_values_as_group - <<-SQL.squish - COALESCE( - ( - SELECT string_agg(cv_sort.value, '.') - FROM #{CustomValue.quoted_table_name} cv_sort - WHERE #{cv_sort_only_custom_field_condition_sql} - AND cv_sort.value IS NOT NULL - ), - '' - ) - SQL - end + # Returns the join statement that is required to group objects by their value + # of the custom field. + def group_by_join_statement + return unless can_be_used_for_grouping? - def select_custom_value_as_decimal - <<-SQL.squish - ( - SELECT CAST(cv_sort.value AS decimal(60,3)) - FROM #{CustomValue.quoted_table_name} cv_sort - WHERE #{cv_sort_only_custom_field_condition_sql} - AND cv_sort.value <> '' - AND cv_sort.value IS NOT NULL - LIMIT 1 - ) - SQL + order_join_statement end - def order_by_list_sql - columns = multi_value? ? "array_agg(co_sort.position ORDER BY co_sort.position)" : "co_sort.position" - limit = multi_value? ? "" : "LIMIT 1" + private + def can_be_used_for_grouping? = field_format.in?(%w[list date bool int float string link]) + + # Template for all the join statements. + # + # For single value custom fields thejoin ensures single value for every + # customized object using DISTINCT ON and selecting first value by id of + # custom value: + # + # LEFT OUTER JOIN ( + # SELECT DISTINCT ON (cv.customized_id), cv.customized_id, xxx "value" + # FROM custom_values cv + # WHERE … + # ORDER BY cv.customized_id, cv.id + # ) cf_order_NNN ON cf_order_NNN.customized_id = … + # + # For multi value custom fields the GROUP BY and value aggregate function + # ensure single value for every customized object: + # + # LEFT OUTER JOIN ( + # SELECT cv.customized_id, ARRAY_AGG(xxx ORDERY BY yyy) "value" + # FROM custom_values cv + # WHERE … + # GROUP BY cv.customized_id, cv.id + # ) cf_order_NNN ON cf_order_NNN.customized_id = … + # + def join_for_order_sql(value:, add_select: nil, join: nil, multi_value: false) <<-SQL.squish - ( - SELECT #{columns} - FROM #{CustomOption.quoted_table_name} co_sort - INNER JOIN #{CustomValue.quoted_table_name} cv_sort - ON cv_sort.value IS NOT NULL AND cv_sort.value != '' AND co_sort.id = cv_sort.value::bigint - WHERE #{cv_sort_only_custom_field_condition_sql} - #{limit} - ) + LEFT OUTER JOIN ( + SELECT + #{multi_value ? '' : 'DISTINCT ON (cv.customized_id)'} + cv.customized_id + , #{value} "value" + #{", #{add_select}" if add_select} + FROM #{CustomValue.quoted_table_name} cv + #{join} + WHERE cv.customized_type = #{CustomValue.connection.quote(self.class.customized_class.name)} + AND cv.custom_field_id = #{id} + AND cv.value IS NOT NULL + AND cv.value != '' + #{multi_value ? 'GROUP BY cv.customized_id' : 'ORDER BY cv.customized_id, cv.id'} + ) cf_order_#{id} + ON cf_order_#{id}.customized_id = #{self.class.customized_class.quoted_table_name}.id SQL end - def order_by_user_sql - columns_array = "ARRAY[cv_user.lastname, cv_user.firstname, cv_user.mail]" + def join_for_order_by_string_sql = join_for_order_sql(value: "cv.value") - columns = multi_value? ? "array_agg(#{columns_array} ORDER BY #{columns_array})" : columns_array - limit = multi_value? ? "" : "LIMIT 1" + def join_for_order_by_int_sql = join_for_order_sql(value: "cv.value::decimal(60)") - <<-SQL.squish - ( - SELECT #{columns} - FROM #{User.quoted_table_name} cv_user - INNER JOIN #{CustomValue.quoted_table_name} cv_sort - ON cv_sort.value IS NOT NULL AND cv_sort.value != '' AND cv_user.id = cv_sort.value::bigint - WHERE #{cv_sort_only_custom_field_condition_sql} - #{limit} - ) - SQL + def join_for_order_by_float_sql = join_for_order_sql(value: "cv.value::double precision") + + def join_for_order_by_list_sql + join_for_order_sql( + value: multi_value? ? "ARRAY_AGG(co.position ORDER BY co.position)" : "co.position", + add_select: "#{multi_value? ? "ARRAY_TO_STRING(ARRAY_AGG(cv.value ORDER BY co.position), '.')" : 'cv.value'} ids", + join: "INNER JOIN #{CustomOption.quoted_table_name} co ON co.id = cv.value::bigint", + multi_value: + ) end - def order_by_version_sql - columns = multi_value? ? "array_agg(cv_version.name ORDER BY cv_version.name)" : "cv_version.name" - limit = multi_value? ? "" : "LIMIT 1" + def join_for_order_by_user_sql + columns_array = "ARRAY[users.lastname, users.firstname, users.mail]" - <<-SQL.squish - ( - SELECT #{columns} - FROM #{Version.quoted_table_name} cv_version - INNER JOIN #{CustomValue.quoted_table_name} cv_sort - ON cv_sort.value IS NOT NULL AND cv_sort.value != '' AND cv_version.id = cv_sort.value::bigint - WHERE #{cv_sort_only_custom_field_condition_sql} - #{limit} - ) - SQL + join_for_order_sql( + value: multi_value? ? "ARRAY_AGG(#{columns_array} ORDER BY #{columns_array})" : columns_array, + join: "INNER JOIN #{User.quoted_table_name} users ON users.id = cv.value::bigint", + multi_value: + ) end - def cv_sort_only_custom_field_condition_sql - <<-SQL.squish - cv_sort.customized_type='#{self.class.customized_class.name}' - AND cv_sort.customized_id=#{self.class.customized_class.quoted_table_name}.id - AND cv_sort.custom_field_id=#{id} - SQL + def join_for_order_by_version_sql + join_for_order_sql( + value: multi_value? ? "array_agg(versions.name ORDER BY versions.name)" : "versions.name", + join: "INNER JOIN #{Version.quoted_table_name} versions ON versions.id = cv.value::bigint", + multi_value: + ) end end diff --git a/app/models/queries/projects/orders/custom_field_order.rb b/app/models/queries/projects/orders/custom_field_order.rb index 30aa9e3b4558..b7ad4e3cf9f7 100644 --- a/app/models/queries/projects/orders/custom_field_order.rb +++ b/app/models/queries/projects/orders/custom_field_order.rb @@ -58,10 +58,16 @@ def available? private def order(scope) - joined_statement = custom_field.order_statements.map do |statement| - Arel.sql("#{statement} #{direction}") + if (join_statement = custom_field.order_join_statement) + scope = scope.joins(join_statement) end - scope.order(joined_statement) + order_statement = "#{custom_field.order_statement} #{direction}" + + if (null_handling = custom_field.order_null_handling(direction == :asc)) + order_statement = "#{order_statement} #{null_handling}" + end + + scope.order(Arel.sql(order_statement)) end end diff --git a/app/models/queries/work_packages/selects/custom_field_select.rb b/app/models/queries/work_packages/selects/custom_field_select.rb index c6e2a107d5da..6120e1111035 100644 --- a/app/models/queries/work_packages/selects/custom_field_select.rb +++ b/app/models/queries/work_packages/selects/custom_field_select.rb @@ -32,21 +32,20 @@ def initialize(custom_field) @cf = custom_field - set_name! - set_sortable! - set_groupable! - set_summable! - end - - def groupable_custom_field?(custom_field) - %w(list date bool int).include?(custom_field.field_format) + @name = custom_field.column_name.to_sym + @sortable = custom_field.order_statement + @sortable_join = custom_field.order_join_statement + @groupable = custom_field.group_by_statement + @groupable_join = custom_field.group_by_join_statement + @groupable_select = custom_field.group_by_select_statement + @summable = summable_statement end def caption @cf.name end - delegate :null_handling, to: :custom_field + def null_handling(...) = custom_field.order_null_handling(...) def custom_field @cf @@ -68,32 +67,6 @@ def self.instances(context = nil) private - def set_name! - self.name = custom_field.column_name.to_sym - end - - def set_sortable! - self.sortable = custom_field.order_statements || false - end - - def set_groupable! - self.groupable = custom_field.group_by_statement if groupable_custom_field?(custom_field) - self.groupable ||= false - end - - def set_summable! - self.summable = if %w(float int).include?(custom_field.field_format) - select = summable_select_statement - - ->(query, grouped) { - Queries::WorkPackages::Selects::WorkPackageSelect - .scoped_column_sum(summable_scope(query), select, grouped && query.group_by_statement) - } - else - false - end - end - def summable_scope(query) WorkPackage .where(id: query.results.work_packages) @@ -105,9 +78,22 @@ def summable_scope(query) def summable_select_statement if custom_field.field_format == "int" - "COALESCE(SUM(value::BIGINT)::BIGINT, 0) #{name}" + "COALESCE(SUM(#{CustomValue.quoted_table_name}.value::BIGINT)::BIGINT, 0) #{name}" + else + "COALESCE(ROUND(SUM(#{CustomValue.quoted_table_name}.value::NUMERIC), 2)::FLOAT, 0.0) #{name}" + end + end + + def summable_statement + if %w[float int].include?(custom_field.field_format) + select = summable_select_statement + + ->(query, grouped) { + Queries::WorkPackages::Selects::WorkPackageSelect + .scoped_column_sum(summable_scope(query), select, grouped:, query:) + } else - "COALESCE(ROUND(SUM(value::NUMERIC), 2)::FLOAT, 0.0) #{name}" + false end end end diff --git a/app/models/queries/work_packages/selects/property_select.rb b/app/models/queries/work_packages/selects/property_select.rb index 48635b1c296e..2515f7b02946 100644 --- a/app/models/queries/work_packages/selects/property_select.rb +++ b/app/models/queries/work_packages/selects/property_select.rb @@ -135,15 +135,12 @@ def caption shared_with_users: { sortable: false, groupable: false - } } def self.instances(_context = nil) - property_selects.filter_map do |name, options| - next unless !options[:if] || options[:if].call - - new(name, options.except(:if)) + property_selects.map do |name, options| + new(name, options) end end end diff --git a/app/models/queries/work_packages/selects/relation_of_type_select.rb b/app/models/queries/work_packages/selects/relation_of_type_select.rb index eb6809cec69c..f7dd9d4d668e 100644 --- a/app/models/queries/work_packages/selects/relation_of_type_select.rb +++ b/app/models/queries/work_packages/selects/relation_of_type_select.rb @@ -28,12 +28,9 @@ class Queries::WorkPackages::Selects::RelationOfTypeSelect < Queries::WorkPackages::Selects::RelationSelect def initialize(type) - self.type = type - super(name) - end + super(:"relations_of_type_#{type[:sym]}") - def name - :"relations_of_type_#{type[:sym]}" + @type = type end def sym diff --git a/app/models/queries/work_packages/selects/relation_select.rb b/app/models/queries/work_packages/selects/relation_select.rb index c12f7663eb2e..224b1e58f743 100644 --- a/app/models/queries/work_packages/selects/relation_select.rb +++ b/app/models/queries/work_packages/selects/relation_select.rb @@ -27,7 +27,7 @@ #++ class Queries::WorkPackages::Selects::RelationSelect < Queries::WorkPackages::Selects::WorkPackageSelect - attr_accessor :type + attr_reader :type def self.granted_by_enterprise_token EnterpriseToken.allows_to?(:work_package_query_relation_columns) diff --git a/app/models/queries/work_packages/selects/relation_to_type_select.rb b/app/models/queries/work_packages/selects/relation_to_type_select.rb index cd6ddcd432fc..7744ed717f10 100644 --- a/app/models/queries/work_packages/selects/relation_to_type_select.rb +++ b/app/models/queries/work_packages/selects/relation_to_type_select.rb @@ -28,14 +28,9 @@ class Queries::WorkPackages::Selects::RelationToTypeSelect < Queries::WorkPackages::Selects::RelationSelect def initialize(type) - super + super(:"relations_to_type_#{type.id}") - set_name! type - self.type = type - end - - def set_name!(type) - self.name = :"relations_to_type_#{type.id}" + @type = type end def caption diff --git a/app/models/queries/work_packages/selects/work_package_select.rb b/app/models/queries/work_packages/selects/work_package_select.rb index a3ffe42aa3b6..9b9603518367 100644 --- a/app/models/queries/work_packages/selects/work_package_select.rb +++ b/app/models/queries/work_packages/selects/work_package_select.rb @@ -27,19 +27,15 @@ #++ class Queries::WorkPackages::Selects::WorkPackageSelect - attr_accessor :highlightable, - :name, - :sortable_join, - :summable, - :default_order, - :association - alias_method :highlightable?, :highlightable - - attr_reader :groupable, - :sortable, - :displayable - - attr_writer :null_handling, + attr_reader :highlightable, + :name, + :sortable_join, + :groupable_join, + :groupable_select, + :summable, + :default_order, + :association, + :null_handling, :summable_select, :summable_work_packages_select @@ -51,17 +47,17 @@ def self.select_group_by(group_by_statement) group_by = group_by_statement group_by = group_by.first if group_by.is_a?(Array) - "#{group_by} id" + "#{group_by} group_id" end - def self.scoped_column_sum(scope, select, group_by) - scope = scope - .except(:order, :select) + def self.scoped_column_sum(scope, select, grouped:, query:) + scope = scope.except(:order, :select) - if group_by + if grouped scope - .group(group_by) - .select(select_group_by(group_by), select) + .joins(query.group_by_join_statement) + .group(query.group_by_statement) + .select(select_group_by(query.group_by_select), select) else scope .select(select) @@ -76,31 +72,25 @@ def null_handling(_asc) @null_handling end - def groupable=(value) - @groupable = name_or_value_or_false(value) + def displayable + @displayable.nil? ? true : @displayable end - def sortable=(value) - @sortable = name_or_value_or_false(value) + def sortable + name_or_value_or_false(@sortable) end - def displayable=(value) - @displayable = value.nil? ? true : value + def groupable + name_or_value_or_false(@groupable) end - def displayable? - displayable - end + def displayable? = !!displayable - # Returns true if the column is sortable, otherwise false - def sortable? - !!sortable - end + def sortable? = !!sortable - # Returns true if the column is groupable, otherwise false - def groupable? - !!groupable - end + def groupable? = !!groupable + + def highlightable? = !!highlightable def summable? summable || @summable_select || @summable_work_packages_select @@ -127,22 +117,25 @@ def value(model) end def initialize(name, options = {}) - self.name = name - - %i(sortable - sortable_join - displayable - groupable - summable - summable_select - summable_work_packages_select - association - null_handling - default_order).each do |attribute| - send(:"#{attribute}=", options[attribute]) + @name = name + + %i[ + sortable + sortable_join + displayable + groupable + groupable_join + summable + summable_select + summable_work_packages_select + association + null_handling + default_order + ].each do |attribute| + instance_variable_set(:"@#{attribute}", options[attribute]) end - self.highlightable = !!options.fetch(:highlightable, false) + @highlightable = !!options.fetch(:highlightable, false) end def caption diff --git a/app/models/query.rb b/app/models/query.rb index d8051b6b1d06..47eabd5b19d6 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -359,7 +359,15 @@ def group_by_column end def group_by_statement - group_by_column.try(:groupable) + group_by_column&.groupable + end + + def group_by_select + group_by_column&.groupable_select || group_by_statement + end + + def group_by_join_statement + group_by_column&.groupable_join end def statement diff --git a/app/models/query/results.rb b/app/models/query/results.rb index dbf58dcbfa8c..8bffcccd8ad0 100644 --- a/app/models/query/results.rb +++ b/app/models/query/results.rb @@ -82,6 +82,7 @@ def filtered_work_packages def sorted_work_packages work_package_scope .joins(sort_criteria_joins) + .joins(query.group_by_join_statement) .order(order_option) .order(sort_criteria_array) end @@ -195,10 +196,8 @@ def columns_hash_for(association = nil) ## # Return the case insensitive version for columns with a string type def case_insensitive_condition(column_key, condition, columns_hash) - if columns_hash[column_key]&.type == :string + if columns_hash[column_key]&.type == :string || custom_field_type(column_key) == "string" "LOWER(#{condition})" - elsif custom_field_type(column_key) == "string" - condition.map { |c| "LOWER(#{c})" } else condition end diff --git a/app/models/query/results/group_by.rb b/app/models/query/results/group_by.rb index b5609d28e876..c3097fdb03e7 100644 --- a/app/models/query/results/group_by.rb +++ b/app/models/query/results/group_by.rb @@ -46,6 +46,7 @@ def work_package_count_for(group) def group_counts_by_group work_packages_with_includes_for_count + .joins(query.group_by_join_statement) .group(group_by_for_count) .visible .references(:statuses, :projects) @@ -67,7 +68,7 @@ def group_by_for_count end def pluck_for_count - Array(query.group_by_statement).map { |statement| Arel.sql(statement) } + + Array(query.group_by_select).map { |statement| Arel.sql(statement) } + [Arel.sql('COUNT(DISTINCT "work_packages"."id")')] end @@ -98,7 +99,7 @@ def transform_list_custom_field_keys(custom_field, groups) groups.transform_keys do |key| if custom_field.multi_value? - key.split(".").map do |subkey| + (key ? key.split(".") : []).map do |subkey| options[subkey].first end else @@ -108,7 +109,7 @@ def transform_list_custom_field_keys(custom_field, groups) end def custom_options_for_keys(custom_field, groups) - keys = groups.keys.map { |k| k.split(".") } + keys = groups.keys.map { |k| 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"]). diff --git a/app/models/query/results/sums.rb b/app/models/query/results/sums.rb index 23509ef6656f..3eaf4dec7cca 100644 --- a/app/models/query/results/sums.rb +++ b/app/models/query/results/sums.rb @@ -42,24 +42,26 @@ def all_total_sums def all_group_sums return nil unless query.grouped? - sums_by_id = sums_select(true).inject({}) do |result, group_sum| - result[group_sum["id"]] = {} + transform_group_keys(sums_by_group_id) + end + + private + + def sums_by_group_id + sums_select(true).inject({}) do |result, group_sum| + result[group_sum["group_id"]] = {} query.summed_up_columns.each do |column| - result[group_sum["id"]][column] = group_sum[column.name.to_s] + result[group_sum["group_id"]][column] = group_sum[column.name.to_s] end result end - - transform_group_keys(sums_by_id) end - private - def sums_select(grouped = false) select = if grouped - ["work_packages.id"] + ["work_packages.group_id"] else [] end @@ -86,7 +88,7 @@ def sums_work_package_scope(grouped) .select(sums_work_package_scope_selects(grouped)) if grouped - scope.group(query.group_by_statement) + scope.joins(query.group_by_join_statement).group(query.group_by_statement) else scope end @@ -96,7 +98,8 @@ def sums_callable_joins(grouped) callable_summed_up_columns .map do |c| join_condition = if grouped - "#{c.name}.id = work_packages.id OR #{c.name}.id IS NULL AND work_packages.id IS NULL" + "#{c.name}.group_id = work_packages.group_id OR " \ + "#{c.name}.group_id IS NULL AND work_packages.group_id IS NULL" else "TRUE" end @@ -109,7 +112,7 @@ def sums_callable_joins(grouped) def sums_work_package_scope_selects(grouped) group_statement = if grouped - [Queries::WorkPackages::Selects::WorkPackageSelect.select_group_by(query.group_by_statement)] + [Queries::WorkPackages::Selects::WorkPackageSelect.select_group_by(query.group_by_select)] else [] end diff --git a/modules/costs/lib/costs/query_currency_select.rb b/modules/costs/lib/costs/query_currency_select.rb index f4d21aa6b19c..795da6bc500b 100644 --- a/modules/costs/lib/costs/query_currency_select.rb +++ b/modules/costs/lib/costs/query_currency_select.rb @@ -57,7 +57,8 @@ def real_value(work_package) Queries::WorkPackages::Selects::WorkPackageSelect .scoped_column_sum(scope, "COALESCE(ROUND(SUM(cost_entries_sum), 2)::FLOAT, 0.0) material_costs", - grouped && query.group_by_statement) + grouped:, + query:) } }, labor_costs: { @@ -70,7 +71,8 @@ def real_value(work_package) Queries::WorkPackages::Selects::WorkPackageSelect .scoped_column_sum(scope, "COALESCE(ROUND(SUM(time_entries_sum), 2)::FLOAT, 0.0) labor_costs", - grouped && query.group_by_statement) + grouped:, + query:) } }, overall_costs: { diff --git a/modules/costs/spec/lib/costs/query_currency_select_spec.rb b/modules/costs/spec/lib/costs/query_currency_select_spec.rb index 93da8b5e9886..90794f8d2985 100644 --- a/modules/costs/spec/lib/costs/query_currency_select_spec.rb +++ b/modules/costs/spec/lib/costs/query_currency_select_spec.rb @@ -86,20 +86,19 @@ query = double("query") result = double("result") - allow(query) - .to receive(:results) - .and_return result - allow(result) .to receive(:work_packages) .and_return(WorkPackage.all) - allow(query) - .to receive(:group_by_statement) - .and_return("author_id") + allow(query).to receive_messages( + results: result, + group_by_join_statement: nil, + group_by_select: "author_id", + group_by_statement: "author_id" + ) expect(ActiveRecord::Base.connection.select_all(instance.summable.(query, true).to_sql).columns) - .to match_array %w(id material_costs) + .to match_array %w(group_id material_costs) end end end @@ -125,20 +124,19 @@ query = double("query") result = double("result") - allow(query) - .to receive(:results) - .and_return result - allow(result) .to receive(:work_packages) .and_return(WorkPackage.all) - allow(query) - .to receive(:group_by_statement) - .and_return("author_id") + allow(query).to receive_messages( + results: result, + group_by_join_statement: nil, + group_by_select: "author_id", + group_by_statement: "author_id" + ) expect(ActiveRecord::Base.connection.select_all(instance.summable.(query, true).to_sql).columns) - .to match_array %w(id labor_costs) + .to match_array %w(group_id labor_costs) end end end diff --git a/spec/models/queries/projects/project_query_custom_field_order_spec.rb b/spec/models/queries/projects/project_query_custom_field_order_spec.rb index fd13984ed995..b38d1e14c469 100644 --- a/spec/models/queries/projects/project_query_custom_field_order_spec.rb +++ b/spec/models/queries/projects/project_query_custom_field_order_spec.rb @@ -116,9 +116,9 @@ def project_without_cf_value let(:projects) do [ + project_without_cf_value, project_with_cf_value("6"), - project_with_cf_value("16"), - project_without_cf_value # TODO: should be at index 0 + project_with_cf_value("16") ] end end @@ -130,9 +130,9 @@ def project_without_cf_value let(:projects) do [ + project_without_cf_value, project_with_cf_value("6.25"), - project_with_cf_value("16"), - project_without_cf_value # TODO: should be at index 0 + project_with_cf_value("16") ] end end @@ -177,11 +177,11 @@ def project_without_cf_value let(:projects) do [ + project_without_cf_value, # sorting is done by position, and not by value project_with_cf_value(id_by_value.fetch("100")), project_with_cf_value(id_by_value.fetch("3")), project_with_cf_value(id_by_value.fetch("20")), - project_without_cf_value # TODO: should be at index 0 ] end end @@ -193,6 +193,7 @@ def project_without_cf_value let(:projects) do [ + project_without_cf_value, project_with_cf_value(*id_by_value.fetch_values("100")), # 100 project_with_cf_value(*id_by_value.fetch_values("3", "100")), # 100, 3 project_with_cf_value(*id_by_value.fetch_values("3", "20", "100")), # 100, 3, 20 @@ -205,14 +206,13 @@ def project_without_cf_value project_with_cf_value(*id_by_value.fetch_values("3")), # 3 project_with_cf_value(*id_by_value.fetch_values("3", "20")), # 3, 20 project_with_cf_value(*id_by_value.fetch_values("20")), # 20 - project_without_cf_value # TODO: decide on order of absent values ] end let(:projects_desc) do indexes = projects.each_index.to_a - # order of values for a work package is ignored, so ordered by falling back on id asc - indexes[2...8] = indexes[2...8].reverse + # order of work packages with same values in different order falls back on next column (id asc) + indexes[3...9] = indexes[3...9].reverse projects.values_at(*indexes.reverse) end end @@ -248,11 +248,11 @@ def project_without_cf_value let(:custom_field_values) do [ + nil, id_by_login.fetch("ax"), id_by_login.fetch("ba"), id_by_login.fetch("bb1"), id_by_login.fetch("bb2"), - nil # TODO: should be at index 0 ] end @@ -272,13 +272,13 @@ def project_without_cf_value let(:custom_field_values) do [ + [], id_by_login.fetch_values("ax"), # ax id_by_login.fetch_values("bb1", "ax"), # ax, bb1 id_by_login.fetch_values("ax", "bb1"), # ax, bb1 id_by_login.fetch_values("ba"), # ba id_by_login.fetch_values("bb1", "ba"), # ba, bb1 id_by_login.fetch_values("ba", "bb2"), # ba, bb2 - [] # TODO: should be at index 0 ] end @@ -290,8 +290,8 @@ def project_without_cf_value let(:projects_desc) do indexes = projects.each_index.to_a - # order of values for a work package is ignored, so ordered by falling back on id asc - indexes[1...3] = indexes[1...3].reverse + # order of work packages with same values in different order falls back on next column (id asc) + indexes[2...4] = indexes[2...4].reverse projects.values_at(*indexes.reverse) end end @@ -316,11 +316,11 @@ def project_without_cf_value let(:projects) do [ + project, project_with_cf_value(id_by_name.fetch("10.10.10")), project_with_cf_value(id_by_name.fetch("10.10.2")), project_with_cf_value(id_by_name.fetch("10.2")), project_with_cf_value(id_by_name.fetch("9")), - project # TODO: should be at index 0 ] end end @@ -332,20 +332,20 @@ def project_without_cf_value let(:projects) do [ + project, project_with_cf_value(*id_by_name.fetch_values("10.10.10")), # 10.10.10 project_with_cf_value(*id_by_name.fetch_values("9", "10.10.10")), # 10.10.10, 9 project_with_cf_value(*id_by_name.fetch_values("10.10.10", "9")), # 10.10.10, 9 project_with_cf_value(*id_by_name.fetch_values("10.10.2")), # 10.10.2 project_with_cf_value(*id_by_name.fetch_values("10.2", "10.10.2")), # 10.10.2, 10.2 project_with_cf_value(*id_by_name.fetch_values("10.10.2", "9")), # 10.10.2, 9 - project # TODO: should be at index 0 ] end let(:projects_desc) do indexes = projects.each_index.to_a - # order of values for a work package is ignored, so ordered by falling back on id asc - indexes[1...3] = indexes[1...3].reverse + # order of work packages with same values in different order falls back on next column (id asc) + indexes[2...4] = indexes[2...4].reverse projects.values_at(*indexes.reverse) end end diff --git a/spec/models/queries/work_packages/selects/shared_query_select_specs.rb b/spec/models/queries/work_packages/selects/shared_query_select_specs.rb index 4fd9758d4c28..8df7f50fddc2 100644 --- a/spec/models/queries/work_packages/selects/shared_query_select_specs.rb +++ b/spec/models/queries/work_packages/selects/shared_query_select_specs.rb @@ -29,25 +29,25 @@ RSpec.shared_examples_for "query column" do |sortable_by_default: false| describe "#groupable" do it "is the name if true is provided" do - instance.groupable = true + instance.instance_variable_set(:@groupable, true) expect(instance.groupable).to eql(instance.name.to_s) end it "is the value if something truthy is provided" do - instance.groupable = "lorem ipsum" + instance.instance_variable_set(:@groupable, "lorem ipsum") expect(instance.groupable).to eql("lorem ipsum") end it "is false if false is provided" do - instance.groupable = false + instance.instance_variable_set(:@groupable, false) expect(instance.groupable).to be_falsey end - it "is false if nothing is provided" do - instance.groupable = nil + it "is false if nil is provided" do + instance.instance_variable_set(:@groupable, nil) expect(instance.groupable).to be_falsey end @@ -55,43 +55,39 @@ describe "#sortable" do it "is the name if true is provided" do - instance.sortable = true + instance.instance_variable_set(:@sortable, true) expect(instance.sortable).to eql(instance.name.to_s) end it "is the value if something truthy is provided" do - instance.sortable = "lorem ipsum" + instance.instance_variable_set(:@sortable, "lorem ipsum") expect(instance.sortable).to eql("lorem ipsum") end it "is false if false is provided" do - instance.sortable = false + instance.instance_variable_set(:@sortable, false) expect(instance.sortable).to be_falsey end - it "is false if nothing is provided" do - instance.sortable = nil + it "is false if nil is provided" do + instance.instance_variable_set(:@sortable, nil) expect(instance.sortable).to be_falsey end end describe "#groupable?" do - it "is false by default" do - expect(instance).not_to be_groupable - end - it "is true if told so" do - instance.groupable = true + instance.instance_variable_set(:@groupable, true) expect(instance).to be_groupable end it "is true if a value is provided (e.g. for specifying sql code)" do - instance.groupable = "COALESCE(null, 1)" + instance.instance_variable_set(:@groupable, "COALESCE(null, 1)") expect(instance).to be_groupable end @@ -107,13 +103,13 @@ end it "is true if told so" do - instance.sortable = true + instance.instance_variable_set(:@sortable, true) expect(instance).to be_sortable end it "is true if a value is provided (e.g. for specifying sql code)" do - instance.sortable = "COALESCE(null, 1)" + instance.instance_variable_set(:@sortable, "COALESCE(null, 1)") expect(instance).to be_sortable end diff --git a/spec/models/query/results_cf_sorting_integration_spec.rb b/spec/models/query/results_cf_sorting_integration_spec.rb index 50cf643c9a41..2983658be07e 100644 --- a/spec/models/query/results_cf_sorting_integration_spec.rb +++ b/spec/models/query/results_cf_sorting_integration_spec.rb @@ -188,11 +188,11 @@ def wp_without_cf_value let(:work_packages) do [ + wp_without_cf_value, # sorting is done by position, and not by value wp_with_cf_value(id_by_value.fetch("100")), wp_with_cf_value(id_by_value.fetch("3")), wp_with_cf_value(id_by_value.fetch("20")), - wp_without_cf_value # TODO: should be at index 0 ] end end @@ -204,6 +204,7 @@ def wp_without_cf_value let(:work_packages) do [ + wp_without_cf_value, wp_with_cf_value(id_by_value.fetch_values("100")), # 100 wp_with_cf_value(id_by_value.fetch_values("3", "100")), # 100, 3 wp_with_cf_value(id_by_value.fetch_values("3", "20", "100")), # 100, 3, 20 @@ -216,14 +217,13 @@ def wp_without_cf_value wp_with_cf_value(id_by_value.fetch_values("3")), # 3 wp_with_cf_value(id_by_value.fetch_values("3", "20")), # 3, 20 wp_with_cf_value(id_by_value.fetch_values("20")), # 20 - wp_without_cf_value # TODO: decide on order of absent values ] end let(:work_packages_desc) do indexes = work_packages.each_index.to_a - # order of values for a project is ignored, so ordered by falling back on id asc - indexes[2...8] = indexes[2...8].reverse + # order of projects with same values in different order falls back on next column (id asc) + indexes[3...9] = indexes[3...9].reverse work_packages.values_at(*indexes.reverse) end end @@ -255,11 +255,11 @@ def wp_without_cf_value let(:work_packages) do [ + wp_without_cf_value, wp_with_cf_value(id_by_login.fetch("ax")), wp_with_cf_value(id_by_login.fetch("ba")), wp_with_cf_value(id_by_login.fetch("bb1")), wp_with_cf_value(id_by_login.fetch("bb2")), - wp_without_cf_value # TODO: should be at index 0 ] end end @@ -271,20 +271,20 @@ def wp_without_cf_value let(:work_packages) do [ + wp_without_cf_value, wp_with_cf_value(id_by_login.fetch_values("ax")), # ax wp_with_cf_value(id_by_login.fetch_values("bb1", "ax")), # ax, bb1 wp_with_cf_value(id_by_login.fetch_values("ax", "bb1")), # ax, bb1 wp_with_cf_value(id_by_login.fetch_values("ba")), # ba wp_with_cf_value(id_by_login.fetch_values("bb1", "ba")), # ba, bb1 wp_with_cf_value(id_by_login.fetch_values("ba", "bb2")), # ba, bb2 - wp_without_cf_value # TODO: should be at index 0 ] end let(:work_packages_desc) do indexes = work_packages.each_index.to_a - # order of values for a project is ignored, so ordered by falling back on id asc - indexes[1...3] = indexes[1...3].reverse + # order of projects with same values in different order falls back on next column (id asc) + indexes[2...4] = indexes[2...4].reverse work_packages.values_at(*indexes.reverse) end end @@ -308,11 +308,11 @@ def wp_without_cf_value let(:work_packages) do [ + wp_without_cf_value, wp_with_cf_value(id_by_name.fetch("10.10.10")), wp_with_cf_value(id_by_name.fetch("10.10.2")), wp_with_cf_value(id_by_name.fetch("10.2")), wp_with_cf_value(id_by_name.fetch("9")), - wp_without_cf_value # TODO: should be at index 0 ] end end @@ -324,20 +324,20 @@ def wp_without_cf_value let(:work_packages) do [ + wp_without_cf_value, wp_with_cf_value(id_by_name.fetch_values("10.10.10")), # 10.10.10 wp_with_cf_value(id_by_name.fetch_values("9", "10.10.10")), # 10.10.10, 9 wp_with_cf_value(id_by_name.fetch_values("10.10.10", "9")), # 10.10.10, 9 wp_with_cf_value(id_by_name.fetch_values("10.10.2")), # 10.10.2 wp_with_cf_value(id_by_name.fetch_values("10.2", "10.10.2")), # 10.10.2, 10.2 wp_with_cf_value(id_by_name.fetch_values("10.10.2", "9")), # 10.10.2, 9 - wp_without_cf_value # TODO: should be at index 0 ] end let(:work_packages_desc) do indexes = work_packages.each_index.to_a - # order of values for a project is ignored, so ordered by falling back on id asc - indexes[1...3] = indexes[1...3].reverse + # order of projects with same values in different order falls back on next column (id asc) + indexes[2...4] = indexes[2...4].reverse work_packages.values_at(*indexes.reverse) end end