[RAM] Make RuleStatusDropdown shareable #130205

Zacqary · 2022-04-13T23:17:30Z

Summary

Adds a lazy loadable getRuleStatusDropdown function to the triggersActionsUI root. It can be shared with other plugins in a similar manner to the Alert Flyout, such as this use by the infra plugin.

Moves previousSnoozeInterval logic into Localstorage, so that it can persist across plugins and page usages.

Also adds an internal __components_sandbox route to TriggersActionsUI, which can only be enabled using an experimental feature flag intended only to be used by the functional testing suite. This enables us to run some rudimentary functional tests on just the RuleStatusDropdown by itself.

Checklist

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

elasticmachine · 2022-04-13T23:17:33Z

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

…atus-dropdown

chrisronline · 2022-04-14T19:17:03Z

...iggers_actions_ui/public/application/sections/rules_list/components/rule_status_dropdown.tsx

@@ -55,34 +56,55 @@ const COMMON_SNOOZE_TIMES: Array<[number, SnoozeUnit]> = [
  [1, 'd'],
 ];

+const PREV_SNOOZE_INTERVAL_KEY = 'previousSnoozeInterval';


Should we prefix this key with our plugin name or something?

do we foresee there being a use case where the consumer might need to pass their own key? if not feel free to disregard this comment.

edit: nevermind, chris already mentioned this in another comment

chrisronline · 2022-04-14T19:19:36Z

x-pack/plugins/triggers_actions_ui/public/plugin.ts

@@ -227,6 +230,9 @@ export class Plugin
      getAlertsTable: (props: AlertsTableProps) => {
        return getAlertsTableLazy(props);
      },
+      getRuleStatusDropdown: (props: RuleStatusDropdownProps) => {


Perhaps we want to give consumers a choice if they want to inherit the previous history or start a new history for just their use case? Right now, it's all shared without choice

chrisronline · 2022-04-14T19:50:36Z

...iggers_actions_ui/public/application/sections/rules_list/components/rule_status_dropdown.tsx

+  const interval = localStorage.getItem(PREV_SNOOZE_INTERVAL_KEY);
+  const [previousSnoozeInterval, setPreviousSnoozeInterval] = useState<string | null>(interval);
+  const storeAndSetPreviousSnoozeInterval = (newInterval: string) => {
+    localStorage.setItem(PREV_SNOOZE_INTERVAL_KEY, newInterval);


So local storage never expires - do we care/want to manage that somehow? I'm not sure - this is a small amount of data

I'm not too worried about storing a single string indefinitely but if it's a blocker I can add an expiration system

Zacqary · 2022-04-14T22:27:28Z

Not sure why the functional test is failing on CI, it's passing on local fine. Will investigate and see if I can get it to be more reliable.

mgiota · 2022-04-15T09:43:38Z

@Zacqary Great! I tested out the use of RuleStatusDropdown component in Observability and works fine with one extra change I had to do in the triggers_actions_ui plugin. Could you export unsnoozeRule and snoozeRule in the this file https://github.com/elastic/kibana/blob/main/x-pack/plugins/triggers_actions_ui/public/index.ts as part of your PR?

mgiota · 2022-04-15T10:03:58Z

A small bug I noticed is in the tooltip in case user snoozed indefinitely

JiaweiWu · 2022-04-18T14:55:28Z

...ui/public/application/internal/shareable_components_sandbox/shareable_components_sandbox.tsx

+import React, { useState } from 'react';
+import { getRuleStatusDropdownLazy } from '../../../common/get_rule_status_dropdown';
+
+export const InternalShareableComponentsSandbox: React.FC<{}> = () => {


So for the components that I'm working on, I am rebasing off of this branch since I also need to E2E test the components. I'm wondering if we could maybe move the rule status dropdown rendering to its own file? so I can add my components in this sandbox as well

maybe something like?

return ( <div> <RuleStatusDropdownSandbox /> <OtherSandboxes /> </div> );

…atus-dropdown # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts

…atus-dropdown # Conflicts: # x-pack/plugins/triggers_actions_ui/public/plugin.ts

JiaweiWu

Aside from Chris's comments, this LGTM!

chrisronline

LGTM!

Zacqary · 2022-04-19T19:42:49Z

@elastic/kibana-operations I upped the triggersActionsUi limit by 1kb because the CI was failing on a 213 byte overage. Can I get an approval on that?

tylersmalley

@Zacqary is going to assist with prioritizing the work to reduce this. This is tracked in #95871

XavierM · 2022-04-19T20:00:36Z

...iggers_actions_ui/public/application/sections/rules_list/components/rule_status_dropdown.tsx

  onRuleChanged: () => void;
  enableRule: () => Promise<void>;
  disableRule: () => Promise<void>;
  snoozeRule: (snoozeEndTime: string | -1, interval: string | null) => Promise<void>;
  unsnoozeRule: () => Promise<void>;
  isEditable: boolean;
-  previousSnoozeInterval: string | null;
+  previousSnoozeInterval?: string | null;


do we need this prop? I feel like we should not have this prop anymore, am I missing something?

@chrisronline requested we keep it: #130205 (comment)

Hmm. I might have missed how this ended up. I think we can provide consumers with the choice to change the key used in local storage so they don't need to inherit previously stored intervals for other solutions

XavierM · 2022-04-19T20:04:08Z

...iggers_actions_ui/public/application/sections/rules_list/components/rule_status_dropdown.tsx

+const usePreviousSnoozeInterval: (p?: string | null) => [string | null, (n: string) => void] = (
+  propsInterval
+) => {
+  const intervalFromStorage = localStorage.getItem(PREV_SNOOZE_INTERVAL_KEY);


I am wondering if we should allow different SNOOZE_INTERVAL_KEY for different solutions, maybe let's see if we get this requirement in the future.

match with this comment #130205 (comment)

XavierM · 2022-04-19T20:27:26Z

Maybe in the future we can try to investigate to test our shareable components like that https://github.com/elastic/kibana/blob/main/x-pack/test/plugin_functional/plugins/timelines_test/public/plugin.ts to avoid dead code in our plugin. what do you think @chrisronline @JiaweiWu @Zacqary ?

kibana-ci · 2022-04-19T21:02:26Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`triggersActionsUi`	362	365	+3

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`	296	307	+11

Async chunks

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

id	before	after	diff
`triggersActionsUi`	700.5KB	711.7KB	+11.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`	24	25	+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`	99.4KB	100.2KB	+791.0B

Unknown metric groups

API count

id	before	after	diff
`triggersActionsUi`	310	321	+11

async chunk count

id	before	after	diff
`triggersActionsUi`	75	77	+2

ESLint disabled line counts

id	before	after	diff
`triggersActionsUi`	168	170	+2

Total ESLint disabled count

id	before	after	diff
`triggersActionsUi`	174	176	+2

History

💔 Build #39194 failed 6d644ed
💚 Build #38738 succeeded 4bbb9bb
💚 Build #38461 succeeded 0b05000
💔 Build #38344 failed 8ce684d
💔 Build #38285 failed e67d15a
💔 Build #38275 failed da36afa

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

Zacqary · 2022-04-19T21:34:58Z

@XavierM sorry didn't realize there was an established convention for writing tests like this, we can port that over. Might help with the bundle size issue anyway

JiaweiWu · 2022-04-20T01:56:20Z

Yea I can work on creating a separate plugins for testing purposes, since I'm gonna be writing more of these tests.

mgiota · 2022-04-20T06:42:27Z

~~@Zacqary We need unsnoozeRule and snoozeRule to be exported for the status dropdown to work for us. I will create a PR and add you as a reviewer~~

Update: I just saw your change! Thx

mgiota · 2022-04-20T10:03:52Z

x-pack/plugins/triggers_actions_ui/public/index.ts

@@ -57,6 +57,8 @@ export { enableRule } from './application/lib/rule_api/enable';
 export { disableRule } from './application/lib/rule_api/disable';
 export { muteRule } from './application/lib/rule_api/mute';
 export { unmuteRule } from './application/lib/rule_api/unmute';
+export { snoozeRule } from './application/lib/rule_api/snooze';


@Zacqary Oops somehow I missed it! Thanks I can now reuse your component. Thanks a ton!

* Add shareable e dropdown and move snooze interval to localstorage * Add internal components sandbox page and status dropdown test * Remove components sandbox tab * Fix typecheck * Fix typechecks and tests * Attempt to deflake tests * Reenable previousSnoozeInterval from props and prefix storage key * Export missing apis * Fix tooltip for indefinite snooze * Attempt to deflake functional test * Modularize Sandbox * Up triggersActionsUi package limit

Zacqary added 3 commits April 13, 2022 15:37

Add shareable e dropdown and move snooze interval to localstorage

718fffc

Add internal components sandbox page and status dropdown test

526dd9e

Remove components sandbox tab

302423b

Zacqary added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesManagement Issues related to the Rules Management UX v8.3.0 labels Apr 13, 2022

Zacqary requested a review from a team as a code owner April 13, 2022 23:17

Zacqary added 5 commits April 13, 2022 18:20

Merge remote-tracking branch 'upstream/main' into 129774-shareable-st…

5840a04

…atus-dropdown

Fix typecheck

fcf0ce8

Merge remote-tracking branch 'upstream/main' into 129774-shareable-st…

da36afa

…atus-dropdown

Fix typechecks and tests

e67d15a

Attempt to deflake tests

8ce684d

chrisronline reviewed Apr 14, 2022

View reviewed changes

This was referenced Apr 15, 2022

[Actionable Observability] Consume shareable RuleStatusDropdown component #130053

Closed

[RAM] [META] Make rules list and rule event log list shareable #129932

Closed

mgiota requested review from mgiota and removed request for mgiota April 15, 2022 10:04

Zacqary added 4 commits April 15, 2022 10:52

Reenable previousSnoozeInterval from props and prefix storage key

e502295

Export missing apis

1578db3

Fix tooltip for indefinite snooze

0809620

Attempt to deflake functional test

0b05000

Zacqary enabled auto-merge (squash) April 15, 2022 20:25

JiaweiWu reviewed Apr 18, 2022

View reviewed changes

Zacqary added 2 commits April 18, 2022 10:36

Modularize Sandbox

618e9c4

Merge remote-tracking branch 'upstream/main' into 129774-shareable-st…

4bbb9bb

…atus-dropdown # Conflicts: # x-pack/plugins/triggers_actions_ui/public/types.ts

Merge remote-tracking branch 'upstream/main' into 129774-shareable-st…

6d644ed

…atus-dropdown # Conflicts: # x-pack/plugins/triggers_actions_ui/public/plugin.ts

JiaweiWu approved these changes Apr 19, 2022

View reviewed changes

chrisronline approved these changes Apr 19, 2022

View reviewed changes

Up triggersActionsUi package limit

c64111e

Zacqary requested a review from a team as a code owner April 19, 2022 19:41

tylersmalley approved these changes Apr 19, 2022

View reviewed changes

XavierM reviewed Apr 19, 2022

View reviewed changes

XavierM disabled auto-merge April 19, 2022 20:14

Zacqary merged commit c0ab947 into elastic:main Apr 19, 2022

kibanamachine added the backport:skip This commit does not require backporting label Apr 19, 2022

mgiota reviewed Apr 20, 2022

View reviewed changes

Zacqary mentioned this pull request Apr 20, 2022

[RAM] Refactor shared components sandbox to only exist in testing suite #130728

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAM] Make RuleStatusDropdown shareable #130205

[RAM] Make RuleStatusDropdown shareable #130205

Zacqary commented Apr 13, 2022 •

edited

Loading

elasticmachine commented Apr 13, 2022

chrisronline Apr 14, 2022

JiaweiWu Apr 19, 2022 •

edited

Loading

chrisronline Apr 14, 2022

chrisronline Apr 14, 2022

Zacqary Apr 15, 2022

Zacqary commented Apr 14, 2022

mgiota commented Apr 15, 2022 •

edited

Loading

mgiota commented Apr 15, 2022

JiaweiWu Apr 18, 2022

Zacqary Apr 18, 2022

JiaweiWu Apr 19, 2022

JiaweiWu left a comment

chrisronline left a comment

Zacqary commented Apr 19, 2022

tylersmalley left a comment

XavierM Apr 19, 2022

Zacqary Apr 19, 2022

chrisronline Apr 20, 2022

XavierM Apr 19, 2022

XavierM Apr 19, 2022

XavierM commented Apr 19, 2022 •

edited

Loading

kibana-ci commented Apr 19, 2022

API count

async chunk count

ESLint disabled line counts

Total ESLint disabled count

Zacqary commented Apr 19, 2022

JiaweiWu commented Apr 20, 2022

mgiota commented Apr 20, 2022 •

edited

Loading

mgiota Apr 20, 2022

[RAM] Make RuleStatusDropdown shareable #130205

[RAM] Make RuleStatusDropdown shareable #130205

Conversation

Zacqary commented Apr 13, 2022 • edited Loading

Summary

Checklist

elasticmachine commented Apr 13, 2022

Choose a reason for hiding this comment

JiaweiWu Apr 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zacqary commented Apr 14, 2022

mgiota commented Apr 15, 2022 • edited Loading

mgiota commented Apr 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JiaweiWu left a comment

Choose a reason for hiding this comment

chrisronline left a comment

Choose a reason for hiding this comment

Zacqary commented Apr 19, 2022

tylersmalley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XavierM commented Apr 19, 2022 • edited Loading

kibana-ci commented Apr 19, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

Zacqary commented Apr 19, 2022

JiaweiWu commented Apr 20, 2022

mgiota commented Apr 20, 2022 • edited Loading

Choose a reason for hiding this comment

Zacqary commented Apr 13, 2022 •

edited

Loading

JiaweiWu Apr 19, 2022 •

edited

Loading

mgiota commented Apr 15, 2022 •

edited

Loading

XavierM commented Apr 19, 2022 •

edited

Loading

mgiota commented Apr 20, 2022 •

edited

Loading