From d4d634ebedd7c714cb2beedaf9c25bbf6e3c901e Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 5 Oct 2023 10:54:43 +0200 Subject: [PATCH] fix: limit max value shifts to min value fields in PT (DHIS2-10235) (#2756) --- .github/pull_request_template.md | 3 + cypress/elements/dimensionModal/index.js | 1 + cypress/elements/optionsModal/index.js | 12 + cypress/elements/optionsModal/limitValues.js | 34 +++ cypress/integration/options/limitValues.cy.js | 260 ++++++++++++++++++ .../Options/MeasureCriteria.js | 89 ++++-- 6 files changed, 380 insertions(+), 19 deletions(-) create mode 100644 cypress/elements/optionsModal/limitValues.js create mode 100644 cypress/integration/options/limitValues.cy.js diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 80ce564f3d..57c9e2ca56 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -18,6 +18,9 @@ _text_ ### TODO +- [ ] Cypress tests +- [ ] Update docs +- [ ] Manual testing - [ ] _task_ --- diff --git a/cypress/elements/dimensionModal/index.js b/cypress/elements/dimensionModal/index.js index a7ea0f168b..9fbc71367f 100644 --- a/cypress/elements/dimensionModal/index.js +++ b/cypress/elements/dimensionModal/index.js @@ -76,6 +76,7 @@ export const expectSourceToNotBeLoading = () => .findBySelLike(transferLoadingEl) .should('not.exist') +// FIXME: unused, remove? export const expectSourceToBeLoading = () => cy .getBySelLike(transferLeftContainerEl) diff --git a/cypress/elements/optionsModal/index.js b/cypress/elements/optionsModal/index.js index 8405186eb1..1f8f7f6bdd 100644 --- a/cypress/elements/optionsModal/index.js +++ b/cypress/elements/optionsModal/index.js @@ -9,6 +9,7 @@ export const OPTIONS_TAB_AXES = 'Axes' export const OPTIONS_TAB_OUTLIERS = 'Outliers' export const OPTIONS_TAB_SERIES = 'Series' export const OPTIONS_TAB_LEGEND = 'Legend' +export const OPTIONS_TAB_LIMIT_VALUES = 'Limit values' export const clickOptionsTab = (name) => cy.getBySel(tabBarEl).contains(name).click() @@ -89,3 +90,14 @@ export { expectLegendKeyToBeVisible, expectLegedKeyItemAmountToBe, } from './legend.js' + +export { + setMinValue, + setMaxValue, + changeMinOperator, + changeMaxOperator, + expectMinValueToBeValue, + expectMaxValueToBeValue, + expectMinOperatorToBeOption, + expectMaxOperatorToBeOption, +} from './limitValues.js' diff --git a/cypress/elements/optionsModal/limitValues.js b/cypress/elements/optionsModal/limitValues.js new file mode 100644 index 0000000000..26ab4a33c4 --- /dev/null +++ b/cypress/elements/optionsModal/limitValues.js @@ -0,0 +1,34 @@ +import { typeInput } from '../common.js' + +const minValueInput = 'measure-critiera-min-value' +const maxValueInput = 'measure-critiera-max-value' +const minOperatorSelect = 'measure-critiera-min-operator' +const minOperatorSelectOption = 'measure-critiera-min-operator-option' +const maxOperatorSelect = 'measure-critiera-max-operator' +const maxOperatorSelectOption = 'measure-critiera-max-operator-option' + +export const setMinValue = (text) => typeInput(minValueInput, text) + +export const setMaxValue = (text) => typeInput(maxValueInput, text) + +export const changeMinOperator = (optionName) => { + cy.getBySel(minOperatorSelect).click() + cy.getBySelLike(minOperatorSelectOption).contains(optionName).click() +} + +export const changeMaxOperator = (optionName) => { + cy.getBySel(maxOperatorSelect).click() + cy.getBySelLike(maxOperatorSelectOption).contains(optionName).click() +} + +export const expectMinValueToBeValue = (value) => + cy.getBySel(minValueInput).find('input').should('have.value', value) + +export const expectMaxValueToBeValue = (value) => + cy.getBySel(maxValueInput).find('input').should('have.value', value) + +export const expectMinOperatorToBeOption = (optionName) => + cy.getBySel(minOperatorSelect).containsExact(optionName) + +export const expectMaxOperatorToBeOption = (optionName) => + cy.getBySel(maxOperatorSelect).containsExact(optionName) diff --git a/cypress/integration/options/limitValues.cy.js b/cypress/integration/options/limitValues.cy.js new file mode 100644 index 0000000000..33dce7fe02 --- /dev/null +++ b/cypress/integration/options/limitValues.cy.js @@ -0,0 +1,260 @@ +import { + DIMENSION_ID_DATA, + DIMENSION_ID_PERIOD, + VIS_TYPE_PIVOT_TABLE, + visTypeDisplayNames, +} from '@dhis2/analytics' +import { expectVisualizationToBeVisible } from '../../elements/chart.js' +import { expectAppToNotBeLoading } from '../../elements/common.js' +import { + selectIndicators, + clickDimensionModalUpdateButton, + unselectAllItemsByButton, + selectFixedPeriods, +} from '../../elements/dimensionModal/index.js' +import { openDimension } from '../../elements/dimensionsPanel.js' +import { + createNewAO, + deleteAO, + saveNewAO, +} from '../../elements/fileMenu/index.js' +import { openOptionsModal } from '../../elements/menuBar.js' +import { + OPTIONS_TAB_LIMIT_VALUES, + changeMaxOperator, + changeMinOperator, + clickOptionsModalHideButton, + clickOptionsModalUpdateButton, + expectMaxOperatorToBeOption, + expectMaxValueToBeValue, + expectMinOperatorToBeOption, + expectMinValueToBeValue, + setMaxValue, + setMinValue, +} from '../../elements/optionsModal/index.js' +import { + expectStartScreenToBeVisible, + goToStartPage, +} from '../../elements/startScreen.js' +import { changeVisType } from '../../elements/visualizationTypeSelector.js' + +const TEST_INDICATOR = 'ANC visits total' +const currentYear = new Date().getFullYear().toString() +const expectTableValueToBe = (value, position) => + cy + .getBySel('visualization-container') + .find('tbody') + .find('tr') + .eq(position) + .find('td') + .invoke('text') + .invoke('trim') + .should('equal', value) + +describe('limit values', () => { + beforeEach(() => { + goToStartPage() + createNewAO() + changeVisType(visTypeDisplayNames[VIS_TYPE_PIVOT_TABLE]) + openDimension(DIMENSION_ID_DATA) + selectIndicators([TEST_INDICATOR]) + clickDimensionModalUpdateButton() + openDimension(DIMENSION_ID_PERIOD) + unselectAllItemsByButton() + selectFixedPeriods( + [ + `January ${currentYear}`, + `February ${currentYear}`, + `March ${currentYear}`, + `April ${currentYear}`, + `May ${currentYear}`, + ], + 'Monthly' + ) + clickDimensionModalUpdateButton() + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + + const expectedValues = [ + '49 231', + '49 605', + '49 500', + '55 385', + '68 886', + ] + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('>') + expectMaxOperatorToBeOption('<') + expectMinValueToBeValue('') + expectMaxValueToBeValue('') + }) + it('min and max value display correctly', () => { + // set limits + changeMinOperator('>=') + changeMaxOperator('<=') + setMinValue('49500') + setMaxValue('55385') + clickOptionsModalUpdateButton() + + // verify limits are applied + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + const expectedValues = ['', '49 605', '49 500', '55 385', ''] + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('>=') + expectMaxOperatorToBeOption('<=') + expectMinValueToBeValue('49500') + expectMaxValueToBeValue('55385') + clickOptionsModalHideButton() + + // save AO, verify limits are applied + saveNewAO(`TEST min max ${new Date().toLocaleString()}`) + expectAppToNotBeLoading() + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('>=') + expectMaxOperatorToBeOption('<=') + expectMinValueToBeValue('49500') + expectMaxValueToBeValue('55385') + clickOptionsModalHideButton() + + // clean up + deleteAO() + expectStartScreenToBeVisible() + }) + it('min value only display correctly', () => { + // set limits + changeMinOperator('>=') + setMinValue('49500') + clickOptionsModalUpdateButton() + + // verify limits are applied + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + const expectedValues = ['', '49 605', '49 500', '55 385', '68 886'] + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('>=') + expectMaxOperatorToBeOption('<') + expectMinValueToBeValue('49500') + expectMaxValueToBeValue('') + clickOptionsModalHideButton() + + // save AO, verify limits are applied + saveNewAO(`TEST min max ${new Date().toLocaleString()}`) + expectAppToNotBeLoading() + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('>=') + expectMaxOperatorToBeOption('<') + expectMinValueToBeValue('49500') + expectMaxValueToBeValue('') + clickOptionsModalHideButton() + + // clean up + deleteAO() + expectStartScreenToBeVisible() + }) + it('max value only display correctly', () => { + // set limits + changeMaxOperator('<=') + setMaxValue('55385') + clickOptionsModalUpdateButton() + + // verify limits are applied + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + const expectedValues = ['49 231', '49 605', '49 500', '55 385', ''] + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('>') + expectMaxOperatorToBeOption('<=') + expectMinValueToBeValue('') + expectMaxValueToBeValue('55385') + clickOptionsModalHideButton() + + // save AO, verify limits are applied + saveNewAO(`TEST min max ${new Date().toLocaleString()}`) + expectAppToNotBeLoading() + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('>') + expectMaxOperatorToBeOption('<=') + expectMinValueToBeValue('') + expectMaxValueToBeValue('55385') + clickOptionsModalHideButton() + + // clean up + deleteAO() + expectStartScreenToBeVisible() + }) + it('equal value display correctly', () => { + // set limits + changeMinOperator('=') + setMinValue('49500') + clickOptionsModalUpdateButton() + + // verify limits are applied + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + const expectedValues = ['', '', '49 500', '', ''] + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('=') + expectMaxOperatorToBeOption('<') + expectMinValueToBeValue('49500') + expectMaxValueToBeValue('') + clickOptionsModalHideButton() + + // save AO, verify limits are applied + saveNewAO(`TEST min max ${new Date().toLocaleString()}`) + expectAppToNotBeLoading() + expectVisualizationToBeVisible(VIS_TYPE_PIVOT_TABLE) + expectedValues.forEach((value, index) => + expectTableValueToBe(value, index) + ) + + // verify options are present when reopening modal + openOptionsModal(OPTIONS_TAB_LIMIT_VALUES) + expectMinOperatorToBeOption('=') + expectMaxOperatorToBeOption('<') + expectMinValueToBeValue('49500') + expectMaxValueToBeValue('') + clickOptionsModalHideButton() + + // clean up + deleteAO() + expectStartScreenToBeVisible() + }) +}) diff --git a/src/components/VisualizationOptions/Options/MeasureCriteria.js b/src/components/VisualizationOptions/Options/MeasureCriteria.js index 541ecef30c..f983007b2d 100644 --- a/src/components/VisualizationOptions/Options/MeasureCriteria.js +++ b/src/components/VisualizationOptions/Options/MeasureCriteria.js @@ -17,15 +17,26 @@ import { tabSectionOptionComplexInline, } from '../styles/VisualizationOptions.style.js' -const OperatorSelect = ({ name, value, onChange }) => { - const options = [ - { id: 'EQ', label: '=' }, - { id: 'GT', label: '>' }, - { id: 'GE', label: '>=' }, - { id: 'LT', label: '<' }, - { id: 'LE', label: '<=' }, - ] - +const EQUAL_OPERATOR_ID = 'EQ' +const GREATER_THAN_OPERATOR_ID = 'GT' +const GREATER_THAN_OR_EQUAL_OPERATOR_ID = 'GE' +const LESS_THAN_OPERATOR_ID = 'LT' +const LESS_THAN_OR_EQUAL_OPERATOR_ID = 'LE' +const EMPTY = '' + +const MIN_OPERATORS = [ + { id: EQUAL_OPERATOR_ID, label: '=' }, + { id: GREATER_THAN_OPERATOR_ID, label: '>' }, + { id: GREATER_THAN_OR_EQUAL_OPERATOR_ID, label: '>=' }, +] + +const MAX_OPERATORS = [ + { id: EQUAL_OPERATOR_ID, label: '=' }, + { id: LESS_THAN_OPERATOR_ID, label: '<' }, + { id: LESS_THAN_OR_EQUAL_OPERATOR_ID, label: '<=' }, +] + +const OperatorSelect = ({ name, value, onChange, operators, dataTest }) => { return (
{ tabIndex="0" inputMaxWidth="106px" dense + dataTest={dataTest} > - {options.map(({ id, label }) => ( - + {operators.map(({ id, label }) => ( + ))}
@@ -48,11 +65,13 @@ const OperatorSelect = ({ name, value, onChange }) => { OperatorSelect.propTypes = { name: PropTypes.string.isRequired, + operators: PropTypes.array.isRequired, value: PropTypes.string.isRequired, onChange: PropTypes.func.isRequired, + dataTest: PropTypes.string, } -const ValueInput = ({ name, value, onChange }) => ( +const ValueInput = ({ name, value, onChange, dataTest }) => ( ( onChange={({ value }) => onChange(value)} width="72px" dense + dataTest={dataTest} /> ) @@ -67,20 +87,41 @@ ValueInput.propTypes = { name: PropTypes.string.isRequired, value: PropTypes.string.isRequired, onChange: PropTypes.func.isRequired, + dataTest: PropTypes.string, } class MeasureCriteria extends Component { constructor(props) { super(props) - this.defaultState = { op1: '', v1: '', op2: '', v2: '' } - - const [op1 = '', v1 = '', op2 = '', v2 = ''] = props.value.split(/[;:]/) + const OP1_DEFAULT = GREATER_THAN_OPERATOR_ID + const OP2_DEFAULT = LESS_THAN_OPERATOR_ID + + this.defaultState = { + op1: OP1_DEFAULT, + v1: EMPTY, + op2: OP2_DEFAULT, + v2: EMPTY, + } + + let [op1 = OP1_DEFAULT, v1 = EMPTY, op2 = OP2_DEFAULT, v2 = EMPTY] = + props.value && props.value.split(/[;:]/) + + if ( + [LESS_THAN_OPERATOR_ID, LESS_THAN_OR_EQUAL_OPERATOR_ID].includes( + op1 + ) + ) { + op2 = op1 + v2 = v1 + op1 = OP1_DEFAULT + v1 = EMPTY + } this.state = { op1, v1, op2, v2 } } - onClear = () => this.setState(this.defaultState, this.props.onChange('')) + onClear = () => this.setState(this.defaultState, this.props.onChange(EMPTY)) onChange = (name) => (value) => { this.setState({ [name]: value }, () => { @@ -95,7 +136,7 @@ class MeasureCriteria extends Component { value.push(`${op2}:${v2}`) } - this.props.onChange(value.length > 0 ? value.join(';') : '') + this.props.onChange(value.length > 0 ? value.join(';') : EMPTY) }) } @@ -119,11 +160,14 @@ class MeasureCriteria extends Component { name="op1" value={op1} onChange={this.onChange('op1')} + operators={MIN_OPERATORS} + dataTest="measure-critiera-min-operator" /> @@ -136,17 +180,24 @@ class MeasureCriteria extends Component { name="op2" value={op2} onChange={this.onChange('op2')} + operators={MAX_OPERATORS} + dataTest="measure-critiera-max-operator" />
-
@@ -161,7 +212,7 @@ MeasureCriteria.propTypes = { } const mapStateToProps = (state) => ({ - value: sGetUiOptions(state).measureCriteria || '', + value: sGetUiOptions(state).measureCriteria || EMPTY, }) const mapDispatchToProps = (dispatch) => ({