[App Search] Added the log retention panel to the Settings page #82982

JasonStoltz · 2020-11-09T17:07:37Z

Summary

This PR adds the log retention section to the App Search Settings page.

Options can be enabled, but can not yet be disabled, as they require the next PR, which will add a modal.

In the meantime, you can navigate directly to the stand-alone UI to enable and disable options.

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

cee-chen

Mostly very minor nit change requests and some rambling from me

cee-chen · 2020-11-09T19:11:54Z

...arch/public/applications/app_search/components/settings/log_retention/messaging/constants.ts

+  }
+);
+
+const API_STORED = (minAgeDays: number | null | undefined) =>


It seems weird to have null and undefined as a type option here, although I'm not sure we can do anything about it since I assume this is data coming from the server

Yeah, it's weird, I agree.

cee-chen · 2020-11-09T19:15:17Z

...ications/app_search/components/settings/log_retention/messaging/determine_tooltip_content.ts

+    logRetentionSettings.retentionPolicy?.isDefault
+  ) {
+    return renderOrReturnMessage(messages.defaultPolicy);
+  }


We're getting an uncovered branch line on line 39 - I assume basically Jest wants to know what happens if all our if branches are skipped and it passes through to the very end, which is unlikely to happen in a real world scenario. Probably not really worth covering right now / something to look at later as tech debt. If you'd like me to look at this after your PRs get in today, LMK

I saw that as well and couldn't quite figure out what the failure was all about.

...ons/app_search/components/settings/log_retention/messaging/determine_tooltip_content.test.ts

cee-chen · 2020-11-09T19:22:59Z

...ons/app_search/components/settings/log_retention/messaging/determine_tooltip_content.test.ts

+            determineTooltipContent(API_MESSAGES, true, {
+              ...BASE_SETTINGS,
+              enabled: false,
+              disabledAt: 'Thu, 05 Nov 2020 18:57:28 +0000',


Not a change request, just an aside: using current dates for tests is always somewhat amusing because they end up acting as a time capsule when looking back at code. Like, someone will come in here 5 years from now and be like "oh I guess this code is 5 years old haha". Personally, I like using Jan 1 1970 as a clear way of saying in a test "this is an arbitrary/mock date".

...rch/public/applications/app_search/components/settings/log_retention/log_retention_panel.tsx

...ublic/applications/app_search/components/settings/log_retention/log_retention_panel.test.tsx

cee-chen · 2020-11-09T19:59:44Z

...ublic/applications/app_search/components/settings/log_retention/log_retention_panel.test.tsx

+  it('renders API switch off when apiLogRetention is false in LogRetentionLogic ', () => {
+    setMockValues({
+      isLogRetentionUpdating: false,
+      logRetention: mockLogRetention({
+        api: {
+          enabled: false,
+        },
+      }),
+    });
+
+    const logRetentionPanel = shallow(<LogRetentionPanel />);
+    expect(
+      logRetentionPanel.find(`[data-test-subj="LogRetentionPanelAPISwitch"]`).prop('checked')
+    ).toEqual(false);
+  });
+
+  it('renders API switch on when apiLogRetention is true in LogRetentionLogic ', () => {


[Not a change request] It might be interesting (later, as tech debt) to combine these two tests into a single test that tests both true/false. Might save on some setup time and combines the tests conceptually. I'd have to DRY out a rerender helper of some kind though.

const logRetentionPanel = shallow(<LogRetentionPanel />); setMockValues({ ... }); logRetentionPanel.setProps({}); // rerenders/updates component expect(switchIsChecked).toEqual(false); setMockValues({ ... }); logRetentionPanel.setProps({}); // rerenders/updates component expect(switchIsChecked).toEqual(true);

…h/components/settings/log_retention/messaging/determine_tooltip_content.test.ts Co-authored-by: Constance <constancecchen@users.noreply.github.com>

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

JasonStoltz · 2020-11-09T20:19:20Z

@constancecchen I think I got all of the blocking feedback. I especially appreciated the accessibility help!

cee-chen

Looks great! Thanks for the speedy changes!

kibanamachine · 2020-11-10T14:57:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 16454f3

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`enterpriseSearch`	453	460	+7

Async chunks

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

id	before	after	diff
`enterpriseSearch`	664.0KB	678.8KB	+14.8KB

History

💚 Build #86926 succeeded 13b5697
💚 Build #86875 succeeded efdb51e
💚 Build #86840 succeeded c01d68f

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

…tic#82982) * Added LogRententionPanel * i18n * fix axe failure * Update x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/messaging/determine_tooltip_content.test.ts Co-authored-by: Constance <constancecchen@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Constance <constancecchen@users.noreply.github.com> * PR Feeback: interpolation * Apply suggestions from code review Co-authored-by: Constance <constancecchen@users.noreply.github.com> Co-authored-by: Constance <constancecchen@users.noreply.github.com>

…kibana into bootstrap-node-details-overlay * 'bootstrap-node-details-overlay' of github.com:phillipb/kibana: (49 commits) [Security Solution] Fix DNS Network table query (elastic#82778) [Workplace Search] Consolidate groups routes (elastic#83015) Adds cloud links to user menu (elastic#82803) [Security Solution][Detections] - follow up cleanup on auto refresh rules (elastic#83023) [App Search] Added the log retention panel to the Settings page (elastic#82982) [Maps] show icon when layer is filtered by time and allow layers to ignore global time range (elastic#83006) [DOCS] Consolidates drilldown pages (elastic#82081) [Maps] add on-prem EMS config (elastic#82525) migrate i18n mixin to KP (elastic#81799) [bundle optimization] fix imports of react-use lib (elastic#82847) [Discover] Add metric on adding filter (elastic#82961) [Lens] Performance refactoring for indexpattern fast lookup and Operation support matrix computation (elastic#82829) skip flaky suite (elastic#82804) Fix SO query for searching across spaces (elastic#83025) renaming built-in alerts to Stack Alerts (elastic#82873) [TSVB] Disable using top_hits in pipeline aggregations (elastic#82278) [Visualizations] Remove kui usage (elastic#82810) [Visualizations] Make the icon buttons labels more descriptive (elastic#82585) [Lens] Do not reset formatting when switching between custom ranges and auto histogram (elastic#82694) Fix ilm navigation (elastic#81664) ...

…na into alerts/stack-alerts-public * 'alerts/stack-alerts-public' of github.com:gmmorris/kibana: [Security Solution] Fix DNS Network table query (elastic#82778) [Workplace Search] Consolidate groups routes (elastic#83015) Adds cloud links to user menu (elastic#82803) [Security Solution][Detections] - follow up cleanup on auto refresh rules (elastic#83023) [App Search] Added the log retention panel to the Settings page (elastic#82982) [Maps] show icon when layer is filtered by time and allow layers to ignore global time range (elastic#83006) [DOCS] Consolidates drilldown pages (elastic#82081) [Maps] add on-prem EMS config (elastic#82525) migrate i18n mixin to KP (elastic#81799) [bundle optimization] fix imports of react-use lib (elastic#82847) [Discover] Add metric on adding filter (elastic#82961) [Lens] Performance refactoring for indexpattern fast lookup and Operation support matrix computation (elastic#82829) skip flaky suite (elastic#82804) Fix SO query for searching across spaces (elastic#83025) renaming built-in alerts to Stack Alerts (elastic#82873) [TSVB] Disable using top_hits in pipeline aggregations (elastic#82278) [Visualizations] Remove kui usage (elastic#82810) [Visualizations] Make the icon buttons labels more descriptive (elastic#82585) [Lens] Do not reset formatting when switching between custom ranges and auto histogram (elastic#82694) :

…) (#83068)

JasonStoltz added 2 commits November 9, 2020 11:24

Added LogRententionPanel

d167e3b

i18n

c01d68f

JasonStoltz requested a review from a team November 9, 2020 17:07

JasonStoltz changed the title ~~Added LogRententionPanel~~ [App Search] Added the log retentional panel to the Settings page Nov 9, 2020

JasonStoltz added 7.11.0 release_note:skip Skip the PR/issue when compiling release notes labels Nov 9, 2020

fix axe failure

efdb51e

JasonStoltz added the v7.11.0 label Nov 9, 2020

JasonStoltz changed the title ~~[App Search] Added the log retentional panel to the Settings page~~ [App Search] Added the log retention panel to the Settings page Nov 9, 2020

JasonStoltz removed the 7.11.0 label Nov 9, 2020

cee-chen reviewed Nov 9, 2020

View reviewed changes

JasonStoltz and others added 4 commits November 9, 2020 15:09

Update x-pack/plugins/enterprise_search/public/applications/app_searc…

bddc519

…h/components/settings/log_retention/messaging/determine_tooltip_content.test.ts Co-authored-by: Constance <constancecchen@users.noreply.github.com>

Apply suggestions from code review

7db5b62

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

PR Feeback: interpolation

406a04d

Apply suggestions from code review

13b5697

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

JasonStoltz requested a review from cee-chen November 9, 2020 20:18

cee-chen approved these changes Nov 9, 2020

View reviewed changes

Merge branch 'master' into log-retention-ui

16454f3

JasonStoltz mentioned this pull request Nov 10, 2020

[App Search] Added the log retention confirmation modal to the Settings page #83009

Merged

7 tasks

JasonStoltz merged commit 291c34c into elastic:master Nov 10, 2020

JasonStoltz deleted the log-retention-ui branch November 10, 2020 15:26

JasonStoltz mentioned this pull request Nov 10, 2020

[7.x] [App Search] Added the log retention panel to the Settings page (#82982) #83068

Merged

JasonStoltz added a commit that referenced this pull request Nov 10, 2020

[App Search] Added the log retention panel to the Settings page (#82982…

e9867e0

…) (#83068)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Search] Added the log retention panel to the Settings page #82982

[App Search] Added the log retention panel to the Settings page #82982

JasonStoltz commented Nov 9, 2020 •

edited

Loading

cee-chen left a comment

cee-chen Nov 9, 2020

JasonStoltz Nov 9, 2020

cee-chen Nov 9, 2020

JasonStoltz Nov 9, 2020

cee-chen Nov 9, 2020

cee-chen Nov 9, 2020

JasonStoltz Nov 9, 2020

JasonStoltz commented Nov 9, 2020 •

edited

Loading

cee-chen left a comment

kibanamachine commented Nov 10, 2020

[App Search] Added the log retention panel to the Settings page #82982

[App Search] Added the log retention panel to the Settings page #82982

Conversation

JasonStoltz commented Nov 9, 2020 • edited Loading

Summary

Checklist

For maintainers

cee-chen left a comment

Choose a reason for hiding this comment

cee-chen Nov 9, 2020

Choose a reason for hiding this comment

JasonStoltz Nov 9, 2020

Choose a reason for hiding this comment

cee-chen Nov 9, 2020

Choose a reason for hiding this comment

JasonStoltz Nov 9, 2020

Choose a reason for hiding this comment

cee-chen Nov 9, 2020

Choose a reason for hiding this comment

cee-chen Nov 9, 2020

Choose a reason for hiding this comment

JasonStoltz Nov 9, 2020

Choose a reason for hiding this comment

JasonStoltz commented Nov 9, 2020 • edited Loading

cee-chen left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 10, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

JasonStoltz commented Nov 9, 2020 •

edited

Loading

JasonStoltz commented Nov 9, 2020 •

edited

Loading