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

allow force-ordering hierarchy nodes hierarchically #5420

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

ajtmccarty
Copy link
Contributor

@ajtmccarty ajtmccarty commented Jan 9, 2025

fixes #5292
IFC-1025

the root cause of the RecursionError is a problem in how we calculate the hierarchy for nodes in a diff
if a hierarchical schema has order_by set, then the nodes returned from the NodeGetHierarchyQuery will be returned according to that order_by value. The DiffHierarchyEnricher expects the nodes returned by NodeGetHierarchyQuery to be in their hierarchical order and, if they are in a different order, the results will be unexpected and can cause the diff to be illegally constructed with nodes referencing one another as their parents

the solution in this PR is to add a new hierarchical_ordering parameter to NodeGetHierarchyQuery and set that to True inside of DiffHierarchyEnricher

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Jan 9, 2025
Copy link

codspeed-hq bot commented Jan 9, 2025

CodSpeed Performance Report

Merging #5420 will not alter performance

Comparing ajtm-01092025-hierarchy-ordering (74ed844) with stable (7e6f318)

Summary

✅ 10 untouched benchmarks

@ajtmccarty ajtmccarty force-pushed the ajtm-01092025-hierarchy-ordering branch from 9565a2c to df9119f Compare January 9, 2025 22:52
@github-actions github-actions bot added the type/documentation Improvements or additions to documentation label Jan 9, 2025
@ajtmccarty ajtmccarty marked this pull request as ready for review January 9, 2025 23:19
@ajtmccarty ajtmccarty requested a review from a team January 9, 2025 23:19
Copy link
Collaborator

@dgarros dgarros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@dgarros dgarros merged commit 7285441 into stable Jan 10, 2025
34 checks passed
@dgarros dgarros deleted the ajtm-01092025-hierarchy-ordering branch January 10, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/backend Issue related to the backend (API Server, Git Agent) type/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: recursion error when opening a proposed change
2 participants