[FLEET] New Integration Policy Details page for use in Integrations section #85355

paul-tavares · 2020-12-08T22:47:10Z

Summary

Add a new Integration Policy details page to the Integrations section of Fleet which is used when a user clicks on a policy from from the Integration Policies list. Page re-uses the existing Edit Package Policy form from the Policies section.

closes #82494

Checklist

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

…s loaded from

…integration-policy-detail-page # Conflicts: # x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/index.tsx # x-pack/plugins/fleet/public/applications/fleet/sections/epm/index.tsx # x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/index.test.tsx # x-pack/plugins/fleet/public/applications/fleet/sections/epm/screens/detail/index.tsx

paul-tavares · 2020-12-09T17:42:16Z

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

+  waitForApi: () => Promise<void>;
+}
+
+const mockApiCalls = (http: MockedFleetStartServices['http']): MockedApi => {


FYI: at some point, I would like to split this concept out into a reusable test utility allows you to compose "multiple mock api handlers" and still be able to "observe" they they are done processing.

elasticmachine · 2020-12-09T17:43:02Z

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

pzl

looks good. minor nit, but nothing to hold it up

pzl · 2020-12-09T19:03:00Z

...ins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx

-        ]);
+        const { data: packagePolicyData } = await sendGetOnePackagePolicy(packagePolicyId);
+        const { data: agentPolicyData } = await sendGetOneAgentPolicy(
+          packagePolicyData?.item.policy_id ?? 'id-missing-in-package-policy'


this seems odd. Not a big fan of putting.. log-like traces into values.

you also don't technically know that that could not be a valid future policy ID.

You should check the existence of that property and value before calling sendGetOneAgentPolicy.
You're in a try already, could check it, and throw if it's missing (if that's the appropriate behavior).

if (!item.policy_id) { throw new Error("ID missing in package policy") }

Or if it's something you move past, by grabbing a default, perhaps something like

const { data: agentPolicyData } = item.policy_id ? await sendGetOneAgentPolicy() : someDefault()

Or if having the value itself is optional

const { data?: agentPolicyData } = item.policy_id ? await sendGetOneAgentPolicy() : null

something along those lines seems more intentional than sending intentionally fake data down the line to return.. who knows what. An error? A null? Accidentally a real object?

I agree with @pzl here we can probably validate that packagePolicyData is not null and the request succeed before sending this request

Also you probably want to check the error field of the response too, if a request fail we probably want to throw that error

Yeah, I meant to comment on this and why I did it.
throwing here would not be good because the entire UI will likely crash. instead, I wanted to piggy back off the error handling in place for failed API calls. If this did happen to error, I wanted to have something that can tell us (or support team) some info about where to go look.

That being said, you just pointed out that all of this is wrapped in a try {} block (why did I not realize that? 🤦‍♂️ ), so I as you suggest, doing a check and then explicitly throw new Error() will be a better approach.

Thanks for the feedback. making the change now

Thanks @nchaulet . Good point on processing each individual API call checking for errors - in the prior code both of those would be handled (use of Promise.all()) but with my change they are not. Will fix

pzl · 2020-12-09T19:07:45Z

...ins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx

+  //  if `from === 'package-edit'` then it links back to the Integration Policy List
+  const cancelUrl = useMemo((): string => {
+    if (packageInfo && policyId) {
+      return from === 'package-edit'


If we expect this to expand to more pages, perhaps a switch here so we can be explicit about the default case (if from is something unexpected, perhaps new places added down the road, where the PR forgot to add themselves here).

yeah, agreed a switch will make sense if we add more types for from. I don't see that happening at this time and when I get around to introducing the layout that the mocks call for, I hope to revert this in favor of having props that specifically allow for setting things like the cancelUrl that should be used in the form.

pzl · 2020-12-09T19:08:10Z

...ins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx

@@ -363,13 +399,21 @@ export const EditPackagePolicyPage: React.FunctionComponent = () => {
          error={
            loadingError ||
            i18n.translate('xpack.fleet.editPackagePolicy.errorLoadingDataMessage', {
-              defaultMessage: 'There was an error loading this intergration information',
+              defaultMessage: 'There was an error loading this integration information',


All credit goes to WebStorm 🎉

…integration-policy-detail-page

nchaulet

As soon this is addressed #85355 (comment) 🚀

kibanamachine · 2020-12-09T22:14:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e5f7a64

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`fleet`	450	452	+2

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.2MB	1.2MB	+3.2KB

Distributable file count

id	before	after	diff
`default`	46986	47746	+760

Page load bundle

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

id	before	after	diff
`fleet`	375.8KB	376.0KB	+193.0B

History

💚 Build #93164 succeeded 9e6a08c
💚 Build #92940 succeeded 73db874

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

* master: (53 commits) Fixing recovered instance reference bug (elastic#85412) Switch to new elasticsearch client for Visualizations (elastic#85245) Switch to new elasticsearch client for TSVB (elastic#85275) Switch to new elasticsearch client for Vega (elastic#85280) [ILM] Add shrink field to hot phase (elastic#84087) Add rolling-file appender to core logging (elastic#84735) [APM] Service overview: Dependencies table (elastic#83416) [Uptime ]Update empty message for certs list (elastic#78575) [Graph] Fix graph saved object references (elastic#85295) [APM] Create new API's to return Latency and Throughput charts (elastic#85242) [Advanced settings] Reset to default for empty strings (elastic#85137) [SECURITY SOLUTION] Bundles _source -> Fields + able to sort on multiple fields in Timeline (elastic#83761) [Fleet] Update agent listing for better status reporting (elastic#84798) [APM] enable 'sanitize_field_names' for Go (elastic#85373) Update dependency @elastic/charts to v24.4.0 (elastic#85452) Introduce external url service (elastic#81234) Deprecate disabling the security plugin (elastic#85159) [FLEET] New Integration Policy Details page for use in Integrations section (elastic#85355) [Security Solutions][Detection Engine] Fixes one liner access control with find_rules REST API chore: 🤖 remove extraPublicDirs (elastic#85454) ...

…ection (#85355) (#85487) * new UI route to show Edit Package Policy page * Package policy List items point to new Integration Policy details page * Refactor to use common service to generate pkgKey * add breadcrumb for edit policy under integrations

paul-tavares added 6 commits December 8, 2020 11:37

new UI route to show Edit Package Policy page

83a9dab

Package policy List items point to new Integration Policy details page

f91631a

Edit package policy redirects to proper section depending on where it…

0b36a2c

…s loaded from

Refactor to use common service to generate pkgKey

f64c17b

add breadcrumb for edit policy under integrations

2dc614a

adjust layout for Package Edit from integrations

73db874

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 Dec 8, 2020

paul-tavares self-assigned this Dec 8, 2020

paul-tavares added 2 commits December 9, 2020 11:26

tests for package policies integration name link

d91dc79

paul-tavares commented Dec 9, 2020

View reviewed changes

paul-tavares marked this pull request as ready for review December 9, 2020 17:43

paul-tavares requested a review from a team December 9, 2020 17:43

pzl approved these changes Dec 9, 2020

View reviewed changes

paul-tavares added 2 commits December 9, 2020 15:00

Throw instead of fake api call

ddb97c4

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

50cd6ad

…integration-policy-detail-page

nchaulet approved these changes Dec 9, 2020

View reviewed changes

paul-tavares added 2 commits December 9, 2020 15:21

Throw instead of fake api call

65d3835

fix handling the API calls

e5f7a64

paul-tavares requested a review from nchaulet December 9, 2020 20:31

nchaulet approved these changes Dec 9, 2020

View reviewed changes

paul-tavares merged commit 6c52ac8 into elastic:master Dec 9, 2020

paul-tavares deleted the task/fleet-82494-integration-policy-detail-page branch December 9, 2020 22:51

paul-tavares mentioned this pull request Dec 9, 2020

[7.x] [FLEET] New Integration Policy Details page for use in Integrations section (#85355) #85487

Merged

paul-tavares mentioned this pull request Dec 9, 2020

[Fleet] Edit Integration Policy flow returns to original page on completion (Cancel/Save) #84581

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FLEET] New Integration Policy Details page for use in Integrations section #85355

[FLEET] New Integration Policy Details page for use in Integrations section #85355

paul-tavares commented Dec 8, 2020 •

edited

Loading

paul-tavares Dec 9, 2020

elasticmachine commented Dec 9, 2020

pzl left a comment

pzl Dec 9, 2020

nchaulet Dec 9, 2020

nchaulet Dec 9, 2020

paul-tavares Dec 9, 2020

paul-tavares Dec 9, 2020

pzl Dec 9, 2020

paul-tavares Dec 9, 2020

pzl Dec 9, 2020

paul-tavares Dec 9, 2020

nchaulet left a comment

kibanamachine commented Dec 9, 2020

[FLEET] New Integration Policy Details page for use in Integrations section #85355

[FLEET] New Integration Policy Details page for use in Integrations section #85355

Conversation

paul-tavares commented Dec 8, 2020 • edited Loading

Summary

Checklist

Choose a reason for hiding this comment

elasticmachine commented Dec 9, 2020

pzl 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

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

nchaulet left a comment

Choose a reason for hiding this comment

kibanamachine commented Dec 9, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

Page load bundle

History

paul-tavares commented Dec 8, 2020 •

edited

Loading