From c1a73416dfdade907a185034964be7c45c52d202 Mon Sep 17 00:00:00 2001 From: Jen Jones Arnesen Date: Fri, 26 Apr 2019 15:25:14 +0200 Subject: [PATCH] fix: org unit dimension property names and event handling (#23) Fixes include: * OrgUnit: use correct property names in returned objects * OrgUnit: handle de-selecting user org unit correctly -- if, there are no remaining user org unit selected, then revert the Selected value to whatever org units were previously selected -- otherwise, just deselect the user org unit that got unchecked * Period: remove duplicate entries in returned list of selected periods --- .../OrgUnitDimension/OrgUnitDimension.js | 41 +++++++------------ .../__tests__/OrgUnitDimension.spec.js | 10 +---- .../PeriodDimension/PeriodDimension.js | 18 +++++--- 3 files changed, 29 insertions(+), 40 deletions(-) diff --git a/src/components/OrgUnitDimension/OrgUnitDimension.js b/src/components/OrgUnitDimension/OrgUnitDimension.js index 9741021e9..3e75df5d9 100644 --- a/src/components/OrgUnitDimension/OrgUnitDimension.js +++ b/src/components/OrgUnitDimension/OrgUnitDimension.js @@ -5,11 +5,7 @@ import DialogContent from '@material-ui/core/DialogContent' import DialogTitle from '@material-ui/core/DialogTitle' import i18n from '@dhis2/d2-i18n' -import { - OrgUnitSelector, - userOrgUnits, - removeOrgUnitLastPathSegment, -} from '@dhis2/d2-ui-org-unit-dialog' +import { OrgUnitSelector, userOrgUnits } from '@dhis2/d2-ui-org-unit-dialog' import { apiFetchOrganisationUnitGroups, @@ -22,7 +18,6 @@ import { GROUP_ID_PREFIX, isLevelId, isGroupId, - getOrgUnitsFromIds, getLevelsFromIds, getGroupsFromIds, sortOrgUnitLevels, @@ -76,10 +71,6 @@ class OrgUnitDimension extends Component { this.setState({ showOrgUnitsTree: false }) } - setOuUiItems = ids => { - this.props.onReorder({ dimensionType: ouId, value: ids }) - } - getUserOrgUnitsFromIds = ids => { return userOrgUnits.filter(ou => ids.includes(ou.id)) } @@ -88,8 +79,8 @@ class OrgUnitDimension extends Component { const levelIds = event.target.value.filter(id => !!id) this.props.onSelect({ - dimensionType: ouId, - value: [ + dimensionId: ouId, + items: [ ...this.props.ouItems.filter(ou => !isLevelId(ou.id)), ...levelIds.map(id => { const levelOu = this.state.ouLevels.find(ou => ou.id === id) @@ -107,8 +98,8 @@ class OrgUnitDimension extends Component { const groupIds = event.target.value.filter(id => !!id) this.props.onSelect({ - dimensionType: ouId, - value: [ + dimensionId: ouId, + items: [ ...this.props.ouItems.filter(ou => !isGroupId(ou.id)), ...groupIds.map(id => { const groupOu = this.state.ouGroups.find(ou => ou.id === id) @@ -124,8 +115,8 @@ class OrgUnitDimension extends Component { onDeselectAllClick = () => this.props.onDeselect({ - dimensionType: ouId, - value: this.props.ouItems.map(ou => ou.id), + dimensionId: ouId, + itemIdsToRemove: this.props.ouItems.map(ou => ou.id), }) loadOrgUnitTree = (d2, displayNameProperty) => { @@ -197,17 +188,16 @@ class OrgUnitDimension extends Component { this.props.ouItems.length === 1 && this.state.selected.length > 0 ) { - this.setOuUiItems(this.state.selected) - - this.setState({ - selected: [], + this.props.onSelect({ + dimensionId: ouId, + items: this.state.selected, + }) + } else { + this.props.onDeselect({ + dimensionId: ouId, + itemIdsToRemove: [event.target.name], }) } - - this.props.onDeselect({ - dimensionId: ouId, - items: [event.target.name], - }) } } @@ -288,7 +278,6 @@ OrgUnitDimension.propTypes = { displayNameProperty: PropTypes.string, onSelect: PropTypes.func, onDeselect: PropTypes.func, - onReorder: PropTypes.func, ouItems: PropTypes.array, current: PropTypes.object, } diff --git a/src/components/OrgUnitDimension/__tests__/OrgUnitDimension.spec.js b/src/components/OrgUnitDimension/__tests__/OrgUnitDimension.spec.js index fa4824777..ea89cddd7 100644 --- a/src/components/OrgUnitDimension/__tests__/OrgUnitDimension.spec.js +++ b/src/components/OrgUnitDimension/__tests__/OrgUnitDimension.spec.js @@ -83,14 +83,8 @@ describe('The OrgUnitDimension component ', () => { props = { d2: {}, ouItems: [], - parentGraphMap: {}, - metadata: {}, - acAddUiItems: jest.fn(), - acRemoveUiItems: jest.fn(), - acAddParentGraphMap: jest.fn(), - acAddMetadata: jest.fn(), - acSetUiItems: jest.fn(), - acSetCurrentFromUi: jest.fn(), + onSelect: jest.fn(), + onDeselect: jest.fn(), current: { id: null }, displayNameProperty: 'displayName', } diff --git a/src/components/PeriodDimension/PeriodDimension.js b/src/components/PeriodDimension/PeriodDimension.js index ec1605f88..ebbbf6a21 100644 --- a/src/components/PeriodDimension/PeriodDimension.js +++ b/src/components/PeriodDimension/PeriodDimension.js @@ -1,5 +1,6 @@ import React, { Component, Fragment } from 'react' import PropTypes from 'prop-types' +import uniqBy from 'lodash/uniqBy' import DialogTitle from '@material-ui/core/DialogTitle' import DialogContent from '@material-ui/core/DialogContent' import { PeriodSelector } from '@dhis2/d2-ui-period-selector-dialog' @@ -13,14 +14,20 @@ const PERIOD = 'PERIOD' export class PeriodDimension extends Component { selectItems = periods => { - const itemsToAdd = periods.reduce((array, item) => { - array.push({ ...item, dimensionItemType: PERIOD }) - return array - }, []) + const newItems = periods.map(p => ({ + id: p.id, + name: p.name, + dimensionItemType: PERIOD, + })) + const alreadySelected = this.props.selectedPeriods.map(p => ({ + id: p.id, + name: p.name, + dimensionItemType: PERIOD, + })) this.props.onSelect({ dimensionId: peId, - items: [...this.props.selectedPeriods, ...itemsToAdd], + items: uniqBy(alreadySelected.concat(newItems), 'id'), }) } @@ -44,7 +51,6 @@ export class PeriodDimension extends Component { render = () => { const { selectedPeriods } = this.props - console.log('PeriodDimension render with', selectedPeriods) return (