[Security Solution] Re-enable fixed rule snoozing Cypress test (#160037)

**Addresses:** https://github.com/elastic/kibana/issues/159349

## Summary

This PR fixes and re-enables rule snoozing Cypress test `Rule editing page / actions tab - adds an action to a snoozed rule`. 

## Details

Basically the test wasn't flaky it just failed on the `main` branch and was skipped. The cause lays in interlacing [Rule snooze tests PR](https://github.com/elastic/kibana/pull/158195) with [Rule Editing page refactoring PR](https://github.com/elastic/kibana/pull/157749). Two PRs were merged independently to the `main` branch (while the tests PR was merged the last) so combining them lead to a test failure while each one separately didn't cause any problems.

### The root cause

The root cause is in a combination of factors.

[useForm](https://github.com/elastic/kibana/blob/main/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_form.ts#L33) hook has a flaw it can't update its state when new `defaultValue` comes in. The same issue also in [useField](https://github.com/elastic/kibana/blob/main/src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts#L77) hook. Rule Editing page fetched a fresh rule's data every time it's rendered via [useRule](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx#L581). As `useRule` hook is based on `React Query` it returns stale data during the first render while firing an HTTP request for the fresh data. Obviously the problem happens if the stale data is passed to `useForm` hook as `defaultValue` and when fresh data is fetched and passed again to `useForm` hook the latter just ignores it.

Staying longer on the Rule Details page helps to avoid the problem as fetched rule's data is cached and returned as stale data on the Rule Editing page after transition. As stale and fresh data are the same the test would pass. Anyway this behaviour can be reproduced in prod with a slow internet connection.

### What was done to fix the problem?

Functionality has been added to update the cached rule's data (React Query cache) upon mutation successful update rule mutation. The mutation if fired by pressing "Save changes" button on the Rule Editing page. It is possible as update rule endpoint returns an updated rule in its response.

Along the way it turned out update rule endpoint's and read rule endpoint's responses weren't properly typed so it lead to types mismatch. To fix it `updateRule`'s and `UseMutationOptions`'s return types were changed to `Rule` instead of  `RuleResponse`.

### Misc

Along the way it turned out `cy.get(RULE_INDICES).should('be.visible');` isn't required anymore to access rule actions form.
This commit is contained in:
Maxim Palenov 2023-08-11 14:05:45 +02:00 committed by GitHub
parent d5c4f461b5
commit 26e5c20238
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 49 additions and 18 deletions

View file

@ -35,7 +35,6 @@ import { duplicateFirstRule, importRules } from '../../../../../tasks/alerts_det
import { goToActionsStepTab } from '../../../../../tasks/create_new_rule';
import { goToRuleEditSettings } from '../../../../../tasks/rule_details';
import { actionFormSelector } from '../../../../../screens/common/rule_actions';
import { RULE_INDICES } from '../../../../../screens/create_new_rule';
import { addEmailConnectorAndRuleAction } from '../../../../../tasks/common/rule_actions';
import { saveEditedRule } from '../../../../../tasks/edit_rule';
import { DISABLED_SNOOZE_BADGE } from '../../../../../screens/rule_snoozing';
@ -169,8 +168,7 @@ describe('rule snoozing', () => {
});
});
// SKIPPED: https://github.com/elastic/kibana/issues/159349
describe.skip('Rule editing page / actions tab', () => {
describe('Rule editing page / actions tab', () => {
beforeEach(() => {
deleteConnectors();
});
@ -178,8 +176,6 @@ describe('rule snoozing', () => {
it('adds an action to a snoozed rule', () => {
createSnoozedRule(getNewRule({ name: 'Snoozed rule' })).then(({ body: rule }) => {
visitWithoutDateRange(ruleEditUrl(rule.id));
// Wait for rule data being loaded
cy.get(RULE_INDICES).should('be.visible');
goToActionsStepTab();
addEmailConnectorAndRuleAction('abc@example.com', 'Test action');

View file

@ -104,10 +104,15 @@ export const createRule = async ({ rule, signal }: CreateRulesProps): Promise<Ru
* @param rule RuleUpdateProps to be updated
* @param signal to cancel request
*
* @returns Promise<Rule> An updated rule
*
* In fact this function should return Promise<RuleResponse> but it'd require massive refactoring.
* It should be addressed as a part of OpenAPI schema adoption.
*
* @throws An error if response is not OK
*/
export const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise<RuleResponse> =>
KibanaServices.get().http.fetch<RuleResponse>(DETECTION_ENGINE_RULES_URL, {
export const updateRule = async ({ rule, signal }: UpdateRulesProps): Promise<Rule> =>
KibanaServices.get().http.fetch<Rule>(DETECTION_ENGINE_RULES_URL, {
method: 'PUT',
body: JSON.stringify(rule),
signal,
@ -198,6 +203,11 @@ export const fetchRules = async ({
* @param id Rule ID's (not rule_id)
* @param signal to cancel request
*
* @returns Promise<Rule>
*
* In fact this function should return Promise<RuleResponse> but it'd require massive refactoring.
* It should be addressed as a part of OpenAPI schema adoption.
*
* @throws An error if response is not OK
*/
export const fetchRuleById = async ({ id, signal }: FetchRuleProps): Promise<Rule> =>

View file

@ -59,3 +59,27 @@ export const useInvalidateFetchRuleByIdQuery = () => {
});
}, [queryClient]);
};
/**
* We should use this hook to update the rules cache when modifying a rule.
* Use it with the new rule data after operations like rule edit.
*
* @returns A rules cache update callback
*/
export const useUpdateRuleByIdCache = () => {
const queryClient = useQueryClient();
/**
* Use this method to update rules data cached by react-query.
* It is useful when we receive new rules back from a mutation query (bulk edit, etc.);
* we can merge those rules with the existing cache to avoid an extra roundtrip to re-fetch updated rules.
*/
return useCallback(
(updatedRuleResponse: Rule) => {
queryClient.setQueryData<ReturnType<typeof useFetchRuleByIdQuery>['data']>(
[...FIND_ONE_RULE_QUERY_KEY, updatedRuleResponse.id],
transformInput(updatedRuleResponse)
);
},
[queryClient]
);
};

View file

@ -6,42 +6,43 @@
*/
import type { UseMutationOptions } from '@tanstack/react-query';
import { useMutation } from '@tanstack/react-query';
import type {
RuleResponse,
RuleUpdateProps,
} from '../../../../../common/api/detection_engine/model/rule_schema';
import type { RuleUpdateProps } from '../../../../../common/api/detection_engine/model/rule_schema';
import { transformOutput } from '../../../../detections/containers/detection_engine/rules/transforms';
import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
import { updateRule } from '../api';
import { useInvalidateFindRulesQuery } from './use_find_rules_query';
import { useInvalidateFetchRuleByIdQuery } from './use_fetch_rule_by_id_query';
import { useUpdateRuleByIdCache } from './use_fetch_rule_by_id_query';
import { useInvalidateFetchRuleManagementFiltersQuery } from './use_fetch_rule_management_filters_query';
import { useInvalidateFetchCoverageOverviewQuery } from './use_fetch_coverage_overview';
import type { Rule } from '../../logic/types';
export const UPDATE_RULE_MUTATION_KEY = ['PUT', DETECTION_ENGINE_RULES_URL];
export const useUpdateRuleMutation = (
options?: UseMutationOptions<RuleResponse, Error, RuleUpdateProps>
options?: UseMutationOptions<Rule, Error, RuleUpdateProps>
) => {
const invalidateFindRulesQuery = useInvalidateFindRulesQuery();
const invalidateFetchRuleManagementFilters = useInvalidateFetchRuleManagementFiltersQuery();
const invalidateFetchRuleByIdQuery = useInvalidateFetchRuleByIdQuery();
const invalidateFetchCoverageOverviewQuery = useInvalidateFetchCoverageOverviewQuery();
const updateRuleCache = useUpdateRuleByIdCache();
return useMutation<RuleResponse, Error, RuleUpdateProps>(
return useMutation<Rule, Error, RuleUpdateProps>(
(rule: RuleUpdateProps) => updateRule({ rule: transformOutput(rule) }),
{
...options,
mutationKey: UPDATE_RULE_MUTATION_KEY,
onSettled: (...args) => {
invalidateFindRulesQuery();
invalidateFetchRuleByIdQuery();
invalidateFetchRuleManagementFilters();
invalidateFetchCoverageOverviewQuery();
if (options?.onSettled) {
options.onSettled(...args);
const [response] = args;
if (response) {
updateRuleCache(response);
}
options?.onSettled?.(...args);
},
}
);