[RAM] [Rule Form V2] Add new rule type selection modal #179285

Zacqary · 2024-03-22T19:00:29Z

Summary

Stack Management Rules Page

Screen.Recording.2024-03-22.at.1.51.06.PM.mov

Observability Rules Page

This adds the new Rule Type Flyout from the Create Rule Flow v2 project. As of this PR, it simply replaces the rule type selection flow from the existing v1 flyout, and opens the v1 flyout when the user selects a rule type.

This functionality works well enough with existing v1 code that I'm comfortable not putting this modal behind a feature flag. Future UI additions for Create Rule v2, such as the full-page experience, will be behind feature flags.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

…lection-modal # Conflicts: # x-pack/plugins/observability_solution/observability/public/pages/rules/rules.tsx

apmmachine · 2024-03-22T19:00:42Z

🤖 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 · 2024-03-22T19:07:00Z

Watching the screen recording back I realized I didn't like the empty prompt, so here's a new one:

…lection-modal

elasticmachine · 2024-03-25T18:21:29Z

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

elasticmachine · 2024-03-25T23:39:48Z

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

maryam-saeidi · 2024-03-26T10:12:20Z

@Zacqary When I run this branch locally, I see some errors in Kibana server, do you get the same locally?

       │          ERROR in /Users/maryam.saeidi/WebstormProjects/kibana-pr/packages/kbn-alerts-ui-shared/src/rule_type_modal/components/index.tsx
       │          Module not found: Error: Can't resolve '../common/hooks' in '/Users/maryam.saeidi/WebstormProjects/kibana-pr/packages/kbn-alerts-ui-shared/src/rule_type_modal/components'
       │          resolve '../common/hooks' in '/Users/maryam.saeidi/WebstormProjects/kibana-pr/packages/kbn-alerts-ui-shared/src/rule_type_modal/components'
       │            using description file: /Users/maryam.saeidi/WebstormProjects/kibana-pr/packages/kbn-alerts-ui-shared/package.json (relative path: ./src/rule_type_modal/components)
       │              Field 'browser' doesn't contain a valid alias configuration
       │              using description file: /Users/maryam.saeidi/WebstormProjects/kibana-pr/packages/kbn-alerts-ui-shared/package.json (relative path: ./src/rule_type_modal/common/hooks)
       │                no extension
       │                  Field 'browser' doesn't contain a valid alias configuration
       │                  /Users/maryam.saeidi/WebstormProjects/kibana-pr/packages/kbn-alerts-ui-shared/src/rule_type_modal/common/hooks doesn't exist
       │                .js

maryam-saeidi · 2024-03-26T10:19:21Z

Also, when I check the rule creation flyout in observability, I don't see what you showed in the PR description, is this expected or should I enable a feature flag for seeing this view?

Screen.Recording.2024-03-26.at.11.15.01.mov

In the view that you shared, I wonder if we can show the first category available in the left-hand menu instead of showing the rule types now found: 🤔

Zacqary · 2024-03-26T16:06:54Z

@maryam-saeidi Pushed a fix, if you check out the branch again the errors should disappear and you'll be able to see the new modal in Observability

…fix'

…lection-modal

doakalexi

Tested locally and works as expected, LGTM
It would be good to add tests for the rule type list, rule type modal, and the load rule types hook.

Zacqary · 2024-03-29T16:35:46Z

@doakalexi Integration tests that depend on the rule type selection from V1 have all been updated. I abstracted out most of the complex functionality of the rule type component into the helper function and covered that with Jest tests, and I wasn't able to think of other testing scenarios for the modal and list components that wouldn't amount to just repeating the code in test form. Is there something you can think of that I missed?

doakalexi · 2024-03-29T16:43:35Z

@doakalexi Integration tests that depend on the rule type selection from V1 have all been updated. I abstracted out most of the complex functionality of the rule type component into the helper function and covered that with Jest tests, and I wasn't able to think of other testing scenarios for the modal and list components that wouldn't amount to just repeating the code in test form. Is there something you can think of that I missed?

Sounds good! Just wanted to make sure everything that maybe needed tests had them.

lukasolson

DataDiscovery code changes LGTM

Zacqary · 2024-04-01T16:17:49Z

@elasticmachine merge upstream

maryam-saeidi

Tested locally and worked as expected!

Just one comment:
In the new implementation, I didn't see the ability to change the rule type as we did it previously:

Before	Now

Zacqary · 2024-04-02T18:21:06Z

@maryam-saeidi Yes, removing the X button is intentional because we're no longer listing the rule types in the flyout. To change the rule type, the user will need to go back to the new modal.

In V2 Observability will link to the full page, from which the user can only change rule types by clicking the Return link to bring them back to the rules list, which is a similar experience to what we're pushing in this PR. As far as I know the usability studies design did showed that the extra clicks required weren't a problem.

...s/kbn-alerts-ui-shared/src/rule_type_modal/components/helpers/filter_and_count_rule_types.ts

JiaweiWu · 2024-04-02T19:09:47Z

...s/kbn-alerts-ui-shared/src/rule_type_modal/components/helpers/filter_and_count_rule_types.ts

+    }
+    return true;
+  });
+  const ruleTypesFilteredBySearchAndProducer = ruleTypesFilteredBySearch.filter((ruleType) => {


let's filter by the producer first, as this might reduce the number of rule types we need to search through

We don't do this for UX reasons. Test cases explain it but I'll add a comment

packages/kbn-alerts-ui-shared/src/rule_type_modal/components/index.tsx

JiaweiWu · 2024-04-02T19:12:24Z

packages/kbn-alerts-ui-shared/src/rule_type_modal/components/index.tsx

+  registeredRuleTypes,
+  ...props
+}) => {
+  const [selectedProducer, setSelectedProducer] = useState<string | null>(null);


perhaps AlertConsumers instead of string?

Producers aren't the same as consumers, and we don't have clear typedefs for these

packages/kbn-alerts-ui-shared/src/rule_type_modal/components/rule_type_list.tsx

JiaweiWu · 2024-04-02T19:18:40Z

packages/kbn-alerts-ui-shared/src/rule_type_modal/components/rule_type_list.tsx

+}
+
+const producerToDisplayName = (producer: string) => {
+  return Reflect.get(PRODUCER_DISPLAY_NAMES, producer) ?? producer;


never seen Reflect.get before haha, is this any different than accessing the object normally?

Gets around Typescript complaints about dynamically indexing an object with an unknown string.

packages/kbn-alerts-ui-shared/src/rule_type_modal/components/rule_type_list.tsx

packages/kbn-alerts-ui-shared/src/rule_type_modal/components/rule_type_modal.tsx

…/Zacqary/kibana into 179104-rule-type-selection-modal

kibana-ci · 2024-04-02T23:45:41Z

💚 Build Succeeded

Buildkite Build
Commit: 7ea00de

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`alerting`	156	164	+8
`observability`	476	484	+8
`securitySolution`	5042	5050	+8
`triggersActionsUi`	690	698	+8
total			+32

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`	90	82	-8

Async chunks

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

id	before	after	diff
`alerting`	89.2KB	90.7KB	+1.5KB
`observability`	262.7KB	270.4KB	+7.7KB
`securitySolution`	16.6MB	16.6MB	+1.3KB
`triggersActionsUi`	1.6MB	1.6MB	+210.0B
total			+10.7KB

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
`@kbn/alerts-ui-shared`	1	2	+1

Page load bundle

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

id	before	after	diff
`triggersActionsUi`	108.7KB	116.2KB	+7.4KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/alerts-ui-shared`	95	87	-8

History

💚 Build #200338 succeeded 417f430
💛 Build #200102 was flaky 55a2632
💔 Build #200088 failed 64cfb3a
💔 Build #200081 failed 2ee66fb
💔 Build #199808 failed de0b6aa
💔 Build #199768 failed 10e9a66

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

maryam-saeidi

LGTM!

jasonrhodes · 2024-04-03T14:15:36Z

Questions from our alerting weekly meeting:

What does this modal look like in serverless (observability project)?
Does the search box in the modal search the description text (like the existing flyout search does today)?

Zacqary · 2024-04-03T16:57:38Z

@jasonrhodes

Does the search box in the modal search the description text (like the existing flyout search does today)?

Yes. See these lines of the search code:

 if (searchString) {
      const lowerCaseSearchString = searchString.toLowerCase();
      return (
        ruleType.name.toLowerCase().includes(lowerCaseSearchString) ||
        (ruleType.description && ruleType.description.toLowerCase().includes(lowerCaseSearchString))
      );
    }

What does this modal look like in serverless (observability project)?

Producers are all listed as Observability so we don't get the categories present on the traditional system. I'll open an issue around that.

Zacqary · 2024-04-03T17:04:12Z

@jasonrhodes Tracking the serverless category issue in #179963

jasonrhodes · 2024-04-03T18:29:45Z

@vinaychandrasekhar please see above answers, thanks!

jasonrhodes · 2024-04-10T19:49:19Z

I spoke with @vinaychandrasekhar and we both agree, we're good with this shipping in 8.14, provided categorization is removed from the flyout for observability context. Thanks!

Zacqary added 3 commits March 22, 2024 11:14

Add initial select rule type modal with visual design

a3ef788

Connect rule type modal to rule add flyout, o11y, filtering

46e94aa

Merge remote-tracking branch 'upstream/main' into 179104-rule-type-se…

aba8e8a

…lection-modal # Conflicts: # x-pack/plugins/observability_solution/observability/public/pages/rules/rules.tsx

Zacqary added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX v8.14.0 labels Mar 22, 2024

Improve empty prompt

47ef2d0

Zacqary added 6 commits March 25, 2024 11:56

Merge remote-tracking branch 'upstream/main' into 179104-rule-type-se…

df69645

…lection-modal

Restructure and add tests

8ee863b

Rename rule type index hook

4862a8c

Fix hook export name

752b013

Move loaCombine load rule types query hook into common

dbc2456

Remove unused exports

5d890e8

Zacqary marked this pull request as ready for review March 25, 2024 18:21

Zacqary requested review from a team as code owners March 25, 2024 18:21

Zacqary added release_note:feature Makes this part of the condensed release notes Team:obs-ux-management Observability Management User Experience Team labels Mar 25, 2024

maryam-saeidi self-requested a review March 26, 2024 10:08

Fix hook import

0f34f1d

Zacqary requested review from JiaweiWu and doakalexi March 26, 2024 16:08

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

376cfd4

…fix'

Zacqary added 5 commits March 27, 2024 11:54

Fix import

de0b6aa

Fix stack and o11y functional tests

c7514ce

Fix jest

8973870

Merge remote-tracking branch 'upstream/main' into 179104-rule-type-se…

2ee66fb

…lection-modal

Fix typecheck

64cfb3a

doakalexi approved these changes Mar 28, 2024

View reviewed changes

Fix discover tests

55a2632

Zacqary requested a review from a team as a code owner March 28, 2024 21:53

lukasolson approved these changes Mar 30, 2024

View reviewed changes

Merge branch 'main' into 179104-rule-type-selection-modal

417f430

maryam-saeidi reviewed Apr 2, 2024

View reviewed changes

JiaweiWu reviewed Apr 2, 2024

View reviewed changes

Zacqary added 4 commits April 2, 2024 16:56

Add translations and optimizations

0aefba4

Update var and prop names

3fb57cc

Merge branch '179104-rule-type-selection-modal' of https://github.com…

b2d8f0d

…/Zacqary/kibana into 179104-rule-type-selection-modal

Use euiTheme constant in ruleTypeModal

7ea00de

Zacqary requested a review from maryam-saeidi April 2, 2024 22:33

maryam-saeidi approved these changes Apr 3, 2024

View reviewed changes

Zacqary merged commit 658cbde into elastic:main Apr 3, 2024
17 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Apr 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAM] [Rule Form V2] Add new rule type selection modal #179285

[RAM] [Rule Form V2] Add new rule type selection modal #179285

Zacqary commented Mar 22, 2024 •

edited

Loading

apmmachine commented Mar 22, 2024

Zacqary commented Mar 22, 2024 •

edited

Loading

elasticmachine commented Mar 25, 2024

elasticmachine commented Mar 25, 2024

maryam-saeidi commented Mar 26, 2024

maryam-saeidi commented Mar 26, 2024

Zacqary commented Mar 26, 2024

doakalexi left a comment •

edited

Loading

Zacqary commented Mar 29, 2024

doakalexi commented Mar 29, 2024

lukasolson left a comment

Zacqary commented Apr 1, 2024

maryam-saeidi left a comment •

edited

Loading

Zacqary commented Apr 2, 2024 •

edited

Loading

JiaweiWu Apr 2, 2024

Zacqary Apr 2, 2024

JiaweiWu Apr 2, 2024

Zacqary Apr 2, 2024

JiaweiWu Apr 2, 2024

Zacqary Apr 2, 2024

kibana-ci commented Apr 2, 2024

API count

maryam-saeidi left a comment

jasonrhodes commented Apr 3, 2024

Zacqary commented Apr 3, 2024

Zacqary commented Apr 3, 2024

jasonrhodes commented Apr 3, 2024

jasonrhodes commented Apr 10, 2024

[RAM] [Rule Form V2] Add new rule type selection modal #179285

[RAM] [Rule Form V2] Add new rule type selection modal #179285

Conversation

Zacqary commented Mar 22, 2024 • edited Loading

Summary

Stack Management Rules Page

Observability Rules Page

Checklist

apmmachine commented Mar 22, 2024

🤖 GitHub comments

Zacqary commented Mar 22, 2024 • edited Loading

elasticmachine commented Mar 25, 2024

elasticmachine commented Mar 25, 2024

maryam-saeidi commented Mar 26, 2024

maryam-saeidi commented Mar 26, 2024

Zacqary commented Mar 26, 2024

doakalexi left a comment • edited Loading

Choose a reason for hiding this comment

Zacqary commented Mar 29, 2024

doakalexi commented Mar 29, 2024

lukasolson left a comment

Choose a reason for hiding this comment

Zacqary commented Apr 1, 2024

maryam-saeidi left a comment • edited Loading

Choose a reason for hiding this comment

Zacqary commented Apr 2, 2024 • edited Loading

JiaweiWu Apr 2, 2024

Choose a reason for hiding this comment

Zacqary Apr 2, 2024

Choose a reason for hiding this comment

JiaweiWu Apr 2, 2024

Choose a reason for hiding this comment

Zacqary Apr 2, 2024

Choose a reason for hiding this comment

JiaweiWu Apr 2, 2024

Choose a reason for hiding this comment

Zacqary Apr 2, 2024

Choose a reason for hiding this comment

kibana-ci commented Apr 2, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

maryam-saeidi left a comment

Choose a reason for hiding this comment

jasonrhodes commented Apr 3, 2024

Zacqary commented Apr 3, 2024

Zacqary commented Apr 3, 2024

jasonrhodes commented Apr 3, 2024

jasonrhodes commented Apr 10, 2024

Zacqary commented Mar 22, 2024 •

edited

Loading

Zacqary commented Mar 22, 2024 •

edited

Loading

doakalexi left a comment •

edited

Loading

maryam-saeidi left a comment •

edited

Loading

Zacqary commented Apr 2, 2024 •

edited

Loading