[Rules] Adding actions to the rules details action menu (Part 1)

[baileycash-elastic]

Summary

Partially implements #199421

This PR adds snooze and enable/disable options to the action menu of the rule details header action menu.

Note: A second PR with the other 2 required actions and a slight redesign is here

[elasticmachine]

Pinging @elastic/response-ops (Team:ResponseOps)

[elasticmachine]

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

[justinkambic]

Functionality LGTM.

I tested this against a custom threshold rule and everything seemed to be working well! I have a bunch of opinionated comments that aren't required for merging but could make the code simpler/more testable.

[justinkambic]

Suggested change

	onRuleChanged={async () => {
	refetch();
	}}
	onRuleChanged={refetch}

[justinkambic]

This is an opinionated comment, but I think we have a good amount of repetition between use_disable_rule and use_enable_rule that we can abstract away. If we were to combine these two files and extract the unique values, we'd end up with a hook that requires fewer tests and allows enable/disable procedures to share common code.

Here's an example re-implementation I made that works fine with your patch:

// common code between the two hooks
function useRuleMutation({
  mutationKey,
  apiEndpoint,
  successMessage,
  errorMessage,
}: {
  mutationKey: string;
  apiEndpoint: (id: string) => string;
  successMessage: string;
  errorMessage: string;
}) {
  const {
    http,
    notifications: { toasts },
  } = useKibana().services;

  const queryClient = useQueryClient();

  return useMutation<string, string, { id: string; untrack?: boolean }>(
    [mutationKey],
    ({ id, untrack }) => {
      const body = untrack ? JSON.stringify({ untrack }) : undefined;
      try {
        return http.post(apiEndpoint(id), { body });
      } catch (e) {
        throw new Error(`Unable to parse id: ${e}`);
      }
    },
    {
      onError: () => {
        toasts.addDanger(errorMessage);
      },
      onSuccess: (_, variables) => {
        queryClient.invalidateQueries({ queryKey: ['fetchRule', variables.id], exact: false });
        toasts.addSuccess(successMessage);
      },
    }
  );
}

// implementation details provided for enable/disable, core functionality is shared
export function useEnableRule() {
  return {
    enableRule: useRuleMutation({
      mutationKey: 'enableRule',
      apiEndpoint: (id) => `/api/alerting/rule/${id}/_enable`,
      successMessage: i18n.translate(
        'xpack.observability.rules.enableConfirmationModal.successNotification.descriptionText',
        {
          defaultMessage: 'Enabled rule',
        }
      ),
      errorMessage: i18n.translate(
        'xpack.observability.rules.enableErrorModal.errorNotification.descriptionText',
        {
          defaultMessage: 'Failed to enable rule',
        }
      ),
    }),
    disableRule: useRuleMutation({
      mutationKey: 'disableRule',
      apiEndpoint: (id) => `/api/alerting/rule/${id}/_disable`,
      successMessage: i18n.translate(
        'xpack.observability.rules.disableConfirmationModal.successNotification.descriptionText',
        {
          defaultMessage: 'Disabled rule',
        }
      ),
      errorMessage: i18n.translate(
        'xpack.observability.rules.disableErrorModal.errorNotification.descriptionText',
        {
          defaultMessage: 'Failed to disable rule',
        }
      ),
    }),
  };
}

This increases the parameter footprint for the internal hook, but makes it so that the functionality of "request query and issue appropriate toast on response" is separated from the actual enable/disable details. Then the implementation in the component side looks like:

  const {
    enableRule: { mutateAsync: enableRule },
    disableRule: { mutateAsync: disableRule },
  } = useEnableRule();

We could also simply return the mutateAsync and rename it to simplify the destructuring in the component.

I wouldn't say this is required for merging but it follows DRY principles a bit more.

[justinkambic]

Suggested change

	getUntrackModal: (props: UntrackAlertsModalProps) => {
	return getUntrackModalLazy(props);
	},
	getUntrackModal: UntrackAlertsModal,

I don't know why they have this pattern here, perhaps someone from the maintaining team can respond, but this works for me locally. Probably best to just keep it this way as they have others around it that follow the same pattern.

[js-jankisalvi]

Yeah this is a long followed practice of this plugin, mostly to allow dynamic import for this components- load them when they are rendered for the first time.

[justinkambic]

Suggested change

	const onDisableModalClose = () => {
	setIsUntrackAlertsModalOpen(false);
	};
	const onDisableModalOpen = () => {
	setIsUntrackAlertsModalOpen(true);
	};

Another opinionated commented; I don't think declaring these functions is necessary.

[justinkambic]

Another opinionated comment.

Suggested change

	const handleEnableRule = () => {
	setIsRuleEditPopoverOpen(false);
	enableRule({
	id: ruleId,
	});
	};
	const handleDisableRule = (untrack: boolean) => {
	setIsRuleEditPopoverOpen(false);
	onDisableModalClose();
	disableRule({
	id: ruleId,
	untrack,
	});
	};

Regarding handleEditRule, handleRemoveRule, handleEnableRule, handleDisableRule, these functions are only referenced once, further down where they're supplied as handlers to some components. Why not just put them there and leave the functions anonymous? The code will be closer to where it's called and will remove the mental load of the reader to go back up and figure out what each handle function is doing.

I realize that the first two handle functions were already there but I think this will simplify the component.

[js-jankisalvi]

These actions are added only to Observability > Rule details page. Response ops changes lgtm 👍

[js-jankisalvi]

Yeah this is a long followed practice of this plugin, mostly to allow dynamic import for this components- load them when they are rendered for the first time.

[prodsecmachine]

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

[kibanamachine]

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/14776534047

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

[elasticmachine]

💔 Build Failed

• Buildkite Build
• Commit: d8d06e6

Failed CI Steps

• FTR Configs #15
• FTR Configs #56
• FTR Configs #56
• FTR Configs #82

Test Failures

• [job] [logs] FTR Configs #82 / apis guided onboarding GET /internal/guided_onboarding/guides returns all created guides (active and inactive)
• [job] [logs] FTR Configs #56 / ObservabilityApp Observability Rule Details page User permissions "after all" hook for "should not show the actions button if user has no permissions"
• [job] [logs] FTR Configs #56 / ObservabilityApp Observability Rule Details page User permissions "after all" hook for "should not show the actions button if user has no permissions"
• [job] [logs] FTR Configs #56 / ObservabilityApp Observability Rule Details page User permissions should show the rule edit and delete button if user has permissions
• [job] [logs] FTR Configs #56 / ObservabilityApp Observability Rule Details page User permissions should show the rule edit and delete button if user has permissions

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
observability	1277	1281	+4
triggersActionsUi	731	732	+1
total			+5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
triggersActionsUi	529	533	+4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
observability	1.3MB	1.3MB	+4.2KB
triggersActionsUi	1.4MB	1.4MB	-3.0KB
total			+1.2KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
triggersActionsUi	36	37	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
observability	94.3KB	94.3KB	-1.0B
triggersActionsUi	100.3KB	102.0KB	+1.7KB
total			+1.7KB

Unknown metric groups

API count

id	before	after	diff
triggersActionsUi	535	539	+4

History

• 💛 Build #297410 was flaky c914968
• 💛 Build #297366 was flaky 367da33
• 💔 Build #297365 failed 18f7765
• 💔 Build #297353 failed 64e5a37