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

[System Actions] Complete API audit and update APIs to comply with System Actions #172937

Merged

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Dec 8, 2023

Summary

Closes #172168

This updates the legacy rewriteActionsReq and rewriteRule transforms to be compatible with System Actions, and replaces all uses of legacy rewriteActionsRes with transformRuleActions, which is already System Actions-compliant.

Affected APIs are:

  • _clone
  • _find
  • _get
  • _update

@Zacqary Zacqary added the Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) label Dec 8, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@Zacqary Zacqary added release_note:skip Skip the PR/issue when compiling release notes v8.13.0 labels Dec 18, 2023
@Zacqary Zacqary marked this pull request as ready for review December 18, 2023 17:02
@Zacqary Zacqary requested a review from a team as a code owner December 18, 2023 17:02
@elasticmachine
Copy link
Contributor

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

@Zacqary Zacqary removed the v8.13.0 label Dec 18, 2023
Comment on lines +27 to +33
return {
id: action.id,
params: action.params,
...(typeof useAlertDataForTemplate !== 'undefined' ? { useAlertDataForTemplate } : {}),
...(action.uuid ? { uuid: action.uuid } : {}),
type: RuleActionTypes.SYSTEM,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's write some unit tests for this

})),
actions: actions.map(
({ id, actionTypeId, params, uuid, type, useAlertDataForTemplate, ...action }) => {
if (type === RuleActionTypes.SYSTEM) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's write some unit tests for this as well

@@ -146,15 +146,13 @@ export const updateRuleRoute = (
router.handleLegacyErrors(
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesClient = (await context.alerting).getRulesClient();
const actionsClient = (await context.actions).getActionsClient();
const { isSystemAction } = (await context.actions).getActionsClient();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have E2E test cases to assert the correct output for these endpoints?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think so but as per Zoom discussion with @XavierM this is out of scope for this PR. Tracked here: #173761

@Zacqary
Copy link
Contributor Author

Zacqary commented Jan 3, 2024

@JiaweiWu ready for review

Note that CI is failing because we're pushing to a feature branch and it can't seem to find the build

Copy link
Contributor

@JiaweiWu JiaweiWu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Zacqary Zacqary merged commit 21f0a78 into elastic:system_actions_mvp Jan 3, 2024
6 of 43 checks passed
@cnasikas cnasikas mentioned this pull request Jan 7, 2024
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants