[59920] Fixed group_by for single and multiselct hierarchy custom field

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

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 },

lib/api/decorators/aggregation_group.rb

@@ -33,21 +33,28 @@ 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),
+                title: group_key.to_s
+              }
+            ]
+          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 +73,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

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)

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