From 161660f30cd7a8fe27dbad8b2d3bb1bf041d9d87 Mon Sep 17 00:00:00 2001 From: Jen Jones Arnesen Date: Mon, 25 Sep 2023 16:05:46 +0200 Subject: [PATCH] Revert "chore: fixed period select refactor (#2958)" This reverts commit 8fdd67eb0b57e3c185cda9da3486d727dc2a64c7. --- .../integration/layers/thematiclayer.cy.js | 81 +------ i18n/en.pot | 15 -- .../calculations/CalculationSelect.js | 62 ----- .../styles/CalculationSelect.module.css | 3 - src/components/core/SelectField.js | 6 - .../edit/thematic/ThematicDialog.js | 18 -- .../edit/thematic/ValueTypeSelect.js | 40 ++-- src/components/layers/overlays/OverlayCard.js | 5 +- src/components/orgunits/OrgUnitData.js | 2 +- src/components/periods/PeriodSelect.js | 217 +++++++++--------- src/constants/dimension.js | 6 - src/util/periods.js | 38 +-- 12 files changed, 142 insertions(+), 351 deletions(-) delete mode 100644 src/components/calculations/CalculationSelect.js delete mode 100644 src/components/calculations/styles/CalculationSelect.module.css diff --git a/cypress/integration/layers/thematiclayer.cy.js b/cypress/integration/layers/thematiclayer.cy.js index 4dacacc0b..d9966fc7d 100644 --- a/cypress/integration/layers/thematiclayer.cy.js +++ b/cypress/integration/layers/thematiclayer.cy.js @@ -6,7 +6,7 @@ import { expectContextMenuOptions, } from '../../elements/map_context_menu.js' import { ThematicLayer } from '../../elements/thematic_layer.js' -import { CURRENT_YEAR, getApiBaseUrl } from '../../support/util.js' +import { CURRENT_YEAR } from '../../support/util.js' const INDICATOR_NAME = 'VCCT post-test counselling rate' @@ -136,83 +136,4 @@ context('Thematic Layers', () => { { name: VIEW_PROFILE }, ]) }) - - // TODO - update demo database with calculations instead of creating on the fly - it('adds a thematic layer with a calculation', () => { - const timestamp = new Date().toUTCString().slice(-24, -4) - const calculationName = `map calc ${timestamp}` - - // add a calculation - cy.request('POST', `${getApiBaseUrl()}/api/expressionDimensionItems`, { - name: calculationName, - shortName: calculationName, - expression: '#{fbfJHSPpUQD}/2', - }).then((response) => { - expect(response.status).to.eq(201) - - const calculationUid = response.body.response.uid - - // open thematic dialog - cy.getByDataTest('add-layer-button').click() - cy.getByDataTest('addlayeritem-thematic').click() - - // choose "Calculation" in item type - cy.getByDataTest('thematic-layer-value-type-select').click() - cy.contains('Calculations').click() - - // assert that the label on the Calculation select is "Calculation" - cy.getByDataTest('calculationselect-label').contains('Calculation') - - // click to open the calculation select - cy.getByDataTest('calculationselect').click() - - // check search box exists "Type to filter options" - cy.getByDataTest('dhis2-uicore-popper') - .find('input[type="text"]') - .should('have.attr', 'placeholder', 'Type to filter options') - - // search for something that doesn't exist - cy.getByDataTest('dhis2-uicore-popper') - .find('input[type="text"]') - .type('foo') - - cy.getByDataTest('dhis2-uicore-select-menu-menuwrapper') - .contains('No options found') - .should('be.visible') - - // try search for something that exists - cy.getByDataTest('dhis2-uicore-popper') - .find('input[type="text"]') - .clear() - - cy.getByDataTest('dhis2-uicore-popper') - .find('input[type="text"]') - .type(calculationName) - - cy.getByDataTest('dhis2-uicore-select-menu-menuwrapper') - .contains(calculationName) - .should('be.visible') - - // select the calculation and close dialog - cy.contains(calculationName).click() - - cy.getByDataTest('dhis2-uicore-modalactions') - .contains('Add layer') - .click() - - // check the layer card title - cy.getByDataTest('layercard') - .contains(calculationName, { timeout: 50000 }) - .should('be.visible') - - // check the map canvas is displayed - cy.get('canvas.maplibregl-canvas').should('be.visible') - - // delete the calculation - cy.request( - 'DELETE', - `${getApiBaseUrl()}/api/expressionDimensionItems/${calculationUid}` - ) - }) - }) }) diff --git a/i18n/en.pot b/i18n/en.pot index 86649bf58..fbce2a3d2 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -17,15 +17,6 @@ msgstr "Map \"{{- name}}\" is saved." msgid "Failed to save map: {{message}}" msgstr "Failed to save map: {{message}}" -msgid "Calculation" -msgstr "Calculation" - -msgid "No calculations found" -msgstr "No calculations found" - -msgid "Calculations can be created in the Data Visualizer app." -msgstr "Calculations can be created in the Data Visualizer app." - msgid "Classification" msgstr "Classification" @@ -430,9 +421,6 @@ msgstr "Event data item is required" msgid "Program indicator is required" msgstr "Program indicator is required" -msgid "Calculation is required" -msgstr "Calculation is required" - msgid "Period is required" msgstr "Period is required" @@ -448,9 +436,6 @@ msgstr "Event data items" msgid "Program indicators" msgstr "Program indicators" -msgid "Calculations" -msgstr "Calculations" - msgid "Item type" msgstr "Item type" diff --git a/src/components/calculations/CalculationSelect.js b/src/components/calculations/CalculationSelect.js deleted file mode 100644 index 8bdc94ad6..000000000 --- a/src/components/calculations/CalculationSelect.js +++ /dev/null @@ -1,62 +0,0 @@ -import { useDataQuery } from '@dhis2/app-runtime' -import i18n from '@dhis2/d2-i18n' -import PropTypes from 'prop-types' -import React from 'react' -import { SelectField, Help } from '../core/index.js' -import { useUserSettings } from '../UserSettingsProvider.js' -import styles from './styles/CalculationSelect.module.css' - -// Load all calculations -const CALCULATIONS_QUERY = { - calculations: { - resource: 'expressionDimensionItems', - params: ({ nameProperty }) => ({ - fields: ['id', `${nameProperty}~rename(name)`], - paging: false, - }), - }, -} - -const CalculationSelect = ({ calculation, className, errorText, onChange }) => { - const { nameProperty } = useUserSettings() - const { loading, error, data } = useDataQuery(CALCULATIONS_QUERY, { - variables: { nameProperty }, - }) - - const items = data?.calculations.expressionDimensionItems - const value = calculation?.id - - return ( -
- onChange(dataItem, 'calculation')} - className={className} - emptyText={i18n.t('No calculations found')} - errorText={ - error?.message || - (!calculation && errorText ? errorText : null) - } - filterable={true} - dataTest="calculationselect" - /> - - {i18n.t( - 'Calculations can be created in the Data Visualizer app.' - )} - -
- ) -} - -CalculationSelect.propTypes = { - onChange: PropTypes.func.isRequired, - calculation: PropTypes.object, - className: PropTypes.string, - errorText: PropTypes.string, -} - -export default CalculationSelect diff --git a/src/components/calculations/styles/CalculationSelect.module.css b/src/components/calculations/styles/CalculationSelect.module.css deleted file mode 100644 index d8cf160d2..000000000 --- a/src/components/calculations/styles/CalculationSelect.module.css +++ /dev/null @@ -1,3 +0,0 @@ -.calculationSelect { - margin-bottom: var(--spacers-dp16); -} diff --git a/src/components/core/SelectField.js b/src/components/core/SelectField.js index fa21e24c9..ecfed175d 100644 --- a/src/components/core/SelectField.js +++ b/src/components/core/SelectField.js @@ -17,7 +17,6 @@ import styles from './styles/InputField.module.css' const SelectField = (props) => { const { dense = true, - emptyText, errorText, helpText, warning, @@ -26,7 +25,6 @@ const SelectField = (props) => { prefix, loading, multiple, - filterable, disabled, onChange, className, @@ -67,14 +65,12 @@ const SelectField = (props) => { label={label} prefix={prefix} selected={!isLoading ? selected : undefined} - filterable={filterable} disabled={disabled} loading={isLoading} error={!!errorText} warning={!!warning} validationText={warning ? warning : errorText} helpText={helpText} - empty={emptyText} onChange={onSelectChange} dataTest={dataTest} > @@ -92,9 +88,7 @@ SelectField.propTypes = { dataTest: PropTypes.string, dense: PropTypes.bool, disabled: PropTypes.bool, - emptyText: PropTypes.string, // If set, shows empty text when no options errorText: PropTypes.string, // If set, shows the error message below the SelectField - filterable: PropTypes.bool, helpText: PropTypes.string, // If set, shows the help text below the SelectField items: PropTypes.arrayOf( PropTypes.shape({ diff --git a/src/components/edit/thematic/ThematicDialog.js b/src/components/edit/thematic/ThematicDialog.js index 7470902a9..263c4120f 100644 --- a/src/components/edit/thematic/ThematicDialog.js +++ b/src/components/edit/thematic/ThematicDialog.js @@ -36,7 +36,6 @@ import { } from '../../../util/analytics.js' import { isPeriodAvailable } from '../../../util/periods.js' import { getStartEndDateError } from '../../../util/time.js' -import CalculationSelect from '../../calculations/CalculationSelect.js' import NumericLegendStyle from '../../classification/NumericLegendStyle.js' import { Tab, Tabs } from '../../core/index.js' import DataElementGroupSelect from '../../dataElement/DataElementGroupSelect.js' @@ -256,7 +255,6 @@ class ThematicDialog extends Component { dataElementError, dataSetError, programError, - calculationError, eventDataItemError, programIndicatorError, periodTypeError, @@ -410,14 +408,6 @@ class ThematicDialog extends Component { /> ), ]} - {valueType === dimConf.calculation.objectName && ( - - )} @@ -619,14 +609,6 @@ class ThematicDialog extends Component { } } - if (valueType === dimConf.calculation.objectName && !dataItem) { - return this.setErrorState( - 'calculationError', - i18n.t('Calculation is required'), - 'data' - ) - } - if (!period && periodType !== START_END_DATES) { return this.setErrorState( 'periodError', diff --git a/src/components/edit/thematic/ValueTypeSelect.js b/src/components/edit/thematic/ValueTypeSelect.js index 0726f9b86..018b87ab8 100644 --- a/src/components/edit/thematic/ValueTypeSelect.js +++ b/src/components/edit/thematic/ValueTypeSelect.js @@ -1,29 +1,11 @@ import i18n from '@dhis2/d2-i18n' import PropTypes from 'prop-types' -import React, { useMemo } from 'react' +import React from 'react' import { dimConf } from '../../../constants/dimension.js' import { SelectField } from '../../core/index.js' -const getValueTypes = () => [ - { id: dimConf.indicator.objectName, name: i18n.t('Indicator') }, - { id: dimConf.dataElement.objectName, name: i18n.t('Data element') }, - { id: dimConf.dataSet.objectName, name: i18n.t('Reporting rates') }, - { - id: dimConf.eventDataItem.objectName, - name: i18n.t('Event data items'), - }, - { - id: dimConf.programIndicator.objectName, - name: i18n.t('Program indicators'), - }, - { - id: dimConf.calculation.objectName, - name: i18n.t('Calculations'), - }, -] - -const ValueTypeSelect = ({ value, onChange, className }) => { - const items = useMemo(() => getValueTypes(), []) +const ValueTypeSelect = (props) => { + const { value, onChange, className } = props // If value type is data element operand, make it data element const type = @@ -31,6 +13,21 @@ const ValueTypeSelect = ({ value, onChange, className }) => { ? dimConf.dataElement.objectName : value + // TODO: Avoid creating on each render (needs to be created after i18next contains translations + const items = [ + { id: dimConf.indicator.objectName, name: i18n.t('Indicator') }, + { id: dimConf.dataElement.objectName, name: i18n.t('Data element') }, + { id: dimConf.dataSet.objectName, name: i18n.t('Reporting rates') }, + { + id: dimConf.eventDataItem.objectName, + name: i18n.t('Event data items'), + }, + { + id: dimConf.programIndicator.objectName, + name: i18n.t('Program indicators'), + }, + ] + return ( { value={type} onChange={(valueType) => onChange(valueType.id)} className={className} - dataTest="thematic-layer-value-type-select" /> ) } diff --git a/src/components/layers/overlays/OverlayCard.js b/src/components/layers/overlays/OverlayCard.js index cfd1e86e3..7c1405dff 100644 --- a/src/components/layers/overlays/OverlayCard.js +++ b/src/components/layers/overlays/OverlayCard.js @@ -105,10 +105,7 @@ const OverlayCard = ({ await set(currentAO) // Open it in another app - window.open( - `${baseUrl}/${APP_URLS[type]}/#/currentAnalyticalObject`, - '_blank' - ) + window.location.href = `${baseUrl}/${APP_URLS[type]}/#/currentAnalyticalObject` } : undefined } diff --git a/src/components/orgunits/OrgUnitData.js b/src/components/orgunits/OrgUnitData.js index 4108d34d1..bce650e02 100644 --- a/src/components/orgunits/OrgUnitData.js +++ b/src/components/orgunits/OrgUnitData.js @@ -23,7 +23,7 @@ const ORGUNIT_PROFILE_QUERY = { // Only YEARLY period type is supported in first version const periodType = 'YEARLY' const currentYear = String(new Date().getFullYear()) -const periods = getFixedPeriodsByType({ periodType, currentYear }) +const periods = getFixedPeriodsByType(periodType, currentYear) const defaultPeriod = filterFuturePeriods(periods)[0] || periods[0] /* diff --git a/src/components/periods/PeriodSelect.js b/src/components/periods/PeriodSelect.js index f8261ac88..f020e3ce4 100644 --- a/src/components/periods/PeriodSelect.js +++ b/src/components/periods/PeriodSelect.js @@ -7,8 +7,7 @@ import { } from '@dhis2/ui' import cx from 'classnames' import PropTypes from 'prop-types' -import React, { useState, useMemo, useCallback, useEffect } from 'react' -import usePrevious from '../../hooks/usePrevious.js' +import React, { Component } from 'react' import { getFixedPeriodsByType, filterFuturePeriods, @@ -17,118 +16,124 @@ import { getYear } from '../../util/time.js' import { SelectField } from '../core/index.js' import styles from './styles/PeriodSelect.module.css' -const PeriodSelect = ({ - onChange, - className, - errorText, - firstDate, - lastDate, - period, - periodType, -}) => { - const [year, setYear] = useState(getYear(period?.startDate || lastDate)) - const prevYear = usePrevious(year) - - // Set periods when periodType or year changes - /* eslint-disable react-hooks/exhaustive-deps */ - const periods = useMemo( - () => - periodType - ? getFixedPeriodsByType({ - periodType, - year, - firstDate, - lastDate, - }) - : [period], // saved map period (not included in depency array by design) - [periodType, year, firstDate, lastDate] - ) - /* eslint-enable react-hooks/exhaustive-deps */ - - // Increment/decrement year - const changeYear = useCallback( - (change) => { - const newYear = year + change - - if ( - (!firstDate || newYear >= getYear(firstDate)) && - (!lastDate || newYear <= getYear(lastDate)) - ) { - setYear(newYear) - } - }, - [year, firstDate, lastDate] - ) - - // Autoselect most recent period - useEffect(() => { - if (!period) { - onChange(filterFuturePeriods(periods)[0] || periods[0]) +class PeriodSelect extends Component { + static propTypes = { + onChange: PropTypes.func.isRequired, + className: PropTypes.string, + errorText: PropTypes.string, + period: PropTypes.shape({ + id: PropTypes.string.isRequired, + startDate: PropTypes.string, + }), + periodType: PropTypes.string, + } + + state = { + year: null, + periods: null, + } + + componentDidMount() { + this.setPeriods() + } + + componentDidUpdate(prevProps, prevState) { + const { periodType, period, onChange } = this.props + const { year, periods } = this.state + + if (periodType !== prevProps.periodType) { + this.setPeriods() + } else if (periods && !period) { + onChange(filterFuturePeriods(periods)[0] || periods[0]) // Autoselect most recent period + } + + // Change period if year is changed (but keep period index) + if (period && prevState.periods && year !== prevState.year) { + const periodIndex = prevState.periods.findIndex( + (item) => item.id === period.id + ) + onChange(periods[periodIndex]) + } + } + + render() { + const { periodType, period, onChange, className, errorText } = + this.props + const { periods } = this.state + + if (!periods) { + return null } - }, [period, periods, year, onChange]) - // Keep the same period position when year changes - useEffect(() => { - if (period && !periods.some((p) => p.id === period.id)) { - const periodId = period.id.replace(prevYear, year) + const value = + period && periods.some((p) => p.id === period.id) ? period.id : null + + return ( +
+ + {periodType && ( +
+ +
+ )} +
+ ) + } + + setPeriods() { + const { periodType, period } = this.props + const year = this.state.year || getYear(period && period.startDate) + let periods - onChange(periods.find((p) => p.id === periodId)) + if (periodType) { + periods = getFixedPeriodsByType(periodType, year) + } else if (period) { + periods = [period] // If period is loaded in favorite } - }, [period, periods, year, prevYear, onChange]) - if (!periods) { - return null + this.setState({ periods, year }) } - const value = - period && periods.some((p) => p.id === period.id) ? period.id : null - - return ( -
- - {periodType && ( -
- -
- )} -
- ) -} + nextYear = () => { + this.changeYear(1) + } + + previousYear = () => { + this.changeYear(-1) + } -PeriodSelect.propTypes = { - onChange: PropTypes.func.isRequired, - className: PropTypes.string, - errorText: PropTypes.string, - firstDate: PropTypes.string, - lastDate: PropTypes.string, - period: PropTypes.shape({ - id: PropTypes.string.isRequired, - startDate: PropTypes.string, - }), - periodType: PropTypes.string, + changeYear = (change) => { + const { periodType } = this.props + const year = this.state.year + change + + this.setState({ + year, + periods: getFixedPeriodsByType(periodType, year), + }) + } } export default PeriodSelect diff --git a/src/constants/dimension.js b/src/constants/dimension.js index 3834de2b0..16b6d48ae 100644 --- a/src/constants/dimension.js +++ b/src/constants/dimension.js @@ -64,12 +64,6 @@ export const dimConf = { objectName: 'pi', itemType: 'PROGRAM_INDICATOR', }, - calculation: { - value: 'expressionDimensionItems', - dimensionName: 'dx', - objectName: 'ed', // Created by Bjorn, don't seem to be in use when the map is saved - itemType: 'EXPRESSION_DIMENSION_ITEM', - }, period: { id: 'period', value: 'period', diff --git a/src/util/periods.js b/src/util/periods.js index 777ab1bd6..366633e8a 100644 --- a/src/util/periods.js +++ b/src/util/periods.js @@ -6,41 +6,23 @@ import { periodTypes, periodGroups } from '../constants/periods.js' const getYearOffsetFromNow = (year) => year - new Date(Date.now()).getFullYear() -const filterPeriods = (periods, firstDate, lastDate) => - periods.filter( - (p) => - (!firstDate || p.startDate >= firstDate) && - (!lastDate || p.endDate <= lastDate) - ) - export const getPeriodTypes = (hiddenPeriods = []) => periodTypes().filter(({ group }) => !hiddenPeriods.includes(group)) -export const getFixedPeriodsByType = ({ - periodType, - year, - firstDate, - lastDate, -}) => { +export const getFixedPeriodsByType = (periodType, year) => { const period = getFixedPeriodsOptionsById(periodType) const forceDescendingForYearTypes = !!periodType.match(/^FY|YEARLY/) const offset = getYearOffsetFromNow(year) - let periods = period?.getPeriods({ offset, reversePeriods: true }) - - if (!periods) { - return null + const periods = period?.getPeriods({ offset, reversePeriods: true }) || null + if (periods && forceDescendingForYearTypes) { + // TODO: the reverse() is a workaround for a bug in the analytics + // getPeriods function that no longer correctly reverses the order + // for YEARLY and FY period types + return periods.reverse() } - - if (firstDate || lastDate) { - periods = filterPeriods(periods, firstDate, lastDate) - } - - // TODO: the reverse() is a workaround for a bug in the analytics - // getPeriods function that no longer correctly reverses the order - // for YEARLY and FY period types - return forceDescendingForYearTypes ? periods.reverse() : periods + return periods } export const getRelativePeriods = (hiddenPeriods = []) => @@ -64,7 +46,7 @@ export const getPeriodNames = () => ({ }, {}), }) -export const filterFuturePeriods = (periods, date) => { - const now = new Date(date || Date.now()) +export const filterFuturePeriods = (periods) => { + const now = new Date(Date.now()) return periods.filter(({ startDate }) => new Date(startDate) < now) }