Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 65 additions & 3 deletions app/models/custom_field/order_statements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
8 changes: 4 additions & 4 deletions app/models/query/results/group_by.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 },
Expand Down
43 changes: 16 additions & 27 deletions lib/api/decorators/aggregation_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/api/v3/utilities/endpoints/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
34 changes: 31 additions & 3 deletions spec/models/custom_field/order_statements_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -56,6 +83,7 @@
SQL
)
end
# rubocop:enable RSpec/ExampleLength
end
end
end
Loading