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

[#58520] add an endpoint to fetch the hierarchy #17075

Merged

Conversation

Kharonus
Copy link
Member

@Kharonus Kharonus commented Oct 29, 2024

Ticket

OP#58520

What are you trying to accomplish?

  • we need an API endpoint to fetch the hierarchy structure of a custom field of type hierarchy
  • the tree is represented as a depth-first flat list

What approach did you choose and why?

  • adding an endpoint GET /custom_fields/:id/items
  • adding an endpoint GET /custom_field_items/:id (needed for the self links)
  • adding API specification for both
  • adding parameter validation contract for the tree structure
  • amended legacy tests

Hint for review

This review is easily separable into

a) the API specification files, located under /docs
b) the code, in /app, /lib/api, and /spec

those could be separate PRs, but sadly I have those changes on the same commits.

@Kharonus Kharonus self-assigned this Oct 29, 2024
@Kharonus Kharonus force-pushed the implementation/58520-api-add-endpoint-for-fetching-the-tree branch from 5e1db5c to d6ac7fb Compare October 29, 2024 11:08
@Kharonus Kharonus requested a review from a team October 29, 2024 11:24
Copy link
Contributor

@brunopagno brunopagno left a comment

Choose a reason for hiding this comment

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

I say the API spec looks beautiful. A work of art. Deserving of praise.
🎉

@Kharonus Kharonus marked this pull request as ready for review November 4, 2024 15:34
@Kharonus Kharonus requested review from brunopagno and a team November 4, 2024 15:35
@Kharonus Kharonus requested review from apfohl and mereghost November 5, 2024 10:16
- implement hashed subtree retrieval in persistence layer
- implement flattening of hashed subtree in applicaiton layer
- added api spec for GET /custom_field_items/:id route
- added self link, parent and children to http response
- added api for single hierarchy item route
- write unit tests for hashed_subtree in HierarchicalItemService
- add parameter validation to api endpoint
@Kharonus Kharonus force-pushed the implementation/58520-api-add-endpoint-for-fetching-the-tree branch from cb062fe to a8c0ec5 Compare November 5, 2024 12:02
Copy link
Contributor

@mereghost mereghost left a comment

Choose a reason for hiding this comment

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

LGTM.

@Kharonus Kharonus merged commit 50022b9 into dev Nov 5, 2024
13 checks passed
@Kharonus Kharonus deleted the implementation/58520-api-add-endpoint-for-fetching-the-tree branch November 5, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants