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

fix(OrganisationUnitTree): deduplicate orgunit roots #1625

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Birkbjo
Copy link
Member

@Birkbjo Birkbjo commented Oct 29, 2024

Implements LIBS-702


Description

In OrganisationUnitTree component, you can pass roots that are descendants of other roots, e.g. if you pass <OrganisationUnitTree roots=[OrgUnitA, OrgUnitB, OrgUnitC] /> where OrgUnitB is a descendant of OrgUnitA and where OrgUnitC is a descendant of OrgUnitA, then you presumably want to display only OrgUnitA as a root (e.g. the deduplicated root).

This PR implements a function deduplicateOrgUnitRoots that will check the levels of the units, and only return the true "minimal roots" - and ignore rendering input IDs as roots, if another root contains it.

Know issues

  • This will still send a request for each input orgunit. But will filter the "nested" units before rendering, once we know the "level".
    • Sidenote: I really think we should update how the roots are fetched. We do not need a separate request for each unit - those can be grabbed by using an in-filter. It quite wasteful currently.
      • This would be simple in it self, but would probably require a lot of work to update tests - and some apps' test might break if we were to do that.

Checklist

  • API docs are generated
  • Tests were added
  • Storybook demos were added

All points above should be relevant for feature PRs. For bugfixes, some points might not be relevant. In that case, just check them anyway to signal the work is done.


@Birkbjo Birkbjo requested a review from a team as a code owner October 29, 2024 15:20
@Birkbjo Birkbjo changed the title Fix/libs 702/deduplicate roots fix(OrganisationUnitTree): deduplicate orgunit roots Oct 29, 2024
@dhis2-bot
Copy link
Contributor

dhis2-bot commented Oct 29, 2024

🚀 Deployed on https://pr-1625--dhis2-ui.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify October 29, 2024 15:27 Inactive
* @returns {Array} - A filtered list of the minimum root units
*/
export const deduplicateOrgUnitRoots = (unitsWithLevel) => {
const sorted = unitsWithLevel.sort((a, b) => a.level - b.level)
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that we could use the "path" instead of level here - because path consists of the ids, and every id has the same length. But it's more semantically correct to use the level, even if I had to add another field to the request.

@dhis2-bot dhis2-bot temporarily deployed to netlify October 29, 2024 15:48 Inactive
@dhis2-bot dhis2-bot temporarily deployed to netlify October 30, 2024 10:57 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants