Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Fix cursor jumping to end of text area when editing Rule Action Message #150823

Merged
merged 6 commits into from
Feb 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
import { getAllActionMessageParams } from '../../../../../../detections/pages/detection_engine/rules/helpers';

import { RuleActionsField } from '../../../../../../detections/components/rules/rule_actions_field';
import { validateRuleActionsField } from '../../../../../../detections/containers/detection_engine/rules/validate_rule_actions_field';
import { debouncedValidateRuleActionsField } from '../../../../../../detections/containers/detection_engine/rules/validate_rule_actions_field';

const CommonUseField = getUseField({ component: Field });

Expand All @@ -61,7 +61,10 @@ const getFormSchema = (
actions: {
validations: [
{
validator: validateRuleActionsField(actionTypeRegistry),
// Debounced validator not explicitly necessary here as the `RuleActionsFormData` form doesn't exhibit the same
// behavior as the `ActionsStepRule` form outlined in https://github.com/elastic/kibana/issues/142217, however
// additional renders are prevented so using for consistency
validator: debouncedValidateRuleActionsField(actionTypeRegistry),
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ export const RuleActionsField: React.FC<Props> = ({ field, messageVariables }) =
triggersActionsUi: { getActionForm },
} = useKibana().services;

// Workaround for setAlertActionsProperty being fired with prevProps when followed by setActionIdByIndex
// For details see: https://github.com/elastic/kibana/issues/142217
const [isInitializingAction, setIsInitializingAction] = useState(false);

const actions: RuleAction[] = useMemo(
() => (!isEmpty(field.value) ? (field.value as RuleAction[]) : []),
[field.value]
Expand All @@ -83,6 +87,9 @@ export const RuleActionsField: React.FC<Props> = ({ field, messageVariables }) =
const setActionIdByIndex = useCallback(
(id: string, index: number) => {
const updatedActions = [...(actions as Array<Partial<RuleAction>>)];
if (isEmpty(updatedActions[index].params)) {
setIsInitializingAction(true);
}
updatedActions[index] = deepMerge(updatedActions[index], { id });
field.setValue(updatedActions);
},
Expand All @@ -98,24 +105,30 @@ export const RuleActionsField: React.FC<Props> = ({ field, messageVariables }) =
(key: string, value: RuleActionParam, index: number) => {
// validation is not triggered correctly when actions params updated (more details in https://github.com/elastic/kibana/issues/142217)
// wrapping field.setValue in setTimeout fixes the issue above
// and triggers validation after params have been updated
setTimeout(
() =>
field.setValue((prevValue: RuleAction[]) => {
const updatedActions = [...prevValue];
updatedActions[index] = {
...updatedActions[index],
params: {
...updatedActions[index].params,
[key]: value,
},
};
return updatedActions;
}),
0
);
// and triggers validation after params have been updated, however it introduced a new issue where any additional input
// would result in the cursor jumping to the end of the text area (https://github.com/elastic/kibana/issues/149885)
const updateValue = () => {
field.setValue((prevValue: RuleAction[]) => {
const updatedActions = [...prevValue];
updatedActions[index] = {
...updatedActions[index],
params: {
...updatedActions[index].params,
[key]: value,
},
};
return updatedActions;
});
};

if (isInitializingAction) {
setTimeout(updateValue, 0);
setIsInitializingAction(false);
} else {
updateValue();
}
},
[field]
[field, isInitializingAction]
);

const actionForm = useMemo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { i18n } from '@kbn/i18n';

import type { ActionTypeRegistryContract } from '@kbn/triggers-actions-ui-plugin/public';
import { validateRuleActionsField } from '../../../containers/detection_engine/rules/validate_rule_actions_field';
import { debouncedValidateRuleActionsField } from '../../../containers/detection_engine/rules/validate_rule_actions_field';

import type { FormSchema } from '../../../../shared_imports';
import type { ActionsStepRule } from '../../../pages/detection_engine/rules/types';
Expand All @@ -21,7 +21,9 @@ export const getSchema = ({
actions: {
validations: [
{
validator: validateRuleActionsField(actionTypeRegistry),
// Debounced validator is necessary here to prevent error validation
// flashing when first adding an action. Also prevents additional renders
validator: debouncedValidateRuleActionsField(actionTypeRegistry),
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
*/

export { validateRuleActionsField } from './validate_rule_actions_field';
export { debouncedValidateRuleActionsField } from './validate_rule_actions_field';
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,22 @@

/* istanbul ignore file */

import type {
ValidationCancelablePromise,
ValidationFuncArg,
ValidationResponsePromise,
} from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib';
import type {
RuleAction,
ActionTypeRegistryContract,
} from '@kbn/triggers-actions-ui-plugin/public';
import type { ValidationFunc, ERROR_CODE, ValidationError } from '../../../../../shared_imports';
import type { RuleActionsFormData } from '../../../../../detection_engine/rule_management_ui/components/rules_table/bulk_actions/forms/rule_actions_form';
import type { ActionsStepRule } from '../../../../pages/detection_engine/rules/types';
import type { ValidationFunc, ERROR_CODE } from '../../../../../shared_imports';
import { getActionTypeName, validateMustache, validateActionParams } from './utils';

export const DEFAULT_VALIDATION_TIMEOUT = 100;

export const validateSingleAction = async (
actionItem: RuleAction,
actionTypeRegistry: ActionTypeRegistryContract
Expand All @@ -26,9 +35,7 @@ export const validateSingleAction = async (

export const validateRuleActionsField =
(actionTypeRegistry: ActionTypeRegistryContract) =>
async (
...data: Parameters<ValidationFunc>
): Promise<ValidationError<ERROR_CODE> | void | undefined> => {
async (...data: Parameters<ValidationFunc>): ValidationResponsePromise<ERROR_CODE> => {
const [{ value, path }] = data as [{ value: RuleAction[]; path: string }];

const errors = [];
Expand All @@ -51,3 +58,40 @@ export const validateRuleActionsField =
};
}
};

/**
* Debounces validation by canceling previous validation requests. Essentially leveraging the async validation
* cancellation behavior from the hook_form_lib. Necessary to prevent error validation flashing when first adding an
* action until root cause of https://github.com/elastic/kibana/issues/142217 is found
*
* See docs for details:
* https://docs.elastic.dev/form-lib/examples/validation#cancel-asynchronous-validation
*
* Note: _.throttle/debounce does not have async support, and so not used https://github.com/lodash/lodash/issues/4815.
*
* @param actionTypeRegistry
* @param defaultValidationTimeout
*/
export const debouncedValidateRuleActionsField =
(
actionTypeRegistry: ActionTypeRegistryContract,
defaultValidationTimeout = DEFAULT_VALIDATION_TIMEOUT
) =>
(data: ValidationFuncArg<ActionsStepRule | RuleActionsFormData>): ValidationResponsePromise => {
let isCanceled = false;
const promise: ValidationCancelablePromise = new Promise((resolve) => {
setTimeout(() => {
if (isCanceled) {
resolve();
} else {
resolve(validateRuleActionsField(actionTypeRegistry)(data));
}
}, defaultValidationTimeout);
});

promise.cancel = () => {
isCanceled = true;
};

return promise;
};