From d38fbd9b6915692d435355dac629ad660783912c Mon Sep 17 00:00:00 2001 From: Mathieu Suen Date: Fri, 26 Jan 2024 11:59:54 +0100 Subject: [PATCH] SONAR-21178 Improve quality gates page accessiblity --- .../components/QualityGateCondition.tsx | 7 +- ...nditionModal.tsx => AddConditionModal.tsx} | 93 +++++-------- .../quality-gates/components/Condition.tsx | 16 +-- .../components/ConditionOperator.tsx | 12 +- .../quality-gates/components/Conditions.tsx | 9 +- .../components/EditConditionModal.tsx | 131 ++++++++++++++++++ .../components/__tests__/QualityGate-it.tsx | 57 ++++---- .../src/main/js/helpers/qualityGates.ts | 10 +- 8 files changed, 215 insertions(+), 120 deletions(-) rename server/sonar-web/src/main/js/apps/quality-gates/components/{ConditionModal.tsx => AddConditionModal.tsx} (67%) create mode 100644 server/sonar-web/src/main/js/apps/quality-gates/components/EditConditionModal.tsx diff --git a/server/sonar-web/src/main/js/apps/overview/components/QualityGateCondition.tsx b/server/sonar-web/src/main/js/apps/overview/components/QualityGateCondition.tsx index 765b6cd79da..dda75f63084 100644 --- a/server/sonar-web/src/main/js/apps/overview/components/QualityGateCondition.tsx +++ b/server/sonar-web/src/main/js/apps/overview/components/QualityGateCondition.tsx @@ -30,6 +30,7 @@ import { import { getBranchLikeQuery } from '../../../helpers/branch-like'; import { translate } from '../../../helpers/l10n'; import { formatMeasure, isDiffMetric, localizeMetric } from '../../../helpers/measures'; +import { getOperatorLabel } from '../../../helpers/qualityGates'; import { getComponentDrilldownUrl, getComponentIssuesUrl, @@ -155,11 +156,7 @@ export default class QualityGateCondition extends React.PureComponent { const threshold = (condition.level === 'ERROR' ? condition.error : condition.warning) as string; const actual = (condition.period ? measure.period?.value : measure.value) as string; - let operator = translate('quality_gates.operator', condition.op); - - if (metric.type === MetricType.Rating) { - operator = translate('quality_gates.operator', condition.op, 'rating'); - } + const operator = getOperatorLabel(condition.op, metric); return this.wrapWithLink(
diff --git a/server/sonar-web/src/main/js/apps/quality-gates/components/ConditionModal.tsx b/server/sonar-web/src/main/js/apps/quality-gates/components/AddConditionModal.tsx similarity index 67% rename from server/sonar-web/src/main/js/apps/quality-gates/components/ConditionModal.tsx rename to server/sonar-web/src/main/js/apps/quality-gates/components/AddConditionModal.tsx index 777a2d36459..61f7ae71dfc 100644 --- a/server/sonar-web/src/main/js/apps/quality-gates/components/ConditionModal.tsx +++ b/server/sonar-web/src/main/js/apps/quality-gates/components/AddConditionModal.tsx @@ -21,10 +21,7 @@ import { ButtonPrimary, FormField, Modal, RadioButton } from 'design-system'; import * as React from 'react'; import { getLocalizedMetricName, translate } from '../../../helpers/l10n'; import { isDiffMetric } from '../../../helpers/measures'; -import { - useCreateConditionMutation, - useUpdateConditionMutation, -} from '../../../queries/quality-gates'; +import { useCreateConditionMutation } from '../../../queries/quality-gates'; import { Condition, Metric, QualityGate } from '../../../types/types'; import { getPossibleOperators } from '../utils'; import ConditionOperator from './ConditionOperator'; @@ -32,32 +29,19 @@ import MetricSelect from './MetricSelect'; import ThresholdInput from './ThresholdInput'; interface Props { - condition?: Condition; - metric?: Metric; - metrics?: Metric[]; - header: string; + metrics: Metric[]; onClose: () => void; qualityGate: QualityGate; } const ADD_CONDITION_MODAL_ID = 'add-condition-modal'; -export default function ConditionModal({ - condition, - metric, - metrics, - header, - onClose, - qualityGate, -}: Readonly) { - const [errorThreshold, setErrorThreshold] = React.useState(condition ? condition.error : ''); +export default function AddConditionModal({ metrics, onClose, qualityGate }: Readonly) { + const [errorThreshold, setErrorThreshold] = React.useState(''); const [scope, setScope] = React.useState<'new' | 'overall'>('new'); - const [selectedMetric, setSelectedMetric] = React.useState(metric); - const [selectedOperator, setSelectedOperator] = React.useState( - condition ? condition.op : undefined, - ); + const [selectedMetric, setSelectedMetric] = React.useState(); + const [selectedOperator, setSelectedOperator] = React.useState(); const { mutateAsync: createCondition } = useCreateConditionMutation(qualityGate.name); - const { mutateAsync: updateCondition } = useUpdateConditionMutation(qualityGate.name); const getSinglePossibleOperator = (metric: Metric) => { const operators = getPossibleOperators(metric); @@ -73,10 +57,7 @@ export default function ConditionModal({ op: getSinglePossibleOperator(selectedMetric) ?? selectedOperator, error: errorThreshold, }; - const submitPromise = condition - ? updateCondition({ id: condition.id, ...newCondition }) - : createCondition(newCondition); - await submitPromise; + await createCondition(newCondition); onClose(); } }; @@ -84,7 +65,7 @@ export default function ConditionModal({ const handleScopeChange = (scope: 'new' | 'overall') => { let correspondingMetric; - if (selectedMetric && metrics) { + if (selectedMetric) { const correspondingMetricKey = scope === 'new' ? `new_${selectedMetric.key}` : selectedMetric.key.replace(/^new_/, ''); correspondingMetric = metrics.find((m) => m.key === correspondingMetricKey); @@ -110,42 +91,34 @@ export default function ConditionModal({ const renderBody = () => { return ( -
- {metric === undefined && ( - -
- - - {translate('quality_gates.conditions.new_code')} - - - - - {translate('quality_gates.conditions.overall_code')} - - -
- - )} + + +
+ + + {translate('quality_gates.conditions.new_code')} + + + + + {translate('quality_gates.conditions.overall_code')} + + +
+ - {metrics && ( - - scope === 'new' ? isDiffMetric(m.key) : !isDiffMetric(m.key), - )} - onMetricChange={handleMetricChange} - /> - )} + + scope === 'new' ? isDiffMetric(m.key) : !isDiffMetric(m.key), + )} + onMetricChange={handleMetricChange} + /> {selectedMetric && ( @@ -182,7 +155,7 @@ export default function ConditionModal({ - {header} + {translate('quality_gates.add_condition')} } secondaryButtonLabel={translate('close')} diff --git a/server/sonar-web/src/main/js/apps/quality-gates/components/Condition.tsx b/server/sonar-web/src/main/js/apps/quality-gates/components/Condition.tsx index 3d388f8afbc..e15de9c0759 100644 --- a/server/sonar-web/src/main/js/apps/quality-gates/components/Condition.tsx +++ b/server/sonar-web/src/main/js/apps/quality-gates/components/Condition.tsx @@ -33,12 +33,12 @@ import { import * as React from 'react'; import { useMetrics } from '../../../app/components/metrics/withMetricsContext'; import { getLocalizedMetricName, translate, translateWithParameters } from '../../../helpers/l10n'; +import { getOperatorLabel } from '../../../helpers/qualityGates'; import { useDeleteConditionMutation } from '../../../queries/quality-gates'; -import { MetricType } from '../../../types/metrics'; import { CaycStatus, Condition as ConditionType, Metric, QualityGate } from '../../../types/types'; import { getLocalizedMetricNameNoDiffMetric, isConditionWithFixedValue } from '../utils'; -import ConditionModal from './ConditionModal'; import ConditionValue from './ConditionValue'; +import EditConditionModal from './EditConditionModal'; export enum ConditionChange { Added = 'added', @@ -67,6 +67,7 @@ export default function ConditionComponent({ const [modal, setModal] = React.useState(false); const { mutateAsync: deleteCondition } = useDeleteConditionMutation(qualityGate.name); const metrics = useMetrics(); + const { op = 'GT' } = condition; const handleOpenUpdate = () => { setModal(true); @@ -84,13 +85,6 @@ export default function ConditionComponent({ setDeleteFormOpen(false); }; - const renderOperator = () => { - const { op = 'GT' } = condition; - return metric.type === MetricType.Rating - ? translate('quality_gates.operator', op, 'rating') - : translate('quality_gates.operator', op); - }; - const isCaycCompliantAndOverCompliant = qualityGate.caycStatus !== CaycStatus.NonCompliant; return ( @@ -100,7 +94,7 @@ export default function ConditionComponent({ {metric.hidden && } - {renderOperator()} + {getOperatorLabel(op, metric)} {modal && ( - { this.props.onOperatorChange(value); }; - getLabel(op: string, metric: Metric) { - return metric.type === 'RATING' - ? translate('quality_gates.operator', op, 'rating') - : translate('quality_gates.operator', op); - } - render() { const operators = getPossibleOperators(this.props.metric); if (Array.isArray(operators)) { const operatorOptions = operators.map((op) => { - const label = this.getLabel(op, this.props.metric); + const label = getOperatorLabel(op, this.props.metric); return { label, value: op }; }); @@ -64,6 +58,6 @@ export default class ConditionOperator extends React.PureComponent { ); } - return {this.getLabel(operators, this.props.metric)}; + return {getOperatorLabel(operators, this.props.metric)}; } } diff --git a/server/sonar-web/src/main/js/apps/quality-gates/components/Conditions.tsx b/server/sonar-web/src/main/js/apps/quality-gates/components/Conditions.tsx index 9267a9eff2c..aeba605928c 100644 --- a/server/sonar-web/src/main/js/apps/quality-gates/components/Conditions.tsx +++ b/server/sonar-web/src/main/js/apps/quality-gates/components/Conditions.tsx @@ -42,11 +42,11 @@ import { Feature } from '../../../types/features'; import { MetricKey } from '../../../types/metrics'; import { CaycStatus, Condition as ConditionType, QualityGate } from '../../../types/types'; import { groupAndSortByPriorityConditions, isQualityGateOptimized } from '../utils'; +import AddConditionModal from './AddConditionModal'; import CaYCConditionsSimplificationGuide from './CaYCConditionsSimplificationGuide'; import CaycCompliantBanner from './CaycCompliantBanner'; import CaycCondition from './CaycCondition'; import CaycFixOptimizeBanner from './CaycFixOptimizeBanner'; -import ConditionModal from './ConditionModal'; import CaycReviewUpdateConditionsModal from './ConditionReviewAndUpdateModal'; import ConditionsTable from './ConditionsTable'; @@ -110,12 +110,7 @@ export default function Conditions({ qualityGate, isFetching }: Readonly) (metric, condition) => metric.key === condition.metric, ); return ( - + ); }, [metrics, qualityGate], diff --git a/server/sonar-web/src/main/js/apps/quality-gates/components/EditConditionModal.tsx b/server/sonar-web/src/main/js/apps/quality-gates/components/EditConditionModal.tsx new file mode 100644 index 00000000000..24faa55b050 --- /dev/null +++ b/server/sonar-web/src/main/js/apps/quality-gates/components/EditConditionModal.tsx @@ -0,0 +1,131 @@ +/* + * SonarQube + * Copyright (C) 2009-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +import { ButtonPrimary, FormField, Highlight, Modal, Note } from 'design-system'; +import { isArray } from 'lodash'; +import * as React from 'react'; +import { getLocalizedMetricName, translate } from '../../../helpers/l10n'; +import { useUpdateConditionMutation } from '../../../queries/quality-gates'; +import { Condition, Metric, QualityGate } from '../../../types/types'; +import { getPossibleOperators } from '../utils'; +import ConditionOperator from './ConditionOperator'; +import ThresholdInput from './ThresholdInput'; + +interface Props { + condition: Condition; + metric: Metric; + header: string; + onClose: () => void; + qualityGate: QualityGate; +} + +const EDIT_CONDITION_MODAL_ID = 'edit-condition-modal'; + +export default function EditConditionModal({ + condition, + metric, + onClose, + qualityGate, +}: Readonly) { + const [errorThreshold, setErrorThreshold] = React.useState(condition ? condition.error : ''); + + const [selectedOperator, setSelectedOperator] = React.useState( + condition ? condition.op : undefined, + ); + const { mutateAsync: updateCondition } = useUpdateConditionMutation(qualityGate.name); + + const getSinglePossibleOperator = (metric: Metric) => { + const operators = getPossibleOperators(metric); + return isArray(operators) ? selectedOperator : operators; + }; + + const handleFormSubmit = async (event: React.FormEvent) => { + event.preventDefault(); + + const newCondition: Omit = { + metric: metric.key, + op: getSinglePossibleOperator(metric), + error: errorThreshold, + }; + await updateCondition({ id: condition.id, ...newCondition }); + onClose(); + }; + + const handleErrorChange = (error: string) => { + setErrorThreshold(error); + }; + + const handleOperatorChange = (op: string) => { + setSelectedOperator(op); + }; + + const renderBody = () => { + return ( + + + +
+ + + + + + +
+ + ); + }; + + return ( + + {translate('quality_gates.update_condition')} + + } + secondaryButtonLabel={translate('close')} + /> + ); +} diff --git a/server/sonar-web/src/main/js/apps/quality-gates/components/__tests__/QualityGate-it.tsx b/server/sonar-web/src/main/js/apps/quality-gates/components/__tests__/QualityGate-it.tsx index eb9a67bd80c..f12283529bc 100644 --- a/server/sonar-web/src/main/js/apps/quality-gates/components/__tests__/QualityGate-it.tsx +++ b/server/sonar-web/src/main/js/apps/quality-gates/components/__tests__/QualityGate-it.tsx @@ -26,7 +26,7 @@ import { searchProjects, searchUsers } from '../../../../api/quality-gates'; import { dismissNotice } from '../../../../api/users'; import { mockLoggedInUser } from '../../../../helpers/testMocks'; import { RenderContext, renderAppRoutes } from '../../../../helpers/testReactTestingUtils'; -import { byRole } from '../../../../helpers/testSelector'; +import { byRole, byTestId } from '../../../../helpers/testSelector'; import { Feature } from '../../../../types/features'; import { CaycStatus } from '../../../../types/types'; import { NoticeType } from '../../../../types/users'; @@ -218,49 +218,52 @@ it('should be able to add a condition', async () => { // On new code await user.click(await screen.findByText('quality_gates.add_condition')); - let dialog = within(screen.getByRole('dialog')); + const dialog = byRole('dialog'); - await user.click(dialog.getByRole('radio', { name: 'quality_gates.conditions.new_code' })); - await selectEvent.select(dialog.getByRole('combobox'), ['Issues']); - await user.click(dialog.getByRole('textbox', { name: 'quality_gates.conditions.value' })); + await user.click(dialog.byRole('radio', { name: 'quality_gates.conditions.new_code' }).get()); + + await selectEvent.select(dialog.byRole('combobox').get(), 'Issues'); + await user.click( + await dialog.byRole('textbox', { name: 'quality_gates.conditions.value' }).find(), + ); await user.keyboard('12'); - await user.click(dialog.getByRole('button', { name: 'quality_gates.add_condition' })); - const newConditions = within(await screen.findByTestId('quality-gates__conditions-new')); - expect(await newConditions.findByRole('cell', { name: 'Issues' })).toBeInTheDocument(); - expect(await newConditions.findByRole('cell', { name: '12' })).toBeInTheDocument(); + await user.click(dialog.byRole('button', { name: 'quality_gates.add_condition' }).get()); + const newConditions = byTestId('quality-gates__conditions-new'); + expect(await newConditions.byRole('cell', { name: 'Issues' }).find()).toBeInTheDocument(); + expect(await newConditions.byRole('cell', { name: '12' }).find()).toBeInTheDocument(); // On overall code await user.click(await screen.findByText('quality_gates.add_condition')); - dialog = within(screen.getByRole('dialog')); - await selectEvent.select(dialog.getByRole('combobox'), ['Info Issues']); - await user.click(dialog.getByRole('radio', { name: 'quality_gates.conditions.overall_code' })); - await user.click(dialog.getByLabelText('quality_gates.conditions.operator')); + await selectEvent.select(dialog.byRole('combobox').get(), ['Info Issues']); + await user.click(dialog.byRole('radio', { name: 'quality_gates.conditions.overall_code' }).get()); + await user.click(dialog.byLabelText('quality_gates.conditions.operator').get()); - await user.click(dialog.getByText('quality_gates.operator.LT')); - await user.click(dialog.getByRole('textbox', { name: 'quality_gates.conditions.value' })); + await user.click(dialog.byText('quality_gates.operator.LT').get()); + await user.click(dialog.byRole('textbox', { name: 'quality_gates.conditions.value' }).get()); await user.keyboard('42'); - await user.click(dialog.getByRole('button', { name: 'quality_gates.add_condition' })); + await user.click(dialog.byRole('button', { name: 'quality_gates.add_condition' }).get()); - const overallConditions = within(await screen.findByTestId('quality-gates__conditions-overall')); + const overallConditions = byTestId('quality-gates__conditions-overall'); - expect(await overallConditions.findByRole('cell', { name: 'Info Issues' })).toBeInTheDocument(); - expect(await overallConditions.findByRole('cell', { name: '42' })).toBeInTheDocument(); + expect( + await overallConditions.byRole('cell', { name: 'Info Issues' }).find(), + ).toBeInTheDocument(); + expect(await overallConditions.byRole('cell', { name: '42' }).find()).toBeInTheDocument(); // Select a rating await user.click(await screen.findByText('quality_gates.add_condition')); - dialog = within(screen.getByRole('dialog')); - await user.click(dialog.getByRole('radio', { name: 'quality_gates.conditions.overall_code' })); - await selectEvent.select(dialog.getByRole('combobox'), ['Maintainability Rating']); - await user.click(dialog.getByLabelText('quality_gates.conditions.value')); - await user.click(dialog.getByText('B')); - await user.click(dialog.getByRole('button', { name: 'quality_gates.add_condition' })); + await user.click(dialog.byRole('radio', { name: 'quality_gates.conditions.overall_code' }).get()); + await selectEvent.select(dialog.byRole('combobox').get(), ['Maintainability Rating']); + await user.click(dialog.byLabelText('quality_gates.conditions.value').get()); + await user.click(dialog.byText('B').get()); + await user.click(dialog.byRole('button', { name: 'quality_gates.add_condition' }).get()); expect( - await overallConditions.findByRole('cell', { name: 'Maintainability Rating' }), + await overallConditions.byRole('cell', { name: 'Maintainability Rating' }).find(), ).toBeInTheDocument(); - expect(await overallConditions.findByRole('cell', { name: 'B' })).toBeInTheDocument(); + expect(await overallConditions.byRole('cell', { name: 'B' }).find()).toBeInTheDocument(); }); it('should be able to edit a condition', async () => { diff --git a/server/sonar-web/src/main/js/helpers/qualityGates.ts b/server/sonar-web/src/main/js/helpers/qualityGates.ts index abba1d81c7a..19c8e161ed7 100644 --- a/server/sonar-web/src/main/js/helpers/qualityGates.ts +++ b/server/sonar-web/src/main/js/helpers/qualityGates.ts @@ -17,12 +17,20 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -import { MetricKey } from '../types/metrics'; +import { MetricKey, MetricType } from '../types/metrics'; import { QualityGateApplicationStatusChildProject, QualityGateProjectStatus, QualityGateStatusCondition, } from '../types/quality-gates'; +import { Metric } from '../types/types'; +import { translate } from './l10n'; + +export function getOperatorLabel(op: string, metric: Metric) { + return metric.type === MetricType.Rating + ? translate('quality_gates.operator', op, 'rating') + : translate('quality_gates.operator', op); +} export function extractStatusConditionsFromProjectStatus( projectStatus: QualityGateProjectStatus, -- 2.39.5