[Fleet] Support for showing an Integration Detail Custom (UI Extension) tab by paul-tavares · Pull Request #83805 · elastic/kibana

paul-tavares · 2020-11-19T14:54:47Z

Summary

Changes the Integration Details view to show a Custom tab if a UI Extension for the given Integration (Package) is registered
Refactor Public Plugin component in support of a UT testing rendering tool

Checklist

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

…g setup

jfsiii · 2020-11-19T15:33:36Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/content.tsx

+      return (
+        (CustomView && (
+          <ExtensionWrapper>
+            <CustomView />
+          </ExtensionWrapper>
+        )) || <Redirect to={getPath('integration_details', { pkgkey: pkgkey ?? '' })} />
+      );


Mind breaking this up multiple statements? It took me a bit to unpack this at first, and I still got it wrong :)

Also, at least by reading, "get url for integrations_details with key empty string" seems odd. e.g. /app/fleet#/integrations/detail/. I think it works in the app, but let's explicitly send the user to named/working view like 'integrations_all`

Perhaps something like:

Suggested change

return (

(CustomView && (

<ExtensionWrapper>

<CustomView />

</ExtensionWrapper>

)) || <Redirect to={getPath('integration_details', { pkgkey: pkgkey ?? '' })} />

);

const redirectUrl = pkgkey

? getPath("integration_details", { pkgkey })

: getPath("integration_all");

return CustomView ? (

<ExtensionWrapper>

<CustomView />

</ExtensionWrapper>

) : (

<Redirect to={redirectUrl} />

);

Yeah, Agreed. This is condition check because I'm trying to grab the value from the current URL. I'm actually going to change to build it manually based on Component's input props (we have what we need). I will also look to see if there is a utility function that does that from a centralized location (I seem remember such utility, but maybe it was server side).

x-pack/plugins/fleet/public/applications/fleet/index.tsx

jfsiii · 2020-11-19T15:56:30Z

x-pack/plugins/fleet/public/applications/fleet/app.tsx

+                    <UIExtensionsContext.Provider value={extensions}>
+                      <FleetStatusProvider>
+                        <IntraAppStateProvider kibanaScopedHistory={history}>
+                          <PackageInstallProvider notifications={coreStart.notifications}>


Can you note why PackageInstallProvider was moved outside Router? I realize this is still Draft and don't ask because I'm against it. Just add a note to highlight and explain the goal when it's ready for review.

I haven't thought about Router & Context recently enough to know what might be affected or potential consequences

actually - now that I look at this again (the refactored version), I don't think it matters. In order to avoid any potential issues, I will move it back inside of <Router>. Thanks @jfsiii

…integrations-policies-list

… render

…integration-custom-tab # Conflicts: # x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/index.tsx

…integration-custom-tab

paul-tavares · 2020-11-25T15:28:54Z

x-pack/plugins/fleet/public/applications/fleet/app.tsx

+import { AppMountParameters } from 'kibana/public';
+import { EuiCode, EuiEmptyPrompt, EuiErrorBoundary, EuiPanel } from '@elastic/eui';
+import { createHashHistory, History } from 'history';
+import { Router, Redirect, Route, Switch } from 'react-router-dom';


Note the subtle change from using HashRouter to using Router along with History's createHashHistory. This is so that it can support testing.

paul-tavares · 2020-11-25T15:37:27Z

x-pack/plugins/fleet/public/applications/fleet/mock/create_test_renderer.tsx

+    }),
+    render: (ui, options) => {
+      let renderResponse: RenderResult;
+      act(() => {


I was having some errors on the test runs output about react state updates being done outside of act() - I think they are coming from useRequest, but even after I added this they did not go away.

Did you try to trigger the timers after the render I know jest offer a way to do that maybe it will fix the warnings?

paul-tavares · 2020-11-25T15:44:00Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/index.test.tsx

+      }
+    });
+
+    it('should not show a custom tab', () => {


I plan to complete these, but might not be part of this PR. I would like to get this due to the major surgery done the Plugin refactoring due to continued master merge conflicts.

paul-tavares · 2020-11-25T15:45:08Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/index.tsx

+        }
+
+        // Don't show `custom` tab if a custom component is not registered
+        if (panelId === 'custom' && !showCustomTab) {


Q. Should the custom tab be visible to packages that are not installed?

I think thats ok, since the custom tab is meant to show integration specific links/info.

I agree, I don't think we should show the tab unless the integration is installed

elasticmachine · 2020-11-25T15:46:38Z

Pinging @elastic/ingest-management (Team:Ingest Management)

paul-tavares · 2020-11-25T17:18:08Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/index.test.tsx

+
+  http.get.mockImplementation(async (path) => {
+    if (typeof path === 'string') {
+      if (path === '/api/fleet/epm/packages/nginx-0.3.7') {


FYI - as I continue to work on this test file, I think I will eventually suggest that we break out these API request test "handlers" into another test utility so that they can be used with other tests.

…integration-custom-tab

kibanamachine · 2020-11-30T14:46:16Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 630f7c2

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`fleet`	440	446	+6

Async chunks

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

id	before	after	diff
`fleet`	1.1MB	1.1MB	+15.0KB

History

💚 Build #90318 succeeded 093fecb
💚 Build #90267 succeeded 8fba859
💚 Build #90252 succeeded 495fbbd
💔 Build #90100 failed 789ad25
💔 Build #90094 failed 4b31a39

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

nchaulet

A few comments nothing blocking, really cool to have a base for UI tests 🚀

nchaulet · 2020-11-30T15:00:22Z

x-pack/plugins/fleet/public/applications/fleet/index.tsx

 ) {
  ReactDOM.render(
-    <IngestManagerApp
+    <FleetApp


👍 of renaming rest of IngestManager plugin

nchaulet · 2020-11-30T15:03:33Z

x-pack/plugins/fleet/public/applications/fleet/mock/create_test_renderer.tsx

+    }),
+    render: (ui, options) => {
+      let renderResponse: RenderResult;
+      act(() => {


Did you try to trigger the timers after the render I know jest offer a way to do that maybe it will fix the warnings?

nchaulet · 2020-11-30T15:04:18Z

x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/content.tsx

+    case 'custom':
+      return CustomView ? (
+        <ExtensionWrapper>
+          <CustomView />


Should we pass some props to that custom view, like the name, version, ?

Yeah, good point @nchaulet . I can see that information being useful to Integrations (maybe even the lastestVersion as well).

I will change the signature for the Component's props and add those in in a subsequent PR. Thanks for the feedback.

kevinlog

LGTM from a functionality standpoint

* master: (25 commits) [Alerting] fixes buggy default message behaviour (elastic#84202) [Graph] Use new ES client and change license API (elastic#84398) [DOCS] Adds redirect to known plugins page (elastic#84001) Update IndexPatternSelect to get fields from indexPatternService instead of savedObject attributes (elastic#84376) Adding timestamps to created events so the sorting is stable (elastic#84515) [DOCS] Redirects for drilldown links (elastic#83846) [Fleet] Support for showing an Integration Detail Custom (UI Extension) tab (elastic#83805) [Enterprise Search] Migrate shared Schema components (elastic#84381) [Discover] Unskip date_nanos and shard links functional tests (elastic#82878) [APM] ML anomaly detection integration: Displaying anomaly job results in the Transaction duration chart is not as intended (elastic#84415) Support for painless language autocomplete within monaco (elastic#80577) [Lens] Time scale ui (elastic#83904) removing beta callouts (elastic#84510) [Lens] (Accessibility) add aria-label to chart type icon (elastic#84493) Trusted Apps signer API. (elastic#83661) increase stdout max listeners for legacy logging (elastic#84497) [APM] Service overview: Add throughput chart (elastic#84439) [Discover] Unskip main functional tests (elastic#84300) Uptime overview overhaul (elastic#83406) [APM] Adjust time formats based on the difference between start and end (elastic#84470) ...

…n) tab (#83805) (#84539) * Support for rendering a custom component in Integration Details * Refactor Fleet app initialization contexts in order to support testing setup * New test rendering helper tool * refactor Endpoint to use mock builder from Fleet

paul-tavares added 2 commits November 18, 2020 15:04

Support for rendering a custom component in Integration Details

c8c3c5d

Refactor Fleet app initialization contexts in order to support testin…

e2e51c9

…g setup

paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0 Team:Defend Workflows “EDR Workflows” sub-team of Security Solution labels Nov 19, 2020

paul-tavares self-assigned this Nov 19, 2020

jfsiii reviewed Nov 19, 2020

View reviewed changes

x-pack/plugins/fleet/public/applications/fleet/index.tsx Show resolved Hide resolved

jfsiii reviewed Nov 19, 2020

View reviewed changes

paul-tavares added 19 commits November 19, 2020 13:11

Initial implementation of test rendering helper tool

9d4afc5

adjust order of contexts to match prior implementation

82a475f

more specific implementation around custom tab component

df94c92

Plugin dependencies mocks + renaming + expose mocks for other plugins

028e0e1

refactor Endpoint to use mock builder from ingest

8bba5d1

Merge remote-tracking branch 'upstream/master' into task/fleet-82485-…

d6e106c

…integrations-policies-list

Fix plugin.ts after merge from master

88784fb

Adjustment to test renderer to use values from returned object during…

4bd5c75

… render

More fixes after merge from master

60a7ff3

Additional fixes after master merge (jen's changes)

333c10e

Initial test file for Integration details

80a5e96

Merge remote-tracking branch 'upstream/master' into task/fleet-82492-…

94f0c01

…integration-custom-tab # Conflicts: # x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/index.tsx

apply feedback from prior code review

e6e8eb9

Refactor FleetStartServices mock

d013587

Fix TestRender's history instance due to use of Hash Router

b29029a

Merge remote-tracking branch 'upstream/master' into task/fleet-82492-…

4b31a39

…integration-custom-tab

Fixed some left over types

789ad25

Fix eslint issue

ae81b29

Merge remote-tracking branch 'upstream/master' into task/fleet-82492-…

495fbbd

…integration-custom-tab

paul-tavares commented Nov 25, 2020

View reviewed changes

remove ts-ignore

8fba859

paul-tavares commented Nov 25, 2020

View reviewed changes

paul-tavares marked this pull request as ready for review November 25, 2020 15:46

paul-tavares requested a review from a team as a code owner November 25, 2020 15:46

paul-tavares requested a review from a team November 25, 2020 15:46

paul-tavares requested a review from a team as a code owner November 25, 2020 15:46

paul-tavares commented Nov 25, 2020

View reviewed changes

paul-tavares added 3 commits November 25, 2020 13:06

some more tests

b3ddc2f

Complete test cases for custom tab in integrations

093fecb

Merge remote-tracking branch 'upstream/master' into task/fleet-82492-…

630f7c2

…integration-custom-tab

nchaulet approved these changes Nov 30, 2020

View reviewed changes

kevinlog approved these changes Nov 30, 2020

View reviewed changes

paul-tavares merged commit 707dbcd into elastic:master Nov 30, 2020

paul-tavares deleted the task/fleet-82492-integration-custom-tab branch November 30, 2020 16:12

paul-tavares mentioned this pull request Nov 30, 2020

[7.x] [Fleet] Support for showing an Integration Detail Custom (UI Extension) tab (#83805) #84539

Merged

Conversation

paul-tavares commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 25, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Nov 30, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

nchaulet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinlog left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

paul-tavares commented Nov 19, 2020 •

edited

Loading