diff --git a/app/models/custom_field/order_statements.rb b/app/models/custom_field/order_statements.rb index 3f3153db0613..ccf043c36a30 100644 --- a/app/models/custom_field/order_statements.rb +++ b/app/models/custom_field/order_statements.rb @@ -75,7 +75,7 @@ def group_by_statement # 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" + return unless field_format == "list" || field_format == "hierarchy" # MIN needed to not add this column to group by, ANY_VALUE can be used when # minimum required PostgreSQL becomes 16 @@ -171,7 +171,69 @@ def join_for_order_by_version_sql end def join_for_order_by_hierarchy_sql - table_name = CustomField::Hierarchy::Item.quoted_table_name - join_for_order_sql(value: "item.label", join: "INNER JOIN #{table_name} item ON item.id = cv.value::bigint") + ancestor_item = CustomField.find(id).hierarchy_root + self_and_descendants = ancestor_item.self_and_descendants + total_descendants = self_and_descendants.count + max_depth = self_and_descendants.max_by(&:depth).depth + items = hierarchy_position_sql(ancestor_id: ancestor_item.id, total_descendants:, max_depth:) + + join_for_order_sql( + value: multi_value? ? "ARRAY_AGG(item.position ORDER BY item.position)" : "item.position", + add_select: "#{multi_value? ? "ARRAY_TO_STRING(ARRAY_AGG(cv.value ORDER BY item.position), '.')" : 'cv.value'} ids", + join: "INNER JOIN (#{items}) item ON item.id = cv.value::bigint", + multi_value: + ) + end + + # Template method for providing correct ordering of hierarchy items as a flat list for grouping + # + # In order to have groups correctly build with the structures above, a flat list of items is required like for + # CustomOptions. CustomOptions naturally have a position field that encodes the ordering: + # + # | id | name | position | custom_field_id + # ---------------------------------------- + # | 12 | foo | 1 | 9 + # | 14 | foo | 2 | 9 + # | 16 | foo | 3 | 9 + # + # For Hierarchy::Items this is different. They are organised in generations as they resemble a tree structure. + # The `sort_order` field is used to encode the position in each generation: + # + # | id | name | parent_id | sort_order | custom_field_id + # ------------------------------------------------------ + # | 12 | foo | 1 | 0 | 9 + # | 14 | bar | 12 | 0 | 9 + # | 16 | baz | 2 | 1 | 9 + # + # The `sort_order` field is not neatly consecutive numbers as for the CustomOption. That make it impossible to collect + # the correct work packages for each group (see code). + # + # To overcome this problem the algorithms of closure_tree can be used. This method implements the same concept of + # `self_and_descendants_preordered` from closure_tree. It uses the mathematical power operation to compute distinct + # position values from `sort_order`, the total number of descendants and the maximum depth of the tree + # (see implementation). + def hierarchy_position_sql(ancestor_id:, total_descendants:, max_depth:) + <<-SQL.squish + SELECT hi.id, + hi.parent_id, + SUM((1 + anc.sort_order) * power(#{total_descendants}, #{max_depth + 1} - depths.generations)) AS position, + hi.label, + hi.short, + hi.is_deleted, + hi.created_at, + hi.updated_at, + hi.custom_field_id + FROM hierarchical_items hi + INNER JOIN hierarchical_item_hierarchies hih + ON hi.id = hih.descendant_id + JOIN hierarchical_item_hierarchies anc_h + ON anc_h.descendant_id = hih.descendant_id + JOIN hierarchical_items anc + ON anc.id = anc_h.ancestor_id + JOIN hierarchical_item_hierarchies depths + ON depths.ancestor_id = #{ancestor_id} AND depths.descendant_id = anc.id + WHERE hih.ancestor_id = #{ancestor_id} + GROUP BY hi.id + SQL end end diff --git a/app/models/query/results/group_by.rb b/app/models/query/results/group_by.rb index efe8d4f18112..6ee9d9ef2805 100644 --- a/app/models/query/results/group_by.rb +++ b/app/models/query/results/group_by.rb @@ -101,9 +101,9 @@ def transform_hierarchy_custom_field_keys(custom_field, groups) groups.transform_keys do |key| if custom_field.multi_value? - Array(key&.split(".")).map { |subkey| items[subkey] } + Array(key&.split(".")).map { |subkey| items[subkey].first } else - items[key] || nil + items[key] || [] end end end @@ -116,8 +116,8 @@ def hierarchy_items_for_keys(custom_field, groups) .new .get_descendants(item: custom_field.hierarchy_root, include_self: false) .fmap do |list| - list.filter_map { |item| CustomField::Hierarchy::HierarchyItemAdapter.new(item:) if keys.include?(item.label) } - .index_by(&:label) + list.filter_map { |item| CustomField::Hierarchy::HierarchyItemAdapter.new(item:) if keys.include?(item.id.to_s) } + .group_by { |item| item.id.to_s } end .either( ->(list) { list }, diff --git a/lib/api/decorators/aggregation_group.rb b/lib/api/decorators/aggregation_group.rb index ec343ba83bbb..ce2763211e47 100644 --- a/lib/api/decorators/aggregation_group.rb +++ b/lib/api/decorators/aggregation_group.rb @@ -33,21 +33,27 @@ def initialize(group_key, count, query:, current_user:) @count = count @query = query - @link = ::API::V3::Utilities::ResourceLinkGenerator.make_link( - group_key.is_a?(Array) ? set_links!(group_key) : group_key - ) + @links = + if group_key.is_a?(Array) + group_key.map do |element| + { + href: ::API::V3::Utilities::ResourceLinkGenerator.make_link(element), + title: element.to_s + } + end + else + [ + { + href: ::API::V3::Utilities::ResourceLinkGenerator.make_link(group_key) + } + ] + end super(group_key, current_user:) end links :valueLink do - if @links - @links - elsif @link - [{ href: @link }] - else - [] - end + @links end property :value, @@ -66,23 +72,6 @@ def model_required? attr_reader :count, :query - ## - # Initializes the links collection for this group if the group has multiple keys - # - # @return [String] A new group key for the multi value custom field. - def set_links!(group_key) - @links = group_key.map do |opt| - { - href: ::API::V3::Utilities::ResourceLinkGenerator.make_link(opt), - title: opt.to_s - } - end - - if group_key.present? - group_key.map(&:name).sort.join(", ") - end - end - def value case represented when TrueClass, FalseClass diff --git a/lib/api/v3/utilities/endpoints/index.rb b/lib/api/v3/utilities/endpoints/index.rb index a79cc5c61152..8690d786f610 100644 --- a/lib/api/v3/utilities/endpoints/index.rb +++ b/lib/api/v3/utilities/endpoints/index.rb @@ -117,7 +117,7 @@ def calculate_resulting_params(query, provided_params) end def calculate_groups(query) - return unless query.respond_to?(:group_by) && query.group_by + return if !query.respond_to?(:group_by) || !query.group_by query.group_values.map do |group, count| ::API::Decorators::AggregationGroup.new(group, count, query:, current_user: User.current) diff --git a/spec/models/custom_field/order_statements_spec.rb b/spec/models/custom_field/order_statements_spec.rb index 79f724ee1fd8..c999399528c4 100644 --- a/spec/models/custom_field/order_statements_spec.rb +++ b/spec/models/custom_field/order_statements_spec.rb @@ -33,20 +33,47 @@ RSpec.describe CustomField::OrderStatements do # integration tests at spec/models/query/results_cf_sorting_integration_spec.rb context "when hierarchy" do + let(:service) { CustomFields::Hierarchy::HierarchicalItemService.new } + let(:item) { service.generate_root(custom_field).value! } + subject(:custom_field) { create(:hierarchy_wp_custom_field) } + before do + service.insert_item(parent: item, label: "Test") + end + describe "#order_statement" do it { expect(subject.order_statement).to eq("cf_order_#{custom_field.id}.value") } end describe "#order_join_statement" do - it do + # rubocop:disable RSpec/ExampleLength + it "must be equal" do expect(custom_field.order_join_statement).to eq( <<-SQL.squish LEFT OUTER JOIN ( - SELECT DISTINCT ON (cv.customized_id) cv.customized_id , item.label "value" + SELECT DISTINCT ON (cv.customized_id) cv.customized_id , item.position "value" , cv.value ids FROM "custom_values" cv - INNER JOIN "hierarchical_items" item ON item.id = cv.value::bigint + INNER JOIN (SELECT hi.id, + hi.parent_id, + SUM((1 + anc.sort_order) * power(2, 2 - depths.generations)) AS position, + hi.label, + hi.short, + hi.is_deleted, + hi.created_at, + hi.updated_at, + hi.custom_field_id + FROM hierarchical_items hi + INNER JOIN hierarchical_item_hierarchies hih + ON hi.id = hih.descendant_id + JOIN hierarchical_item_hierarchies anc_h + ON anc_h.descendant_id = hih.descendant_id + JOIN hierarchical_items anc + ON anc.id = anc_h.ancestor_id + JOIN hierarchical_item_hierarchies depths + ON depths.ancestor_id = #{item.id} AND depths.descendant_id = anc.id + WHERE hih.ancestor_id = #{item.id} + GROUP BY hi.id) item ON item.id = cv.value::bigint WHERE cv.customized_type = 'WorkPackage' AND cv.custom_field_id = #{custom_field.id} AND cv.value IS NOT NULL @@ -56,6 +83,7 @@ SQL ) end + # rubocop:enable RSpec/ExampleLength end end end