From 3c3ee9dbc3e54a3b1920e1db3006174b205dee91 Mon Sep 17 00:00:00 2001 From: stanislavh Date: Wed, 6 Dec 2023 11:08:25 +0100 Subject: [PATCH] SONAR-21179 Use update endpoint when reactivating a rule --- .../js/api/mocks/CodingRulesServiceMock.ts | 46 ++++++++++--- .../src/main/js/api/mocks/data/ids.ts | 1 + .../src/main/js/api/mocks/data/rules.ts | 15 ++++- .../coding-rules/__tests__/CustomRule-it.ts | 24 +++++++ .../components/CustomRuleFormModal.tsx | 66 ++++++++++++++----- .../components/RuleDetailsDescription.tsx | 2 +- .../main/js/apps/coding-rules/utils-tests.tsx | 1 + server/sonar-web/src/main/js/queries/rules.ts | 11 +++- 8 files changed, 137 insertions(+), 29 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 e7f50d93529..d0c9612a95a 100644 --- a/server/sonar-web/src/main/js/api/mocks/CodingRulesServiceMock.ts +++ b/server/sonar-web/src/main/js/api/mocks/CodingRulesServiceMock.ts @@ -17,6 +17,7 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +import { HttpStatusCode } from 'axios'; import { cloneDeep, countBy, pick, trim } from 'lodash'; import { RuleDescriptionSections } from '../../apps/coding-rules/rule'; import { getStandards } from '../../helpers/security-standard'; @@ -30,7 +31,7 @@ import { import { RuleRepository, SearchRulesResponse } from '../../types/coding-rules'; import { ComponentQualifier, Visibility } from '../../types/component'; import { RawIssuesResponse } from '../../types/issues'; -import { SearchRulesQuery } from '../../types/rules'; +import { RuleStatus, SearchRulesQuery } from '../../types/rules'; import { SecurityStandard } from '../../types/security'; import { Dict, Rule, RuleActivation, RuleDetails, RulesUpdateRequest } from '../../types/types'; import { NoticeType } from '../../types/users'; @@ -174,7 +175,7 @@ export default class CodingRulesServiceMock { cwe, activation, }: FacetFilter) { - let filteredRules = this.rules; + let filteredRules = this.getRulesFilteredByRemovedStatus(); if (cleanCodeAttributeCategories) { filteredRules = filteredRules.filter( (r) => @@ -266,12 +267,16 @@ export default class CodingRulesServiceMock { this.rulesActivations = mockRulesActivationsInQP(); } + getRulesFilteredByRemovedStatus() { + return this.rules.filter((r) => r.status !== RuleStatus.Removed); + } + allRulesCount() { - return this.rules.length; + return this.getRulesFilteredByRemovedStatus().length; } allRulesName() { - return this.rules.map((r) => r.name); + return this.getRulesFilteredByRemovedStatus().map((r) => r.name); } allQualityProfile(language: string) { @@ -322,12 +327,14 @@ export default class CodingRulesServiceMock { }; handleUpdateRule = (data: RulesUpdateRequest): Promise => { - const rule = this.rules.find((r) => r.key === data.key); + // find rule if key is in the list of rules or key is a part of 'repo:key' + const rule = this.rules.find((r) => data.key.split(':').some((part) => part === r.key)); if (rule === undefined) { return Promise.reject({ errors: [{ msg: `No rule has been found for id ${data.key}` }], }); } + const template = this.rules.find((r) => r.key === rule.templateKey); // Lets not convert the md to html in test. @@ -337,6 +344,7 @@ export default class CodingRulesServiceMock { rule.mdNote = data.markdown_note !== undefined ? data.markdown_note : rule.mdNote; 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; if (template && data.params) { rule.params = []; data.params.split(';').forEach((param) => { @@ -377,6 +385,27 @@ export default class CodingRulesServiceMock { }) ?? [], }); + const rulesFromTemplateWithSameKeys = this.rules.filter( + (rule) => rule.templateKey === newRule.templateKey && rule.key === newRule.key, + ); + + if (rulesFromTemplateWithSameKeys.length > 0) { + if ( + rulesFromTemplateWithSameKeys.find( + (rule) => rule.status === RuleStatus.Removed && rule.key === newRule.key, + ) + ) { + return Promise.reject({ + status: HttpStatusCode.Conflict, + errors: [{ msg: `Rule with the same was removed before` }], + }); + } + + return Promise.reject({ + errors: [{ msg: `A rule with key ${newRule.key} already exists` }], + }); + } + this.rules.push(newRule); return this.reply(newRule); @@ -469,6 +498,7 @@ export default class CodingRulesServiceMock { activation, }); } + const responseRules = filteredRules.slice((currentP - 1) * currentPs, currentP * currentPs); return this.reply({ actives: qprofile ? this.rulesActivations : undefined, @@ -485,13 +515,13 @@ export default class CodingRulesServiceMock { handleBulkActivateRules = () => { if (this.applyWithWarning) { return this.reply({ - succeeded: this.rules.length - 1, + succeeded: this.getRulesFilteredByRemovedStatus().length - 1, failed: 1, errors: [{ msg: 'c rule c:S6069 cannot be activated on cpp profile SonarSource' }], }); } return this.reply({ - succeeded: this.rules.length, + succeeded: this.getRulesFilteredByRemovedStatus().length, failed: 0, errors: [], }); @@ -499,7 +529,7 @@ export default class CodingRulesServiceMock { handleBulkDeactivateRules = () => { return this.reply({ - succeeded: this.rules.length, + succeeded: this.getRulesFilteredByRemovedStatus().length, failed: 0, }); }; diff --git a/server/sonar-web/src/main/js/api/mocks/data/ids.ts b/server/sonar-web/src/main/js/api/mocks/data/ids.ts index f680b17e2a8..502e5600ffd 100644 --- a/server/sonar-web/src/main/js/api/mocks/data/ids.ts +++ b/server/sonar-web/src/main/js/api/mocks/data/ids.ts @@ -50,6 +50,7 @@ export const RULE_8 = 'rule8'; export const RULE_9 = 'rule9'; export const RULE_10 = 'rule10'; export const RULE_11 = 'rule11'; +export const RULE_12 = 'rule12'; // Quality Profiles. export const QP_1 = 'p1'; diff --git a/server/sonar-web/src/main/js/api/mocks/data/rules.ts b/server/sonar-web/src/main/js/api/mocks/data/rules.ts index f7a1aca3dbf..3616cb845e4 100644 --- a/server/sonar-web/src/main/js/api/mocks/data/rules.ts +++ b/server/sonar-web/src/main/js/api/mocks/data/rules.ts @@ -25,6 +25,7 @@ import { SoftwareImpactSeverity, SoftwareQuality, } from '../../../types/clean-code-taxonomy'; +import { RuleStatus } from '../../../types/rules'; import { ADVANCED_RULE, QP_1, @@ -34,6 +35,7 @@ import { RULE_1, RULE_10, RULE_11, + RULE_12, RULE_2, RULE_3, RULE_4, @@ -214,7 +216,7 @@ export function mockRuleDetailsList() { { key: '1', type: 'TEXT', htmlDesc: 'html description for key 1' }, { key: '2', type: 'NUMBER', defaultValue: 'default value for key 2' }, ], - templateKey: 'rule8', + templateKey: RULE_8, }), mockRuleDetails({ createdAt: '2022-12-16T17:26:54+0100', @@ -242,6 +244,17 @@ export function mockRuleDetailsList() { langName: 'Java', name: 'Common java rule', }), + mockRuleDetails({ + key: RULE_12, + type: 'BUG', + severity: 'MINOR', + lang: 'py', + langName: 'Python', + tags: ['awesome'], + name: 'Deleted custom rule based on rule8', + templateKey: RULE_8, + status: RuleStatus.Removed, + }), ]; } 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 850c4314fed..7533d1f4810 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 @@ -21,6 +21,7 @@ import selectEvent from 'react-select-event'; import CodingRulesServiceMock from '../../../api/mocks/CodingRulesServiceMock'; import SettingsServiceMock from '../../../api/mocks/SettingsServiceMock'; import { mockLoggedInUser } from '../../../helpers/testMocks'; +import { byText } from '../../../helpers/testSelector'; import { SoftwareQuality } from '../../../types/clean-code-taxonomy'; import { getPageObjects, renderCodingRulesApp } from '../utils-tests'; @@ -101,6 +102,29 @@ describe('custom rule', () => { expect(ui.customRuleItemLink('New Custom Rule').get()).toBeInTheDocument(); }); + it('can reactivate custom rule', async () => { + const { ui, user } = getPageObjects(); + rulesHandler.setIsAdmin(); + renderCodingRulesApp(mockLoggedInUser(), 'coding_rules?open=rule8'); + await ui.detailsloaded(); + + // Try create custom rule with existing rule with removed status + await user.click(ui.createCustomRuleButton.get()); + await user.type(ui.ruleNameTextbox.get(), 'Reactivate custom Rule'); + await user.clear(ui.keyTextbox.get()); + await user.type(ui.keyTextbox.get(), 'rule12'); + await user.type(ui.descriptionTextbox.get(), 'Some description for custom rule'); + + await user.click(ui.createButton.get()); + + expect(byText('coding_rules.reactivate.help').get()).toBeInTheDocument(); + + // Reactivate rule + await user.click(ui.reactivateButton.get()); + // Verify the rule is reactivated + expect(ui.customRuleItemLink('Reactivate custom Rule').get()).toBeInTheDocument(); + }); + it('can edit custom rule', async () => { const { ui, user } = getPageObjects(); rulesHandler.setIsAdmin(); diff --git a/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormModal.tsx b/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormModal.tsx index dce99ab2ba7..87d3c2dd768 100644 --- a/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormModal.tsx +++ b/server/sonar-web/src/main/js/apps/coding-rules/components/CustomRuleFormModal.tsx @@ -74,17 +74,22 @@ export default function CustomRuleFormModal(props: Readonly) { templateRule.cleanCodeAttribute ?? CleanCodeAttribute.Conventional, ); const [impacts, setImpacts] = React.useState(templateRule?.impacts ?? []); - const { mutate: updateRule, isLoading: updatingRule } = useUpdateRuleMutation(props.onClose); + const customRulesSearchParams = { + f: 'name,severity,params', + template_key: templateRule.key, + }; + const { mutate: updateRule, isLoading: updatingRule } = useUpdateRuleMutation( + customRulesSearchParams, + props.onClose, + ); const { mutate: createRule, isLoading: creatingRule } = useCreateRuleMutation( - { - f: 'name,severity,params', - template_key: templateRule.key, - }, + customRulesSearchParams, props.onClose, (response: Response) => { setReactivating(response.status === HttpStatusCode.Conflict); }, ); + const warningRef = React.useRef(null); const submitting = updatingRule || creatingRule; const hasError = impacts.length === 0; @@ -99,18 +104,41 @@ export default function CustomRuleFormModal(props: Readonly) { params: stringifiedParams, status, }; - return customRule - ? updateRule({ ...ruleData, key: customRule.key }) - : createRule({ - ...ruleData, - customKey: key, - preventReactivation: !reactivating, - templateKey: templateRule.key, - cleanCodeAttribute: ccAttribute, - impacts, - }); + + if (customRule) { + updateRule({ + ...ruleData, + key: customRule.key, + }); + } else if (reactivating) { + updateRule({ + ...ruleData, + key: `${templateRule.repo}:${key}`, + }); + } else { + createRule({ + ...ruleData, + customKey: key, + templateKey: templateRule.key, + preventReactivation: true, + cleanCodeAttribute: ccAttribute, + impacts, + }); + } }; + // If key changes, then most likely user did it to create a new rule instead of reactivating one + React.useEffect(() => { + setReactivating(false); + }, [key]); + + // scroll to warning when it appears + React.useEffect(() => { + if (reactivating) { + warningRef.current?.scrollIntoView({ behavior: 'smooth' }); + } + }, [reactivating]); + const NameField = React.useMemo( () => ( ) { }} > {reactivating && ( - - {translate('coding_rules.reactivate.help')} - +
+ + {translate('coding_rules.reactivate.help')} + +
)} diff --git a/server/sonar-web/src/main/js/apps/coding-rules/components/RuleDetailsDescription.tsx b/server/sonar-web/src/main/js/apps/coding-rules/components/RuleDetailsDescription.tsx index f3357a6a793..22905f3f68f 100644 --- a/server/sonar-web/src/main/js/apps/coding-rules/components/RuleDetailsDescription.tsx +++ b/server/sonar-web/src/main/js/apps/coding-rules/components/RuleDetailsDescription.tsx @@ -47,7 +47,7 @@ export default function RuleDetailsDescription(props: Readonly) { const [descriptionForm, setDescriptionForm] = React.useState(false); const [removeDescriptionModal, setDescriptionModal] = React.useState(false); - const { mutate: updateRule, isLoading: updatingRule } = useUpdateRuleMutation(() => + const { mutate: updateRule, isLoading: updatingRule } = useUpdateRuleMutation(undefined, () => setDescriptionForm(false), ); diff --git a/server/sonar-web/src/main/js/apps/coding-rules/utils-tests.tsx b/server/sonar-web/src/main/js/apps/coding-rules/utils-tests.tsx index 1bb89ffafb7..9e149265f6b 100644 --- a/server/sonar-web/src/main/js/apps/coding-rules/utils-tests.tsx +++ b/server/sonar-web/src/main/js/apps/coding-rules/utils-tests.tsx @@ -191,6 +191,7 @@ const selectors = { statusSelect: byRole('combobox', { name: 'coding_rules.filters.status' }), descriptionTextbox: byRole('textbox', { name: 'description' }), createButton: byRole('button', { name: 'create' }), + reactivateButton: byRole('button', { name: 'coding_rules.reactivate' }), deleteButton: byRole('button', { name: 'delete' }), }; diff --git a/server/sonar-web/src/main/js/queries/rules.ts b/server/sonar-web/src/main/js/queries/rules.ts index b7c303b2e67..22e03e2f257 100644 --- a/server/sonar-web/src/main/js/queries/rules.ts +++ b/server/sonar-web/src/main/js/queries/rules.ts @@ -74,13 +74,22 @@ export function useCreateRuleMutation( }); } -export function useUpdateRuleMutation(onSuccess?: (rule: RuleDetails) => unknown) { +export function useUpdateRuleMutation( + searchQuery?: SearchRulesQuery, + onSuccess?: (rule: RuleDetails) => unknown, +) { const queryClient = useQueryClient(); return useMutation({ mutationFn: updateRule, onSuccess: (rule) => { onSuccess?.(rule); + queryClient.setQueryData( + getRulesQueryKey('search', searchQuery), + (oldData) => { + return oldData ? { ...oldData, rules: [rule, ...oldData.rules] } : undefined; + }, + ); queryClient.setQueryData<{ actives?: RuleActivation[]; rule: RuleDetails }>( getRulesQueryKey('details', rule.key), (oldData) => { -- 2.39.5