From b6e076e3c45232a4dfa5408ac52da5c6a2cbbb8c Mon Sep 17 00:00:00 2001 From: Ismail Cherri Date: Wed, 13 Nov 2024 16:50:17 +0100 Subject: [PATCH] SONAR-23582 Update custom rule dialog is MQR/Standard modes compatible --- .../js/api/mocks/CodingRulesServiceMock.ts | 6 + server/sonar-web/src/main/js/api/rules.ts | 13 +- .../coding-rules/__tests__/CustomRule-it.ts | 151 +++++++++- .../components/CustomRuleButton.tsx | 1 + .../components/CustomRuleFormFieldsCCT.tsx | 101 ++++--- .../components/CustomRuleFormModal.tsx | 267 +++++++++++++----- .../coding-rules/components/RuleDetails.tsx | 99 ++++--- .../components/RuleDetailsCustomRules.tsx | 77 +++-- .../components/SeveritySelect.tsx | 10 +- .../main/js/apps/coding-rules/utils-tests.tsx | 10 + server/sonar-web/src/main/js/types/types.ts | 9 + .../resources/org/sonar/l10n/core.properties | 8 +- 12 files changed, 535 insertions(+), 217 deletions(-) diff --git a/server/sonar-web/src/main/js/api/mocks/CodingRulesServiceMock.ts b/server/sonar-web/src/main/js/api/mocks/CodingRulesServiceMock.ts index 6d14b4948c0..9627db0956d 100644 --- a/server/sonar-web/src/main/js/api/mocks/CodingRulesServiceMock.ts +++ b/server/sonar-web/src/main/js/api/mocks/CodingRulesServiceMock.ts @@ -390,6 +390,12 @@ export default class CodingRulesServiceMock { rule.htmlNote = data.markdown_note !== undefined ? data.markdown_note : rule.htmlNote; rule.name = data.name !== undefined ? data.name : rule.name; rule.status = rule.status === RuleStatus.Removed ? RuleStatus.Ready : rule.status; + rule.cleanCodeAttribute = + data.cleanCodeAttribute !== undefined ? data.cleanCodeAttribute : rule.cleanCodeAttribute; + rule.impacts = data.impacts !== undefined ? data.impacts : rule.impacts; + rule.type = data.type !== undefined ? data.type : rule.type; + rule.severity = data.severity !== undefined ? data.severity : rule.severity; + if (template && data.params) { rule.params = []; data.params.split(';').forEach((param) => { diff --git a/server/sonar-web/src/main/js/api/rules.ts b/server/sonar-web/src/main/js/api/rules.ts index a1af5c27051..3b75ebed8a9 100644 --- a/server/sonar-web/src/main/js/api/rules.ts +++ b/server/sonar-web/src/main/js/api/rules.ts @@ -31,19 +31,22 @@ import { RuleActivation, RuleDetails, RulesUpdateRequest, + RuleType, } from '../types/types'; const RULES_ENDPOINT = '/api/v2/clean-code-policy/rules'; export interface CreateRuleData { - cleanCodeAttribute: CleanCodeAttribute; + cleanCodeAttribute?: CleanCodeAttribute; impacts: SoftwareImpact[]; key: string; markdownDescription: string; name: string; parameters?: Partial[]; + severity?: string; status?: string; templateKey: string; + type?: RuleType; } export function getRulesApp(): Promise { @@ -94,5 +97,11 @@ export function deleteRule(parameters: { key: string }) { } export function updateRule(data: RulesUpdateRequest): Promise { - return postJSON('/api/rules/update', data).then((r) => r.rule, throwGlobalError); + const impacts = + data.impacts && + Object.values(data.impacts) + .map((impact) => `${impact.softwareQuality}=${impact.severity}`) + .join(';'); + + return postJSON('/api/rules/update', { ...data, impacts }).then((r) => r.rule, throwGlobalError); } diff --git a/server/sonar-web/src/main/js/apps/coding-rules/__tests__/CustomRule-it.ts b/server/sonar-web/src/main/js/apps/coding-rules/__tests__/CustomRule-it.ts index 73cbba1cfcc..68ed8ca3990 100644 --- a/server/sonar-web/src/main/js/apps/coding-rules/__tests__/CustomRule-it.ts +++ b/server/sonar-web/src/main/js/apps/coding-rules/__tests__/CustomRule-it.ts @@ -22,7 +22,9 @@ import { byRole, byText } from '~sonar-aligned/helpers/testSelector'; import CodingRulesServiceMock from '../../../api/mocks/CodingRulesServiceMock'; import SettingsServiceMock from '../../../api/mocks/SettingsServiceMock'; import { mockLoggedInUser } from '../../../helpers/testMocks'; -import { SoftwareQuality } from '../../../types/clean-code-taxonomy'; +import { SoftwareImpactSeverity, SoftwareQuality } from '../../../types/clean-code-taxonomy'; +import { IssueSeverity, IssueType } from '../../../types/issues'; +import { SettingsKey } from '../../../types/settings'; import { getPageObjects, renderCodingRulesApp } from '../utils-tests'; const rulesHandler = new CodingRulesServiceMock(); @@ -42,7 +44,7 @@ afterEach(() => { }); describe('custom rule', () => { - it('can create custom rule', async () => { + it('can create custom rule in MQR mode', async () => { const { ui, user } = getPageObjects(); rulesHandler.setIsAdmin(); renderCodingRulesApp(mockLoggedInUser()); @@ -99,7 +101,9 @@ describe('custom rule', () => { byRole('option', { name: 'severity_impact.MEDIUM severity_impact.MEDIUM' }).get(), ); - expect(ui.createCustomRuleDialog.byText('severity_impact.MEDIUM').get()).toBeInTheDocument(); + expect( + ui.createCustomRuleDialog.byRole('combobox', { name: 'severity' }).getAll()[1], + ).toHaveValue('severity_impact.MEDIUM'); await user.click(ui.statusSelect.get()); await user.click(byRole('option', { name: 'rules.status.BETA' }).get()); @@ -113,6 +117,97 @@ describe('custom rule', () => { expect(ui.customRuleItemLink('New Custom Rule').get()).toBeInTheDocument(); }); + it('hides severities if security hotspot is selected in MQR mode', async () => { + const { ui, user } = getPageObjects(); + rulesHandler.setIsAdmin(); + renderCodingRulesApp(mockLoggedInUser(), 'coding_rules?open=rule8'); + await ui.detailsloaded(); + + // Create custom rule + await user.click(ui.createCustomRuleButton.get()); + // Switch type to Security hotspot + await user.click(ui.cctIssueTypeSelect.get()); + await user.click( + byRole('option', { name: 'coding_rules.custom.type.option.SECURITY_HOTSPOT' }).get(), + ); + expect(ui.cleanCodeCategorySelect.query()).not.toBeInTheDocument(); + + // Switch type back to Issue + await user.click(ui.cctIssueTypeSelect.get()); + await user.click(byRole('option', { name: 'coding_rules.custom.type.option.ISSUE' }).get()); + expect(ui.cleanCodeCategorySelect.get()).toBeInTheDocument(); + }); + + it('can create custom rule in Standard mode', async () => { + settingsHandler.set(SettingsKey.MQRMode, 'false'); + const { ui, user } = getPageObjects(); + rulesHandler.setIsAdmin(); + renderCodingRulesApp(mockLoggedInUser()); + await ui.facetsLoaded(); + + await user.click(await ui.templateFacet.find()); + await user.click(ui.facetItem('coding_rules.filters.template.is_template').get()); + + // Shows only one template rule + expect(ui.getAllRuleListItems()).toHaveLength(1); + + // Show template rule details + await user.click(ui.ruleListItemLink('Template rule').get()); + expect(ui.ruleTitle('Template rule').get()).toBeInTheDocument(); + expect(ui.customRuleSectionTitle.get()).toBeInTheDocument(); + + // Create custom rule + await user.click(ui.createCustomRuleButton.get()); + await user.type(ui.ruleNameTextbox.get(), 'New Custom Rule'); + expect(ui.keyTextbox.get()).toHaveValue('New_Custom_Rule'); + await user.clear(ui.keyTextbox.get()); + await user.type(ui.keyTextbox.get(), 'new_custom_rule'); + + // Select type as bug + await user.click(ui.standardIssueTypeSelect.get()); + await user.click(byRole('option', { name: 'issue.type.BUG' }).get()); + + // Select Severity as Major + await user.click(ui.standardSeveritySelect.get()); + await user.click(byRole('option', { name: 'severity.MAJOR' }).get()); + + expect(ui.createCustomRuleDialog.byRole('combobox', { name: 'severity' }).get()).toHaveValue( + 'severity.MAJOR', + ); + + await user.click(ui.statusSelect.get()); + await user.click(byRole('option', { name: 'rules.status.BETA' }).get()); + + await user.type(ui.descriptionTextbox.get(), 'Some description for custom rule'); + await user.type(ui.paramInput('1').get(), 'Default value'); + + await user.click(ui.createButton.get()); + + // Verify the rule is created + expect(ui.customRuleItemLink('New Custom Rule').get()).toBeInTheDocument(); + }); + + it('hides severities if security hotspot is selected in Standard mode', async () => { + settingsHandler.set(SettingsKey.MQRMode, 'false'); + const { ui, user } = getPageObjects(); + rulesHandler.setIsAdmin(); + renderCodingRulesApp(mockLoggedInUser(), 'coding_rules?open=rule8'); + await ui.detailsloaded(); + + // Create custom rule + await user.click(ui.createCustomRuleButton.get()); + // Switch type to Security hotspot + await user.click(ui.standardIssueTypeSelect.get()); + await user.click(byRole('option', { name: 'issue.type.SECURITY_HOTSPOT' }).get()); + + expect(ui.standardSeveritySelect.query()).not.toBeInTheDocument(); + + // Switch type back to Bug + await user.click(ui.standardIssueTypeSelect.get()); + await user.click(byRole('option', { name: 'issue.type.BUG' }).get()); + expect(ui.standardSeveritySelect.get()).toBeInTheDocument(); + }); + it('can reactivate custom rule', async () => { const { ui, user } = getPageObjects(); rulesHandler.setIsAdmin(); @@ -136,7 +231,45 @@ describe('custom rule', () => { expect(ui.customRuleItemLink('Reactivate custom Rule').get()).toBeInTheDocument(); }); - it('can edit custom rule', async () => { + it('can edit custom rule in MQR mode', async () => { + const { ui, user } = getPageObjects(); + rulesHandler.setIsAdmin(); + renderCodingRulesApp(mockLoggedInUser(), 'coding_rules?open=rule9'); + await ui.detailsloaded(); + + await user.click(ui.editCustomRuleButton.get()); + + // Change name and description of custom rule + await user.clear(ui.ruleNameTextbox.get()); + await user.type(ui.ruleNameTextbox.get(), 'Updated custom rule name'); + await user.type(ui.descriptionTextbox.get(), 'Some description for custom rule'); + + // Maintainability should not be checked and should be disabled + expect(ui.cleanCodeQualityCheckbox(SoftwareQuality.Maintainability).get()).not.toBeChecked(); + expect(ui.cleanCodeQualityCheckbox(SoftwareQuality.Maintainability).get()).toHaveAttribute( + 'aria-disabled', + 'true', + ); + expect(ui.cleanCodeQualityCheckbox(SoftwareQuality.Reliability).get()).toHaveAttribute( + 'aria-disabled', + 'true', + ); + expect(ui.cleanCodeQualityCheckbox(SoftwareQuality.Reliability).get()).toBeChecked(); + + // Set severity + await user.click(ui.cleanCodeSeveritySelect(SoftwareQuality.Reliability).get()); + await user.click(byRole('option', { name: 'severity_impact.HIGH severity_impact.HIGH' }).get()); + + await user.click(ui.saveButton.get(ui.updateCustomRuleDialog.get())); + + expect(ui.ruleTitle('Updated custom rule name').get()).toBeInTheDocument(); + expect( + ui.ruleSoftwareQualityPill(SoftwareQuality.Reliability, SoftwareImpactSeverity.High).get(), + ).toBeInTheDocument(); + }); + + it('can edit custom rule in Standard Mode', async () => { + settingsHandler.set(SettingsKey.MQRMode, 'false'); const { ui, user } = getPageObjects(); rulesHandler.setIsAdmin(); renderCodingRulesApp(mockLoggedInUser(), 'coding_rules?open=rule9'); @@ -149,9 +282,19 @@ describe('custom rule', () => { await user.type(ui.ruleNameTextbox.get(), 'Updated custom rule name'); await user.type(ui.descriptionTextbox.get(), 'Some description for custom rule'); + // Type should be Bug and should be disabled + expect(ui.standardIssueTypeSelect.get()).toHaveValue('issue.type.BUG'); + expect(ui.standardIssueTypeSelect.get()).toBeDisabled(); + + // Select Severity as INFO + await user.click(ui.standardSeveritySelect.get()); + await user.click(byRole('option', { name: 'severity.INFO' }).get()); + await user.click(ui.saveButton.get(ui.updateCustomRuleDialog.get())); expect(ui.ruleTitle('Updated custom rule name').get()).toBeInTheDocument(); + expect(ui.ruleIssueTypePill(IssueType.Bug).get()).toBeInTheDocument(); + expect(ui.ruleIssueTypePillSeverity(IssueSeverity.Info).get()).toBeInTheDocument(); }); it('can delete custom rule', async () => { diff --git a/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleButton.tsx b/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleButton.tsx index 2e6fe61a4e4..237e753414a 100644 --- a/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleButton.tsx +++ b/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleButton.tsx @@ -40,6 +40,7 @@ export default function CustomRuleButton(props: Props) { customRule={customRule} onClose={() => setModalOpen(false)} templateRule={templateRule} + isOpen={modalOpen} /> )} diff --git a/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormFieldsCCT.tsx b/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormFieldsCCT.tsx index e7e9ee0fc50..cb3040808df 100644 --- a/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormFieldsCCT.tsx +++ b/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormFieldsCCT.tsx @@ -18,17 +18,10 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -import React from 'react'; +import { Checkbox, Select, Text } from '@sonarsource/echoes-react'; +import { useEffect, useMemo, useRef } from 'react'; import { useIntl } from 'react-intl'; -import { - Checkbox, - FormField, - Highlight, - InputSelect, - LightPrimary, - RequiredIcon, - TextError, -} from '~design-system'; +import { FormField, RequiredIcon } from '~design-system'; import SoftwareImpactSeverityIcon from '../../../components/icon-mappers/SoftwareImpactSeverityIcon'; import { CLEAN_CODE_ATTRIBUTES_BY_CATEGORY, @@ -65,14 +58,16 @@ export function CleanCodeCategoryField(props: Readonly - props.onChange(option?.value as CleanCodeAttributeCategory)} - isClearable={false} + props.onChange(option as CleanCodeAttribute)} isDisabled={disabled} isSearchable={false} - value={attributes.find((attribute) => attribute.value === value)} + isNotClearable + value={attributes.find((attribute) => attribute.value === value)?.value} /> ); } export function SoftwareQualitiesFields( - props: Readonly & { error: boolean }>, + props: Readonly & { error: boolean; qualityUpdateDisabled: boolean }>, ) { - const { value, disabled, error } = props; + const { value, disabled, error, qualityUpdateDisabled } = props; const intl = useIntl(); - const severities = React.useMemo( + const severities = useMemo( () => IMPACT_SEVERITIES.map((severity) => ({ value: severity, label: intl.formatMessage({ id: `severity_impact.${severity}` }), - Icon: , + prefix: , })), [intl], ); - const handleSoftwareQualityChange = (quality: SoftwareQuality, checked: boolean) => { - if (checked) { + const handleSoftwareQualityChange = (quality: SoftwareQuality, checked: boolean | string) => { + if (checked === true) { props.onChange([ ...value, { softwareQuality: quality, severity: SoftwareImpactSeverity.Low }, @@ -162,19 +157,19 @@ export function SoftwareQualitiesFields( return (
- + {intl.formatMessage({ id: 'software_quality' })} - - + + {intl.formatMessage({ id: 'severity' })} - + {SOFTWARE_QUALITIES.map((quality) => { const selectedQuality = value.find((impact) => impact.softwareQuality === quality); const selectedSeverity = selectedQuality - ? severities.find((severity) => severity.value === selectedQuality.severity) + ? severities.find((severity) => severity.value === selectedQuality.severity)?.value : null; return ( @@ -186,38 +181,42 @@ export function SoftwareQualitiesFields( )} { handleSoftwareQualityChange(quality, checked); }} - label={quality} - > - - {intl.formatMessage({ id: `software_quality.${quality}` })} - - - + {intl.formatMessage({ id: `software_quality.${quality}` })} + + } + /> + + (value ? setCCTType(value as CustomRuleType) : '')} + data={typeOptions} + isSearchable={false} + value={typeOptions.find((s) => s.value === cctType)?.value} + /> + + ); + }, [cctType, isDisabledInUpdate]); + + const StatusField = useMemo(() => { + const statusesOptions = RULE_STATUSES.map((status) => ({ label: translate('rules.status', status), value: status, })); @@ -237,32 +288,77 @@ export default function CustomRuleFormModal(props: Readonly) { label={translate('coding_rules.filters.status')} htmlFor="coding-rules-custom-rule-status" > - ) => setStatus(value)} - options={statusesOptions} + onChange={(value) => (value ? setStatus(value) : undefined)} + data={statusesOptions} isSearchable={false} - value={statusesOptions.find((s) => s.value === status)} + value={statusesOptions.find((s) => s.value === status)?.value} /> ); }, [status, submitting]); - const handleParameterChange = React.useCallback( - (event: React.SyntheticEvent) => { + const StandardTypeField = useMemo(() => { + const ruleTypeOption: LabelValueSelectOption[] = RULE_TYPES.map((type) => ({ + label: translate('issue.type', type), + value: type, + prefix: , + })); + return ( + +