[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form #187434

JiaweiWu · 2024-07-03T00:16:25Z

Summary

Final PR of the rule actions V2 PR (2/2 of the actions PRs). This PR contains the actions modal and actions form. This PR depends on #186490.

I have also created a example plugin to demonstrate this PR. To access:

Run the branch with yarn start --run-examples
Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/create/ (I use .es-query)
Create a rule
Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/edit/ with the rule you just created to edit the rule

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…fix'

elasticmachine · 2024-07-03T00:16:28Z

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

ryankeairns

SCSS review only - file renamed.

JiaweiWu · 2024-09-20T07:15:43Z

@cnasikas I believe this PR is ready for review now as I've addressed all of your comments

0fd8265 changes:

Address remaining comments
Added tests for:
- packages/kbn-alerts-ui-shared/src/rule_form/utils/has_fields_for_aad.ts
- packages/kbn-alerts-ui-shared/src/rule_form/utils/get_selected_action_group.ts
- packages/kbn-alerts-ui-shared/src/rule_form/validation/validate_params_for_warnings.ts

I see that the index is passed in some components but given now that we switched to action.uuid I do not think is needed. What do you think of removing it? I know that the connectors' params still using it. To avoid changing all connectors maybe we can fake it to 0 and ignore it at the framework level.

Could you clarify what you mean by index?

I am a bit confused with the purpose of onUseDefaultMessage. What is the purpose of it? Could we remove it?

Looks like it's being used by the connector params components, check here for an example:

x-pack/plugins/stack_connectors/public/connector_types/teams/teams_params.test.tsx line 33

The normal actions and the system actions have, correctly, different components. They share a common UI but a good amount of different logic. If in the future we would need to apply a UI fix or enhance it we would have to remember to do it in both places. Is it feasible to do something like where the CommonRuleActionItem has the common UI and then each *Item contains its logic?

I would agree but again, I think it's easier to have 2 for now since I'm trying to match feature parity with the existing components. I think we went cross this road we can refactor it.

JiaweiWu · 2024-09-23T21:32:05Z

@elasticmachine merge upstream

cnasikas · 2024-09-24T13:54:47Z

Hey @JiaweiWu! I tested and I can confirm that the bugs are fixed except for this one:

When I load a rule with the wrong ID I get the following error. Should we change the text from rule type -> rule?

Also, I noticed if we get a 404 error from the _resolve API, react query retries 3 times before showing the above error (screenshot). I think, if possible, it is better to not retry on 404 as nothing will change after 3 retries. We can leave this for another PR.

Could you clarify what you mean by index?

I was talking about the index prop passed to the RuleActionsItem. Nevermind, we can clean it up on another PR.

Looks like it's being used by the connector params components, check here for an example:

Ok!

I would agree but again, I think it's easier to have 2 for now since I'm trying to match feature parity with the existing components. I think we went cross this road we can refactor it.

Ok!

not forget to revert back to ALERTING_FEATURE_ID as we discussed offline :).

A reminder of the above

JiaweiWu · 2024-09-25T03:31:06Z

In the commit 2cd1eed:

Fixed messaging for when the user tries to edit a rule that doesn't exist, also no longer calls _resolve 3 times
Changed rule form to use alerts instead of stackAlerts for default consumer
Added the use AAD template for mustache switch

cnasikas

I tested and all bugs are fixed! LGTM! I left some nit comments but we can address them on a different PR.

cnasikas · 2024-09-24T14:02:06Z

packages/kbn-alerts-ui-shared/src/rule_form/rule_page/rule_page.test.tsx

Could you please add a test where we test the permissions for the actions section?

cnasikas · 2024-09-24T14:59:48Z

packages/kbn-alerts-ui-shared/src/common/hooks/use_load_rule_type_aad_template_fields.ts

  enabled: boolean;
 }

 export const useLoadRuleTypeAadTemplateField = (props: UseLoadRuleTypeAadTemplateFieldProps) => {
  const { http, ruleTypeId, enabled } = props;

  const queryFn = () => {
+    if (!ruleTypeId) {
+      return;


How do you feel about returning Promise.resolve([])? If I recall returning a promise conditionally can lead to weird race condition bugs.

cnasikas · 2024-09-24T15:06:46Z

packages/kbn-alerts-ui-shared/src/rule_form/hooks/use_load_dependencies.test.tsx

Should we test the new capabilities checks?

cnasikas · 2024-09-25T11:31:31Z

packages/kbn-alerts-ui-shared/src/rule_form/edit_rule_form.tsx

This file has quite a logic with hiding/showing some components. Should we add some basic unit tests for that?

cnasikas · 2024-09-25T12:00:53Z

packages/kbn-alerts-ui-shared/src/rule_form/rule_actions/rule_actions_item.tsx

+      payload: {
+        uuid: action.uuid!,
+        key: 'useAlertDataForTemplate',
+        value: !!!action.useAlertDataForTemplate,


nit: !Boolean(action.useAlertDataForTemplate)

joana-cps · 2024-09-25T15:01:31Z

Thanks, @JiaweiWu! I think the pattern in Kibana is to hide the section entirely if the user does not have access to a feature (though in cases we show a similar same message). I tested on the current flyout form and it is hidden. Should we follow the same pattern? @joana-cps Wdyt? About the editing, it seems that we do not allow it if you do not have access to actions. Just curiosity (not to do anything), what are the technical constraints with allowing the users to edit the rule but not the actions?

Although I don't like to have a step in the form that is disabled, I think it is a good idea to keep the section as @JiaweiWu did and inform that the user doesn't have privileges (at least for now).
And, I agree that we should explore the use cases for the Edit because it is weird to block the full edit capability when only a part of the rule requires a different privilege.

JiaweiWu · 2024-09-26T01:14:51Z

@elasticmachine merge upstream

kibana-ci · 2024-09-26T02:41:42Z

💚 Build Succeeded

Buildkite Build
Commit: f4f5704

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`alerting`	235	237	+2
`observability`	1058	1060	+2
`securitySolution`	5829	5831	+2
`triggersActionsUi`	790	798	+8
total			+14

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
`@kbn/alerts-ui-shared`	287	301	+14

Async chunks

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

id	before	after	diff
`triggersActionsUi`	1.6MB	1.6MB	-1.4KB

Page load bundle

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

id	before	after	diff
`triggersActionsUi`	121.4KB	123.3KB	+1.8KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/alerts-ui-shared`	303	317	+14

References to deprecated APIs

id	before	after	diff
`@kbn/alerts-ui-shared`	0	8	+8

History

💛 Build #236838 was flaky 2cd1eed
💛 Build #236530 was flaky f4e5341
💔 Build #236508 failed cdf7c28
💔 Build #236256 failed 364cbe7
💔 Build #235820 failed 0fd8265
💔 Build #235065 failed e0fab84

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2024-09-27T06:34:50Z

Starting backport for target branches: 8.x

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

…Form (elastic#187434) ## Summary Issue: elastic#179105 Related PR: elastic#180539 Final PR of the rule actions V2 PR (2/2 of the actions PRs). This PR contains the actions modal and actions form. This PR depends on elastic#186490. I have also created a example plugin to demonstrate this PR. To access: 1. Run the branch with yarn start --run-examples 2. Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/create/<ruleTypeId> (I use .es-query) 3. Create a rule 4. Navigate to http://localhost:5601/app/triggersActionsUiExample/rule/edit/<ruleId> with the rule you just created to edit the rule <img width="1236" alt="Screenshot 2024-07-02 at 5 15 51 PM" src="https://github.com/elastic/kibana/assets/74562234/1dc5d2a9-804a-4861-94ba-814de73dc3ab"> ![Screenshot 2024-07-08 at 10 53 44 PM](https://github.com/elastic/kibana/assets/74562234/07efade1-4b9c-485f-9833-84698dc29219) <img width="1087" alt="Screenshot 2024-07-02 at 5 15 58 PM" src="https://github.com/elastic/kibana/assets/74562234/903e66b5-f9a1-4d09-b121-b1dcecdff72c"> ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 54659e8)

kibanamachine · 2024-09-27T06:39:55Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

…tions Form (#187434) (#194254) # Backport This will backport the following commits from `main` to `8.x`: - [[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form (#187434)](#187434)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jiawei Wu <[email protected]>

JiaweiWu and others added 24 commits May 13, 2024 10:04

Initial commit

0c08c14

Revert rule-data-utils

557b271

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

3cf8e5e

…fix'

Fix sandbox

21e65be

Merge branch 'main' into rule-form-v2-rule-definition

4143a3c

Initial commit

27f519d

Remove semicolon

934935c

Add reducer

2d056f8

Add edit/create page

958450f

Update examples

828fc87

Add nav

17c6886

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

4560553

…fix'

fix multi consumer

d174f6a

Add circuit breaker error and no actions modal

5842cf5

Merge conflicts

bc5a0ac

Fix checks

775c6a1

Merge conflicts

ae2fe3a

Tests

a24c872

Merge branch 'main' into rule-form-v2-create-edit

3e9bd8d

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

6aefc7f

…fix'

Fix CI

d2734de

Fix tests

fde46b9

Prepare dependency for action form

591fde8

Actions form

6c99a7a

JiaweiWu added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.16.0 labels Jul 3, 2024

JiaweiWu requested a review from a team as a code owner July 3, 2024 00:16

ryankeairns approved these changes Sep 19, 2024

View reviewed changes

Address comments

0fd8265

Fix tests

364cbe7

elasticmachine and others added 2 commits September 23, 2024 23:32

Merge branch 'main' into rule-form-v2-action-modal-and-form

cdf7c28

Fix test and null connector bug

f4e5341

Address comments

2cd1eed

cnasikas approved these changes Sep 25, 2024

View reviewed changes

Merge branch 'main' into rule-form-v2-action-modal-and-form

f4f5704

JiaweiWu added v9.0.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed backport:skip This commit does not require backporting labels Sep 27, 2024

JiaweiWu merged commit 54659e8 into elastic:main Sep 27, 2024
37 checks passed

kibanamachine mentioned this pull request Sep 27, 2024

[8.x] [Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form (#187434) #194254

Merged

This was referenced Oct 2, 2024

[RAM] [Rule Form V2] Add action connector selection modal #179106

Closed

[Rule Form V2] Action Connector Form for Rule Form V2 #181659

Closed

[RAM] [Rule Form V2] Build new settings/message tabbed UI for actions #179107

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form #187434

[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form #187434

JiaweiWu commented Jul 3, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Jul 3, 2024

ryankeairns left a comment

JiaweiWu commented Sep 20, 2024 •

edited

Loading

JiaweiWu commented Sep 23, 2024

cnasikas commented Sep 24, 2024 •

edited

Loading

JiaweiWu commented Sep 25, 2024

cnasikas left a comment

cnasikas Sep 24, 2024

cnasikas Sep 24, 2024

cnasikas Sep 24, 2024

cnasikas Sep 25, 2024

cnasikas Sep 25, 2024

joana-cps commented Sep 25, 2024

JiaweiWu commented Sep 26, 2024

kibana-ci commented Sep 26, 2024

API count

References to deprecated APIs

kibanamachine commented Sep 27, 2024

kibanamachine commented Sep 27, 2024

[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form #187434

[Response Ops][Rule Form V2] Rule form v2: Actions Modal and Actions Form #187434

Conversation

JiaweiWu commented Jul 3, 2024 • edited by kibanamachine Loading

Summary

Checklist

elasticmachine commented Jul 3, 2024

ryankeairns left a comment

Choose a reason for hiding this comment

JiaweiWu commented Sep 20, 2024 • edited Loading

JiaweiWu commented Sep 23, 2024

cnasikas commented Sep 24, 2024 • edited Loading

JiaweiWu commented Sep 25, 2024

cnasikas left a comment

Choose a reason for hiding this comment

cnasikas Sep 24, 2024

Choose a reason for hiding this comment

cnasikas Sep 24, 2024

Choose a reason for hiding this comment

cnasikas Sep 24, 2024

Choose a reason for hiding this comment

cnasikas Sep 25, 2024

Choose a reason for hiding this comment

cnasikas Sep 25, 2024

Choose a reason for hiding this comment

joana-cps commented Sep 25, 2024

JiaweiWu commented Sep 26, 2024

kibana-ci commented Sep 26, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

References to deprecated APIs

History

kibanamachine commented Sep 27, 2024

kibanamachine commented Sep 27, 2024

💚 All backports created successfully

Questions ?

JiaweiWu commented Jul 3, 2024 •

edited by kibanamachine

Loading

JiaweiWu commented Sep 20, 2024 •

edited

Loading

cnasikas commented Sep 24, 2024 •

edited

Loading