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
Open
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const createResponse = ({ fields, id, path, displayName, children }) => ({
...(fields.includes('id') ? { id } : {}),
...(fields.includes('path') ? { path } : {}),
...(fields.includes('path') ? { path, level: path.split('/').length } : {}),
...(fields.includes('displayName') ? { displayName } : {}),
...(fields.includes('children::size') ? { children: children.length } : {}),
...(fields.includes('children[id,path,displayName]') ? { children } : {}),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Find the minimum/common root units from a list of orgunits.
* This is used to "deduplicate" a list of units, where some unit in the list
* may be a parent of another, and thus only the parent should be included as a root.
*
* This is mainly because of the /me API returning the verbatim selected units,
* where the user is able to select children deep in a tree, even if an ancestor above is selected
* @param {Array} unitsWithLevel - List of orgunits with their level
* @returns {Array} - A filtered list of the minimum root units
*/
export const findCommonOrgUnitRoots = (unitsWithLevel) => {
const sorted = unitsWithLevel.sort((a, b) => a.level - b.level)

const minimumRoots = sorted.filter((ou, index, array) => {
// since the array is sorted by level we can just check the previous units,
// because we want to get the "minimum" level
const previousUnits = array.slice(0, index)
// if a previous unit has a path that is a prefix of the current path,
// then the current path is a child and should not be included
return !previousUnits.some((pu) => ou.path.startsWith(pu.path))
})

return minimumRoots
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { findCommonOrgUnitRoots } from './find-common-orgunit-roots.js'

const unitToPath = {
sierra: '/ImspTQPwCqd',
tanzania: '/N5hLlID8ihI',
ethiopia: '/LK1v9z1Jt3k',
'sierra/bo': '/ImspTQPwCqd/O6uvpzGd5pu',
'sierra/bo/bargbe': '/ImspTQPwCqd/O6uvpzGd5pu/dGheVylzol6',
'sierra/bo/bargbe/barlie':
'/ImspTQPwCqd/O6uvpzGd5pu/dGheVylzol6/y5hLlID8ihI',
'sierra/bargbe/barlie': '/ImspTQPwCqd/dGheVylzol6/y5hLlID8ihI',
'sierra/bargbe/barlie/ngalu':
'/ImspTQPwCqd/dGheVylzol6/y5hLlID8ihI/Aj5v9z1Jt3k',
'sierra/bo/baoma': '/ImspTQPwCqd/O6uvpzGd5pu/vWbkYPRmKyS',
'sierra/bo/baoma/faabu': '/ImspTQPwCqd/O6uvpzGd5pu/vWbkYPRmKyS/ZpE2POxvl9P',
'sierra/bo/badjia': '/ImspTQPwCqd/O6uvpzGd5pu/YuQRtpLP10I',
'sierra/bo/badjia/ngelehun':
'/ImspTQPwCqd/O6uvpzGd5pu/YuQRtpLP10I/DiszpKrYNg8',
'sierra/bombali': '/ImspTQPwCqd/fdc6uOvgoji',
}

describe('findCommonOrgUnitRoots', () => {
it('should return a single root unit when there is only one unit', () => {
const units = [{ path: unitToPath.sierra, level: 1 }]
const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([{ path: unitToPath.sierra, level: 1 }])
// should not mutate the input
expect(units).toStrictEqual(units)
})

it('should return two root units when there are two sibling units', () => {
const units = [
{ path: unitToPath['sierra/bo'], level: 2 },
{ path: unitToPath['sierra/bombali'], level: 2 },
]

const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([
{ path: unitToPath['sierra/bo'], level: 2 },
{ path: unitToPath['sierra/bombali'], level: 2 },
])
expect(units).toStrictEqual(units)
})

it('should return only the root unit when one unit is a child of another', () => {
const units = [
{ path: unitToPath['sierra'], level: 1 },
{ path: unitToPath['sierra/bo'], level: 2 },
]
const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([{ path: unitToPath['sierra'], level: 1 }])
expect(units).toStrictEqual(units)
})

it('should return only the root unit when one unit is a deep child of another', () => {
const units = [
{ path: unitToPath['sierra'], level: 1 },
{ path: unitToPath['sierra/bo/badjia/ngelehun'], level: 4 },
]
const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([{ path: unitToPath['sierra'], level: 1 }])
expect(units).toStrictEqual(units)
})

it('should return multiple root units when paths do not overlap', () => {
const units = [
{ path: unitToPath['sierra'], level: 1 },
{ path: unitToPath['tanzania'], level: 1 },
{ path: unitToPath['ethiopia'], level: 1 },
]
const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([
{ path: unitToPath['sierra'], level: 1 },
{ path: unitToPath['tanzania'], level: 1 },
{ path: unitToPath['ethiopia'], level: 1 },
])
expect(units).toStrictEqual(units)
})

it('should return the correct root units when there is a mix of roots and children', () => {
const units = [
{ path: unitToPath['sierra/bo/badjia/ngelehun'], level: 4 },
{ path: unitToPath['sierra/bo/baoma/faabu'], level: 4 },
{ path: unitToPath['sierra/bo/baoma'], level: 3 },
{ path: unitToPath['sierra/bo/bargbe'], level: 3 },
]
const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([
{ path: unitToPath['sierra/bo/baoma'], level: 3 },
{ path: unitToPath['sierra/bo/bargbe'], level: 3 },
{ path: unitToPath['sierra/bo/badjia/ngelehun'], level: 4 },
])
expect(units).toStrictEqual(units)
})

it('should return the root units when multiple nested children exist', () => {
const units = [
{ path: unitToPath['sierra/bo'], level: 2 },
{ path: unitToPath['sierra/bo/badjia'], level: 3 },
{ path: unitToPath['sierra/bo/baoma'], level: 3 },
{ path: unitToPath['sierra/bargbe/barlie'], level: 3 },
{ path: unitToPath['sierra/bargbe/barlie/ngalu'], level: 4 },
]
const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([
{ path: unitToPath['sierra/bo'], level: 2 },
{ path: unitToPath['sierra/bargbe/barlie'], level: 3 },
])
expect(units).toStrictEqual(units)
})

it('should handle empty input and return an empty array', () => {
const units = []
const result = findCommonOrgUnitRoots(units)
expect(result).toEqual([])
expect(units).toStrictEqual(units)
})
})
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { requiredIf } from '@dhis2/prop-types'
import PropTypes from 'prop-types'
import React, { useEffect, useState } from 'react'
import React, { useEffect, useMemo, useState } from 'react'
import { OrganisationUnitNode } from '../organisation-unit-node/index.js'
import { orgUnitPathPropType } from '../prop-types.js'
import { findCommonOrgUnitRoots } from './find-common-orgunit-roots.js'

Check failure on line 6 in components/organisation-unit-tree/src/organisation-unit-tree/organisation-unit-tree.js

View workflow job for this annotation

GitHub Actions / lint

`./find-common-orgunit-roots.js` import should occur after import of `./filter-root-ids.js`
import { defaultRenderNodeLabel } from './default-render-node-label/index.js'
import { filterRootIds } from './filter-root-ids.js'
import { OrganisationUnitTreeRootError } from './organisation-unit-tree-root-error.js'
Expand Down Expand Up @@ -49,6 +50,13 @@
suppressAlphabeticalSorting,
})

const rootNodes = useMemo(() => {
if (!data) {
return []
}
return findCommonOrgUnitRoots(Object.values(data))
}, [data])

const { expanded, handleExpand, handleCollapse } = useExpanded({
initiallyExpanded,
onExpand,
Expand Down Expand Up @@ -79,36 +87,32 @@
{error && <OrganisationUnitTreeRootError error={error} />}
{!error &&
!isLoading &&
rootIds.map((rootId) => {
const rootNode = data[rootId]

return (
<OrganisationUnitNode
key={rootNode.path}
rootId={rootId}
autoExpandLoadingError={autoExpandLoadingError}
dataTest={dataTest}
disableSelection={disableSelection}
displayName={rootNode.displayName}
expanded={expanded}
highlighted={highlighted}
id={rootId}
isUserDataViewFallback={isUserDataViewFallback}
filter={filter}
path={rootNode.path}
renderNodeLabel={renderNodeLabel}
selected={selected}
singleSelection={singleSelection}
suppressAlphabeticalSorting={
suppressAlphabeticalSorting
}
onChange={onChange}
onChildrenLoaded={onChildrenLoaded}
onCollapse={handleCollapse}
onExpand={handleExpand}
/>
)
})}
rootNodes.map((rootNode) => (
<OrganisationUnitNode
key={rootNode.path}
rootId={rootNode.id}
autoExpandLoadingError={autoExpandLoadingError}
dataTest={dataTest}
disableSelection={disableSelection}
displayName={rootNode.displayName}
expanded={expanded}
highlighted={highlighted}
id={rootNode.id}
isUserDataViewFallback={isUserDataViewFallback}
filter={filter}
path={rootNode.path}
renderNodeLabel={renderNodeLabel}
selected={selected}
singleSelection={singleSelection}
suppressAlphabeticalSorting={
suppressAlphabeticalSorting
}
onChange={onChange}
onChildrenLoaded={onChildrenLoaded}
onCollapse={handleCollapse}
onExpand={handleExpand}
/>
))}
</div>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const createRootQuery = (ids) =>
resource: `organisationUnits`,
params: ({ isUserDataViewFallback }) => ({
isUserDataViewFallback,
fields: ['displayName', 'path', 'id'],
fields: ['displayName', 'path', 'id', 'level'],
}),
},
}),
Expand Down
Loading