[Fleet] disable upgrade action when package does not have upgrade available #111510

hop-dev · 2021-09-08T10:51:30Z

Summary

The upgrade package policy action was not disabled for the latest version of a package.

I had missed this when solving a previous issue in this area because the hasUpgrade logic was duplicated in this component, I have now consolidated this logic.

Checklist

elasticmachine · 2021-09-08T10:51:32Z

Pinging @elastic/fleet (Team:Fleet)

hop-dev · 2021-09-08T10:52:36Z

...t/public/applications/integrations/sections/epm/screens/detail/policies/package_policies.tsx

      const hasUpgrade =
        !!updatableIntegrationRecord &&
        updatableIntegrationRecord.policiesToUpgrade.some(
-          ({ id }) => id === packagePolicy.policy_id
+          ({ pkgPolicyId }) => pkgPolicyId === packagePolicy.id


This is line is the fix to disable the button

So policy_id was an incorrect field?

Yes, updatableIntegration.policiesToUpgrade contains information about the agent policy (Default policy in this case) and the package policy (which has an upgrade available), where id is the ID of the agent policy and pkgPolicyId is the ID of the package policy.

Previously we were incorrectly comparing the agent policy IDs so every package policy in an agent policy would show as needing to upgrade. I have changed it so that we compare the package policy ID so that only the out of date packages show as needing an upgrade.

hop-dev · 2021-09-08T10:53:23Z

...t/public/applications/integrations/sections/epm/screens/detail/policies/package_policies.tsx

-            packagePolicy.package?.name ?? ''
-          );
-
-          const hasUpgrade =


InMemoryPackagePolicy has a hasUpgrade property so we don't need to duplicate the logic here

nchaulet

🚀 Do you think it will make sense to write a test here? (something like rendering the menu with different scenarios)

hop-dev · 2021-09-10T10:58:21Z

@elasticmachine merge upstream

…-disabled' into bugfix-111318-upgrade-button-not-disabled

hop-dev · 2021-09-10T16:07:22Z

x-pack/plugins/fleet/public/components/package_policy_actions_menu.test.tsx

+  };
+}
+
+test('Should disable upgrade button if package does not have upgrade', async () => {


@nchaulet these test were much simpler to write :D

hop-dev · 2021-09-10T16:08:13Z

x-pack/plugins/fleet/public/components/package_policy_actions_menu.tsx

+  viewDataStep,
+  showAddAgent,
+  upgradePackagePolicyHref,
+  isOpen = false,


I have added this isOpen prop to aid testing, I could've fired a click event but I didn't think it was unreasonable for the component to have this prop?

@hop-dev nitpick what do you think of naming this defaultIsOpen as the way I saw it it's the default value for the state correct?

hop-dev · 2021-09-10T16:08:37Z

x-pack/plugins/fleet/public/components/package_policy_actions_menu.tsx

@@ -84,6 +92,7 @@ export const PackagePolicyActionsMenu: React.FunctionComponent<{
      disabled={!packagePolicy.hasUpgrade}
      icon="refresh"
      href={upgradePackagePolicyHref}
+      key="packagePolicyUpgrade"


a warning was being logged because this menu item didn't have a key

nchaulet

🚀

hop-dev · 2021-09-13T08:57:16Z

@elasticmachine merge upstream

kibanamachine · 2021-09-13T11:15:46Z

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id	before	after	diff
`fleet`	1020.5KB	1020.1KB	-404.0B

History

💚 Build #152497 succeeded 8cd4ac8
💚 Build #151770 succeeded 42f39a3

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

cc @hop-dev

…ilable (elastic#111510) * fix: consolidate hasUpgrade logic * tidy: move type defs to top of file * test: add unit tests for actions menu * tidy: PR feedback Co-authored-by: Kibana Machine <[email protected]>

kibanamachine · 2021-09-13T11:18:38Z

💚 Backport successful

Status	Branch	Result
✅	7.15
✅	7.x

The backport PRs will be merged automatically after passing CI.

…ilable (#111510) (#111940) * fix: consolidate hasUpgrade logic * tidy: move type defs to top of file * test: add unit tests for actions menu * tidy: PR feedback Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Mark Hopkin <[email protected]>

…ade available (#111510) (#111939) * [Fleet] disable upgrade action when package does not have upgrade available (#111510) * fix: consolidate hasUpgrade logic * tidy: move type defs to top of file * test: add unit tests for actions menu * tidy: PR feedback Co-authored-by: Kibana Machine <[email protected]> * test: fix tests to match 7.15 UI Co-authored-by: Mark Hopkin <[email protected]>

amolnater-qasource · 2021-09-14T10:49:52Z

Hi @EricDavisX
We are validating this PR in lieu of reported issue at #111318
We have revalidated this on 7.15.0 Snapshot and found it fixed now.

Observations:

Upgrade package policy button is greyed out on Upgraded integration under Integration>Policies page.

Screenshot:

Build details:
Build: 44016
Commit: 6e24618

Thanks

fix: consolidate hasUpgrade logic

8f3faf1

hop-dev 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 auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Sep 8, 2021

hop-dev self-assigned this Sep 8, 2021

hop-dev requested a review from a team as a code owner September 8, 2021 10:51

hop-dev commented Sep 8, 2021

View reviewed changes

tidy: move type defs to top of file

42f39a3

nchaulet approved these changes Sep 8, 2021

View reviewed changes

jen-huang added the v7.15.0 label Sep 8, 2021

Merge branch 'master' into bugfix-111318-upgrade-button-not-disabled

8cd4ac8

hop-dev closed this Sep 10, 2021

hop-dev reopened this Sep 10, 2021

hop-dev added 2 commits September 10, 2021 17:04

test: add unit tests for actions menu

d003713

Merge remote-tracking branch 'origin/bugfix-111318-upgrade-button-not…

fb12f1c

…-disabled' into bugfix-111318-upgrade-button-not-disabled

hop-dev commented Sep 10, 2021

View reviewed changes

hop-dev requested a review from nchaulet September 10, 2021 16:08

nchaulet approved these changes Sep 10, 2021

View reviewed changes

tidy: PR feedback

2d5334f

Merge branch 'master' into bugfix-111318-upgrade-button-not-disabled

ad74c95

hop-dev enabled auto-merge (squash) September 13, 2021 08:59

hop-dev merged commit 3f82b29 into elastic:master Sep 13, 2021

kibanamachine mentioned this pull request Sep 13, 2021

[7.15] [Fleet] disable upgrade action when package does not have upgrade available (#111510) #111939

Merged

kibanamachine mentioned this pull request Sep 13, 2021

[7.x] [Fleet] disable upgrade action when package does not have upgrade available (#111510) #111940

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] disable upgrade action when package does not have upgrade available #111510

[Fleet] disable upgrade action when package does not have upgrade available #111510

hop-dev commented Sep 8, 2021

elasticmachine commented Sep 8, 2021

hop-dev Sep 8, 2021

juliaElastic Sep 8, 2021

hop-dev Sep 8, 2021

hop-dev Sep 8, 2021

nchaulet left a comment

hop-dev commented Sep 10, 2021

hop-dev Sep 10, 2021

hop-dev Sep 10, 2021

nchaulet Sep 10, 2021

hop-dev Sep 10, 2021

nchaulet left a comment

hop-dev commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

amolnater-qasource commented Sep 14, 2021

[Fleet] disable upgrade action when package does not have upgrade available #111510

[Fleet] disable upgrade action when package does not have upgrade available #111510

Conversation

hop-dev commented Sep 8, 2021

Summary

Checklist

elasticmachine commented Sep 8, 2021

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

hop-dev commented Sep 10, 2021

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

hop-dev commented Sep 13, 2021

kibanamachine commented Sep 13, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Sep 13, 2021

💚 Backport successful

amolnater-qasource commented Sep 14, 2021