Skip to content

Commit

Permalink
fix: org unit filter dialog crash (DHIS2-13816) (#2118)
Browse files Browse the repository at this point in the history
Fixes https://dhis2.atlassian.net/browse/DHIS2-13816.

The OrgUnitDialog from analytics had a breaking change in v21.0.0, a different set of props is needed to make it work.

Several Cypress tests were updated to work with the new OrgUnitDimension component. There are still 4 cypress tests failing because of a real bug in the app.

(cherry picked from commit c1ce958)
  • Loading branch information
edoardo committed Sep 29, 2022
1 parent 2980782 commit 4e1ebef
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 83 deletions.
2 changes: 1 addition & 1 deletion cypress/elements/dashboardFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ export const filterDimensionsPanelSel = '[data-test="dashboard-filter-popover"]'
export const unselectedItemsSel =
'[data-test*="dimension-transfer-sourceoptions"]'

export const orgUnitCheckboxesSel = '*[role="button"]'
export const orgUnitTreeSel = '[data-test="org-unit-tree"]'

export const dimensionsModalSel = '[data-test="dimension-modal"]'
15 changes: 6 additions & 9 deletions cypress/integration/common/view/add_a_FILTERTYPE_filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ import { When } from 'cypress-cucumber-preprocessor/steps'
import {
unselectedItemsSel,
filterDimensionsPanelSel,
dimensionsModalSel,
orgUnitCheckboxesSel,
orgUnitTreeSel,
} from '../../../elements/dashboardFilter.js'
import { EXTENDED_TIMEOUT } from '../../../support/utils.js'

const PERIOD = 'Last 6 months'
const OU = 'Sierra Leone'
const OU_ID = 'ImspTQPwCqd' //Sierra Leone
const FACILITY_TYPE = 'Clinic'

When('I add a {string} filter', (dimensionType) => {
Expand All @@ -20,12 +19,10 @@ When('I add a {string} filter', (dimensionType) => {
// select an item in the modal
if (dimensionType === 'Period') {
cy.get(unselectedItemsSel).contains(PERIOD).dblclick()
} else if (dimensionType === 'Organisation Unit') {
cy.get(dimensionsModalSel, EXTENDED_TIMEOUT).find('.arrow').click()
cy.get(dimensionsModalSel, EXTENDED_TIMEOUT)
.find(orgUnitCheckboxesSel, EXTENDED_TIMEOUT)
.contains(OU, EXTENDED_TIMEOUT)
.click()
} else if (dimensionType === 'Organisation unit') {
cy.get(orgUnitTreeSel, EXTENDED_TIMEOUT)
.find('[type="checkbox"]', EXTENDED_TIMEOUT)
.check(OU_ID)
} else {
cy.get(unselectedItemsSel).contains(FACILITY_TYPE).dblclick()
}
Expand Down
7 changes: 5 additions & 2 deletions cypress/integration/edit/edit_dashboard/edit_dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@ import {
dashboardChipSel,
dashboardTitleSel,
} from '../../../elements/viewDashboard.js'
import { EXTENDED_TIMEOUT } from '../../../support/utils.js'
import {
EXTENDED_TIMEOUT,
createDashboardTitle,
} from '../../../support/utils.js'

// the length of the root route of the app (after the slash): #/
const ROOT_ROUTE_LENGTH = 0
// the length of UIDs (after the slash): '#/nghVC4wtyzi'
const UID_LENGTH = 11

export const TEST_DASHBOARD_TITLE = 'aa' + new Date().toUTCString()
export const TEST_DASHBOARD_TITLE = createDashboardTitle('aa')

const ROUTE_EDIT = 'edit'
const ROUTE_NEW = 'new'
Expand Down
3 changes: 2 additions & 1 deletion cypress/integration/edit/filter_restrict/filter_restrict.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import {
dashboardTitleSel,
clickViewActionButton,
} from '../../../elements/viewDashboard.js'
import { createDashboardTitle } from '../../../support/utils.js'

const TEST_DASHBOARD_TITLE = `aaa-${new Date().toUTCString()}`
const TEST_DASHBOARD_TITLE = createDashboardTitle('aaa')

let dashboardId

Expand Down
13 changes: 6 additions & 7 deletions cypress/integration/view/dashboard_filter.feature
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
Feature: Dashboard filter

# FIXME: needs adjustments in order to work
# Scenario: I add a Period filter
# When I start a new dashboard
# And I add a MAP and a CHART and save
# Then the dashboard displays in view mode
# When I add a "Period" filter
# Then the Period filter is applied to the dashboard
Scenario: I add a Period filter
When I start a new dashboard
And I add a MAP and a CHART and save
Then the dashboard displays in view mode
When I add a "Period" filter
Then the Period filter is applied to the dashboard

Scenario: I add a Organisation unit filter
Given I open existing dashboard
Expand Down
7 changes: 5 additions & 2 deletions cypress/integration/view/dashboard_filter/create_dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import {
dashboardChipSel,
dashboardTitleSel,
} from '../../../elements/viewDashboard.js'
import { EXTENDED_TIMEOUT } from '../../../support/utils.js'
import {
EXTENDED_TIMEOUT,
createDashboardTitle,
} from '../../../support/utils.js'

const TEST_DASHBOARD_TITLE = 'a filter ' + new Date().toUTCString()
const TEST_DASHBOARD_TITLE = createDashboardTitle('af')

When('I add a MAP and a CHART and save', () => {
//add the title
Expand Down
10 changes: 6 additions & 4 deletions cypress/integration/view/dashboard_filter/dashboard_filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ Then('the Period filter is applied to the dashboard', () => {

// check the CHART
cy.get(`${gridItemSel}.VISUALIZATION`)
.find(chartSubtitleSel, EXTENDED_TIMEOUT)
.scrollIntoView()
.contains(PERIOD, EXTENDED_TIMEOUT)
.should('be.visible')
.find(`${chartSubtitleSel} > title`, EXTENDED_TIMEOUT)
.invoke('text')
.then((text) => {
const commas = (text.match(/,/g) || []).length
expect(commas).to.equal(5) // a list of 6 months has 5 commas
})

cy.get(innerScrollContainerSel).scrollTo('top')
// check the MAP
Expand Down
6 changes: 3 additions & 3 deletions cypress/integration/view/offline/offline.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
EXTENDED_TIMEOUT,
goOnline,
goOffline,
createDashboardTitle,
} from '../../../support/utils.js'

beforeEach(() => {
Expand All @@ -33,9 +34,8 @@ const CACHED_DASHBOARD_ITEM_NAME = 'ANC: 1 and 3 coverage Yearly'
const UNCACHED_DASHBOARD_ITEM_NAME = 'ANC: 1-3 trend lines last 12 months'
const MAKE_AVAILABLE_OFFLINE_TEXT = 'Make available offline'

const UNCACHED_DASHBOARD_TITLE =
'aa un' + new Date().toUTCString().slice(-12, -4)
const CACHED_DASHBOARD_TITLE = 'aa ca' + new Date().toUTCString().slice(-12, -4)
const UNCACHED_DASHBOARD_TITLE = createDashboardTitle('aa un')
const CACHED_DASHBOARD_TITLE = createDashboardTitle('aa ca')

const createDashboard = (cacheState) => {
const cachedDashboard = cacheState === CACHED
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ import {
dashboardChipSel,
dashboardTitleSel,
} from '../../../elements/viewDashboard.js'
import { EXTENDED_TIMEOUT } from '../../../support/utils.js'
import {
EXTENDED_TIMEOUT,
createDashboardTitle,
} from '../../../support/utils.js'

const TEST_DASHBOARD_TITLE =
'0filterfail' + new Date().toUTCString().slice(-12, -4)
export const TEST_DASHBOARD_TITLE = createDashboardTitle('0ff')

// Scenario: Item visualization fails when filter applied [DHIS2-11303]

Expand Down
4 changes: 4 additions & 0 deletions cypress/support/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ export const goOnline = () => {
})
})
}

export const createDashboardTitle = (prefix) => {
return prefix + new Date().toUTCString().slice(-12, -4)
}
55 changes: 38 additions & 17 deletions src/AppWrapper.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CachedDataQueryProvider } from '@dhis2/analytics'
import { useDataEngine } from '@dhis2/app-runtime'
import { D2Shim } from '@dhis2/app-runtime-adapter-d2'
import React from 'react'
Expand Down Expand Up @@ -30,28 +31,48 @@ if (authorization) {
d2Config.headers = { Authorization: authorization }
}

const query = {
rootOrgUnits: {
resource: 'organisationUnits',
params: {
fields: 'id,displayName,name',
userDataViewFallback: true,
paging: false,
},
},
}

const providerDataTransformation = ({ rootOrgUnits }) => ({
rootOrgUnits: rootOrgUnits.organisationUnits,
})

const AppWrapper = () => {
const dataEngine = useDataEngine()

return (
<ReduxProvider store={configureStore(dataEngine)}>
<D2Shim d2Config={d2Config} i18nRoot="./i18n">
{({ d2 }) => {
if (!d2) {
// TODO: Handle errors in d2 initialization
return null
}
return (
<SystemSettingsProvider>
<UserSettingsProvider>
<WindowDimensionsProvider>
<App />
</WindowDimensionsProvider>
</UserSettingsProvider>
</SystemSettingsProvider>
)
}}
</D2Shim>
<CachedDataQueryProvider
query={query}
dataTransformation={providerDataTransformation}
>
<D2Shim d2Config={d2Config} i18nRoot="./i18n">
{({ d2 }) => {
if (!d2) {
// TODO: Handle errors in d2 initialization
return null
}
return (
<SystemSettingsProvider>
<UserSettingsProvider>
<WindowDimensionsProvider>
<App />
</WindowDimensionsProvider>
</UserSettingsProvider>
</SystemSettingsProvider>
)
}}
</D2Shim>
</CachedDataQueryProvider>
</ReduxProvider>
)
}
Expand Down
4 changes: 4 additions & 0 deletions src/__tests__/AppWrapper.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import React from 'react'
import ReactDOM from 'react-dom'
import AppWrapper from '../AppWrapper.js'

jest.mock('@dhis2/analytics', () => ({
...jest.requireActual('@dhis2/analytics'),
CachedDataQueryProvider: () => <div className="CachedDataQueryProvider" />,
}))
jest.mock('@dhis2/app-runtime-adapter-d2', () => {
return {
D2Shim: ({ children }) => children({ d2: {} }),
Expand Down
43 changes: 9 additions & 34 deletions src/pages/view/TitleBar/FilterDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {
BIWEEKLY,
MONTHLY,
BIMONTHLY,
useCachedDataQuery,
} from '@dhis2/analytics'
import { useD2 } from '@dhis2/app-runtime-adapter-d2'
import i18n from '@dhis2/d2-i18n'
import {
Button,
Expand Down Expand Up @@ -43,32 +43,14 @@ const FilterDialog = ({
onClose,
}) => {
const [filters, setFilters] = useState(initiallySelectedItems)
const { d2 } = useD2()
const { userSettings } = useUserSettings()
const { systemSettings } = useSystemSettings()
const { rootOrgUnits } = useCachedDataQuery()

const onSelectItems = ({ dimensionId, items }) => {
setFilters({ [dimensionId]: items })
}

const onDeselectItems = ({ dimensionId, itemIdsToRemove }) => {
const oldList = filters[dimensionId] || []
const newList = oldList.filter(
(item) => !itemIdsToRemove.includes(item.id)
)

setFilters({ ...filters, [dimensionId]: newList })
}

const onReorderItems = ({ dimensionId, itemIds }) => {
const oldList = filters[dimensionId] || []
const reorderedList = itemIds.map((id) =>
oldList.find((item) => item.id === id)
)

setFilters({ ...filters, [dimensionId]: reorderedList })
}

const saveFilter = () => {
const id = dimension.id
const filterItems = filters[id]
Expand Down Expand Up @@ -106,21 +88,14 @@ const FilterDialog = ({
}

const renderDialogContent = () => {
const commonProps = {
d2,
onSelect: onSelectItems,
onDeselect: onDeselectItems,
onReorder: onReorderItems,
}

const selectedItems = filters[dimension.id] || []

switch (dimension.id) {
case DIMENSION_ID_PERIOD: {
return (
<PeriodDimension
selectedPeriods={selectedItems}
onSelect={commonProps.onSelect}
onSelect={onSelectItems}
excludedPeriodTypes={getExcludedPeriodTypes(
systemSettings
)}
Expand All @@ -130,19 +105,19 @@ const FilterDialog = ({
case DIMENSION_ID_ORGUNIT:
return (
<OrgUnitDimension
displayNameProperty={
userSettings.keyAnalysisDisplayProperty
}
ouItems={selectedItems}
{...commonProps}
roots={rootOrgUnits.map(
(rootOrgUnit) => rootOrgUnit.id
)}
selected={selectedItems}
onSelect={onSelectItems}
/>
)
default:
return (
<DynamicDimension
selectedItems={selectedItems}
dimensionId={dimension.id}
onSelect={commonProps.onSelect}
onSelect={onSelectItems}
dimensionTitle={dimension.name}
displayNameProp={
userSettings.keyAnalysisDisplayProperty
Expand Down

0 comments on commit 4e1ebef

Please sign in to comment.