fix: exclude packages in Upgrade panel

.changeset/upgrades-exclude-installed-packages.md

@@ -0,0 +1,5 @@
+---
+"landscape-ui": patch
+---
+
+Don't upgrade packages that are excluded in the Packages tab of the Upgrades form

src/features/packages/hooks/usePackages.ts

@@ -18,6 +18,7 @@ import type {
   SearchUpgradesRequest,
 } from "../types";
 import { mapGroupedResultToInstancePackage } from "../helpers";
+import { toInstanceQuery } from "@/utils/_helpers";
 
 interface GetInstancePackagesParams extends ComputerPackageSearchParams {
   instance_id: number;
@@ -56,9 +57,13 @@ export interface InstancePackagesToExclude {
 }
 
 interface UpgradeInstancePackagesParams {
-  computers: InstancePackagesToExclude[];
+  computers: (InstancePackagesToExclude & {
+    excludePackageNames: string[];
+  })[];
 }
 
+const INSTALLED_PACKAGES_SEARCH_LIMIT = 500;
+
 const sanitizeFilterState = (state?: FilterState): FilterState | undefined => {
   if (!state || state === FilterState.UNSPECIFIED) {
     return undefined;
@@ -201,8 +206,59 @@ export default function usePackages() {
     AxiosError<ApiError>,
     UpgradeInstancePackagesParams
   >({
-    mutationFn: async (params) =>
-      authFetch.post("/computers/upgrade-packages", params),
+    mutationFn: async ({ computers }) => {
+      const excludedNames = [
+        ...new Set(
+          computers.flatMap(({ excludePackageNames }) => excludePackageNames),
+        ),
+      ];
+      const installedIdsByName = new Map<string, number[]>();
+
+      // excluded names must be resolved to installed versions across the selection
+      if (excludedNames.length) {
+        let offset = 0;
+        let count = Infinity;
+
+        while (offset < count) {
+          const { data } = await authFetch.post<SearchPackagesResponse>(
+            "packages:search",
+            {
+              computer_query: toInstanceQuery(computers.map(({ id }) => id)),
+              names: excludedNames.join(","),
+              installed: FilterState.TRUE,
+              limit: INSTALLED_PACKAGES_SEARCH_LIMIT,
+              offset,
+            },
+          );
+
+          for (const { id, name } of data.packages) {
+            installedIdsByName.set(name, [
+              ...(installedIdsByName.get(name) ?? []),
+              id,
+            ]);
+          }
+
+          ({ count } = data);
+          offset += INSTALLED_PACKAGES_SEARCH_LIMIT;
+        }
+      }
+
+      return authFetch.post("/computers/upgrade-packages", {
+        computers: computers.map(
+          ({ id, exclude_packages, excludePackageNames }) => ({
+            id,
+            exclude_packages: [
+              ...new Set([
+                ...exclude_packages,
+                ...excludePackageNames.flatMap(
+                  (name) => installedIdsByName.get(name) ?? [],
+                ),
+              ]),
+            ],
+          }),
+        ),
+      });
+    },
     onSuccess: async () =>
       Promise.all([
         queryClient.invalidateQueries({ queryKey: ["packages"] }),

src/features/upgrades/components/Upgrades/Upgrades.test.tsx

@@ -1,12 +1,17 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
-import { screen } from "@testing-library/react";
+import { screen, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
+import { http, HttpResponse } from "msw";
+import { API_URL } from "@/constants";
 import type { AuthContextProps } from "@/context/auth";
+import { hasUpgrades } from "@/features/instances";
 import useAuth from "@/hooks/useAuth";
+import { activities } from "@/tests/mocks/activity";
 import { instances } from "@/tests/mocks/instance";
 import { setEndpointStatus } from "@/tests/controllers/controller";
 import { expectErrorNotification } from "@/tests/helpers";
 import { renderWithProviders } from "@/tests/render";
+import server from "@/tests/server";
 import Upgrades from "./Upgrades";
 
 vi.mock("@/hooks/useAuth");
@@ -196,6 +201,238 @@ describe("Upgrades", () => {
     ).toBeInTheDocument();
   });
 
+  it("excludes packages from the packages tab by their installed package ids", async () => {
+    const user = userEvent.setup();
+    const upgradeCandidate = {
+      id: 9001,
+      name: "foo",
+      summary: "Foo",
+      version: "2.0.0",
+      computers: { count: instances.length },
+    };
+    const installedPackage = { ...upgradeCandidate, id: 101, version: "1.0.0" };
+    let installedSearchBody: unknown;
+    let upgradeBody: unknown;
+
+    server.use(
+      http.post(`${API_URL}packages\\:search-upgrades`, () =>
+        HttpResponse.json({
+          packages: [upgradeCandidate],
+          count: 1,
+          next: null,
+          prev: null,
+        }),
+      ),
+      http.post(`${API_URL}packages\\:search`, async ({ request }) => {
+        installedSearchBody = await request.json();
+        return HttpResponse.json({
+          packages: [installedPackage],
+          count: 1,
+          next: null,
+          prev: null,
+        });
+      }),
+      http.post(`${API_URL}computers/upgrade-packages`, async ({ request }) => {
+        upgradeBody = await request.json();
+        return HttpResponse.json(activities[0]);
+      }),
+    );
+
+    renderWithProviders(<Upgrades selectedInstances={instances} />);
+
+    await user.click(screen.getByRole("tab", { name: /packages/i }));
+    await user.click(
+      await screen.findByRole("checkbox", { name: /toggle foo package/i }),
+    );
+    await user.click(screen.getByRole("button", { name: "Upgrade" }));
+
+    expect(
+      await screen.findByText("You queued packages to be upgraded"),
+    ).toBeInTheDocument();
+    expect(installedSearchBody).toMatchObject({
+      names: "foo",
+      installed: "true",
+    });
+    const { computers } = upgradeBody as { computers: object[] };
+    expect(computers.length).toBeGreaterThan(0);
+    for (const computer of computers) {
+      expect(Object.keys(computer).sort()).toEqual(["exclude_packages", "id"]);
+      expect(computer).toMatchObject({
+        exclude_packages: [installedPackage.id],
+      });
+    }
+  });
+
+  it("passes instances tab exclusions through without resolving package names", async () => {
+    const user = userEvent.setup();
+    const instance = instances.find(({ alerts }) => hasUpgrades(alerts));
+    assert(instance);
+    const installedId = 777;
+    let installedSearchCalled = false;
+    let upgradeBody: unknown;
+
+    server.use(
+      http.get(`${API_URL}computers/:id/packages/search`, () =>
+        HttpResponse.json({
+          count: 1,
+          results: [
+            {
+              name: "bar",
+              summary: "Bar",
+              installed_version: "1.0.0",
+              installed_id: installedId,
+              held: false,
+              security: false,
+              installation_candidates: [
+                { id: 778, version: "2.0.0", upgrade: true, security: false },
+              ],
+            },
+          ],
+        }),
+      ),
+      http.post(`${API_URL}packages\\:search`, () => {
+        installedSearchCalled = true;
+        return HttpResponse.json({
+          packages: [],
+          count: 0,
+          next: null,
+          prev: null,
+        });
+      }),
+      http.post(`${API_URL}computers/upgrade-packages`, async ({ request }) => {
+        upgradeBody = await request.json();
+        return HttpResponse.json(activities[0]);
+      }),
+    );
+
+    renderWithProviders(<Upgrades selectedInstances={[instance]} />);
+
+    const instanceRow = (await screen.findAllByRole("row")).find(
+      ({ firstChild }) => firstChild?.textContent === instance.title,
+    );
+    assert(instanceRow);
+    await user.click(within(instanceRow).getByRole("button"));
+    await user.click(
+      await screen.findByRole("checkbox", { name: /toggle bar package/i }),
+    );
+    await user.click(screen.getByRole("button", { name: "Upgrade" }));
+
+    expect(
+      await screen.findByText("You queued packages to be upgraded"),
+    ).toBeInTheDocument();
+    expect(installedSearchCalled).toBe(false);
+    expect(upgradeBody).toEqual({
+      computers: [{ id: instance.id, exclude_packages: [installedId] }],
+    });
+  });
+
+  it("keeps an instances tab exclusion on its own instance when its id is also a packages tab upgrade", async () => {
+    const user = userEvent.setup();
+    const [instanceA, instanceB] = instances.filter(({ alerts }) =>
+      hasUpgrades(alerts),
+    );
+    assert(instanceA && instanceB);
+    const installedOnA = 1001;
+    // Upgrade candidate for instance A and the installed version on instance B.
+    const installedOnB = 2002;
+    let upgradeBody: unknown;
+
+    server.use(
+      http.post(`${API_URL}packages\\:search-upgrades`, () =>
+        HttpResponse.json({
+          packages: [
+            {
+              id: installedOnB,
+              name: "foo",
+              summary: "Foo",
+              version: "2.0.0",
+              computers: { count: 1 },
+            },
+          ],
+          count: 1,
+          next: null,
+          prev: null,
+        }),
+      ),
+      http.get(`${API_URL}computers/:id/packages/search`, () =>
+        HttpResponse.json({
+          count: 1,
+          results: [
+            {
+              name: "foo",
+              summary: "Foo",
+              installed_version: "2.0.0",
+              installed_id: installedOnB,
+              held: false,
+              security: false,
+              installation_candidates: [
+                { id: 3003, version: "3.0.0", upgrade: true, security: false },
+              ],
+            },
+          ],
+        }),
+      ),
+      http.post(`${API_URL}packages\\:search`, () =>
+        HttpResponse.json({
+          packages: [
+            {
+              id: installedOnA,
+              name: "foo",
+              summary: "Foo",
+              version: "1.0.0",
+              computers: { count: 1 },
+            },
+            {
+              id: installedOnB,
+              name: "foo",
+              summary: "Foo",
+              version: "2.0.0",
+              computers: { count: 1 },
+            },
+          ],
+          count: 2,
+          next: null,
+          prev: null,
+        }),
+      ),
+      http.post(`${API_URL}computers/upgrade-packages`, async ({ request }) => {
+        upgradeBody = await request.json();
+        return HttpResponse.json(activities[0]);
+      }),
+    );
+
+    renderWithProviders(
+      <Upgrades selectedInstances={[instanceA, instanceB]} />,
+    );
+
+    await user.click(screen.getByRole("tab", { name: /packages/i }));
+    await screen.findByRole("checkbox", { name: /toggle foo package/i });
+    await user.click(screen.getByRole("tab", { name: /instances/i }));
+
+    const instanceBRow = (await screen.findAllByRole("row")).find(
+      ({ firstChild }) => firstChild?.textContent === instanceB.title,
+    );
+    assert(instanceBRow);
+    await user.click(within(instanceBRow).getByRole("button"));
+    await user.click(
+      await screen.findByRole("checkbox", { name: /toggle foo package/i }),
+    );
+    await user.click(screen.getByRole("button", { name: "Upgrade" }));
+
+    expect(
+      await screen.findByText("You queued packages to be upgraded"),
+    ).toBeInTheDocument();
+    expect(upgradeBody).toEqual({
+      computers: [
+        { id: instanceA.id, exclude_packages: [] },
+        {
+          id: instanceB.id,
+          exclude_packages: expect.arrayContaining([installedOnB]),
+        },
+      ],
+    });
+  });
+
   it("updates excluded usn list from usns tab interaction", async () => {
     const user = userEvent.setup();
 

src/features/upgrades/components/Upgrades/Upgrades.tsx

@@ -13,10 +13,14 @@ import { pluralize } from "@/utils/_helpers";
 import { Form, Tabs } from "@canonical/react-components";
 import { useFormik } from "formik";
 import type { FC } from "react";
-import { Suspense, useState } from "react";
+import { Suspense, useRef, useState } from "react";
 import UpgradeInfo from "../UpgradeInfo";
 import { TAB_LINKS, TAB_PANELS, VALIDATION_SCHEMA } from "./constants";
-import { getInitialValues, getTabLinks } from "./helpers";
+import {
+  getExcludedPackagesWithNames,
+  getInitialValues,
+  getTabLinks,
+} from "./helpers";
 import type { UpgradesFormProps } from "./types";
 
 interface UpgradesProps {
@@ -27,6 +31,9 @@ const Upgrades: FC<UpgradesProps> = ({ selectedInstances }) => {
   const [activeTabLinkId, setActiveTabLinkId] = useState<string>(
     TAB_LINKS[0].id,
   );
+  // The packages tab lists upgrade candidates, whose ids the upgrade endpoint
+  // ignores, so their names are kept to resolve installed ids on submit.
+  const packageNamesByIdRef = useRef(new Map<number, string>());
 
   const affectedInstances = selectedInstances.filter(({ alerts }) =>
     hasUpgrades(alerts),
@@ -58,7 +65,10 @@ const Upgrades: FC<UpgradesProps> = ({ selectedInstances }) => {
         });
       } else {
         await upgradeInstancesPackages({
-          computers: values.excludedPackages,
+          computers: getExcludedPackagesWithNames(
+            values.excludedPackages,
+            packageNamesByIdRef.current,
+          ),
         });
       }
 
@@ -115,6 +125,11 @@ const Upgrades: FC<UpgradesProps> = ({ selectedInstances }) => {
                 excludedPackages={formik.values.excludedPackages}
                 instances={affectedInstances}
                 onExcludedPackagesChange={handleExcludedPackagesChange}
+                onPackagesLoad={(packages) => {
+                  for (const { id, name } of packages) {
+                    packageNamesByIdRef.current.set(id, name);
+                  }
+                }}
               />
             </Suspense>
           )}