Rework patches for packages endpoint leveraging existing errata info

assets/js/common/UpgradablePackagesList/UpgradablePackagesList.jsx

@@ -9,7 +9,6 @@ const upgradablePackagesDefault = [];
 function UpgradablePackagesList({
   upgradablePackages = upgradablePackagesDefault,
   onPatchClick = noop,
-  onLoad = noop,
 }) {
   const [sortDirection, setSortDirection] = useState('asc');
 
@@ -29,7 +28,6 @@ function UpgradablePackagesList({
   const config = {
     pagination: true,
     usePadding: false,
-    onPageChange: onLoad,
     columns: [
       {
         title: 'Installed Packages',

assets/js/lib/api/softwareUpdates.js

@@ -3,9 +3,9 @@ import { networkClient } from '@lib/network';
 export const getSoftwareUpdates = (hostID) =>
   networkClient.get(`/hosts/${hostID}/software_updates`);
 
-export const getPatchesForPackages = (packageIDs) =>
+export const getPatchesForPackages = (hostID) =>
   networkClient.get(`/software_updates/packages`, {
-    params: { package_ids: packageIDs },
+    params: { host_id: hostID },
   });
 
 export const getAdvisoryErrata = (advisoryName) =>

assets/js/pages/UpgradablePackagesPage/UpgradablePackagesPage.jsx

@@ -30,6 +30,12 @@ function UpgradablePackagesPage() {
     getUpgradablePackages(state, hostID)
   );
 
+  useEffect(() => {
+    if (upgradablePackages.length > 0) {
+      dispatch(fetchUpgradablePackagesPatches({ hostID }));
+    }
+  }, [upgradablePackages.length]);
+
   return (
     <>
       <BackButton url={`/hosts/${hostID}`}>Back to Host Details</BackButton>
@@ -39,15 +45,6 @@ function UpgradablePackagesPage() {
         onPatchClick={(advisoryID) =>
           navigate(`/hosts/${hostID}/patches/${advisoryID}`)
         }
-        onLoad={(items) => {
-          if (items.length) {
-            const packageIDs = items.map(
-              ({ to_package_id: packageID }) => packageID
-            );
-
-            dispatch(fetchUpgradablePackagesPatches({ hostID, packageIDs }));
-          }
-        }}
       />
     </>
   );

assets/js/state/sagas/softwareUpdates.js

@@ -1,5 +1,5 @@
-import { get, chunk } from 'lodash';
-import { all, put, call, takeEvery } from 'redux-saga/effects';
+import { get } from 'lodash';
+import { put, call, takeEvery } from 'redux-saga/effects';
 import {
   getSoftwareUpdates,
   getPatchesForPackages,
@@ -43,27 +43,21 @@ export function* fetchSoftwareUpdates({ payload: hostID }) {
   }
 }
 
-export function* fetchUpgradablePackagesPatches({
-  payload: { hostID, packageIDs },
-}) {
-  const chunks = chunk(packageIDs, 50);
-
-  const effects = chunks.map((packageIDsChunk) =>
-    call(getPatchesForPackages, packageIDsChunk)
-  );
-
+export function* fetchUpgradablePackagesPatches({ payload: { hostID } }) {
   try {
-    const responses = yield all(effects);
-    const patches = responses
-      .map(({ data: { patches: patchesForChunk } }) =>
-        patchesForChunk.map((patch) => ({
+    const {
+      data: { patches },
+    } = yield call(getPatchesForPackages, hostID);
+
+    yield put(
+      setPatchesForPackages({
+        hostID,
+        patches: patches.map((patch) => ({
           ...patch,
           package_id: Number(patch.package_id),
-        }))
-      )
-      .flat();
-
-    yield put(setPatchesForPackages({ hostID, patches }));
+        })),
+      })
+    );
     yield put(setSettingsConfigured());
   } catch (error) {
     const errorCode = get(error, ['response', 'status']);

assets/js/state/sagas/softwareUpdates.test.js

@@ -168,37 +168,6 @@ describe('Software Updates saga', () => {
       ]);
     });
 
-    it('chunks 600 unique package IDs', async () => {
-      const axiosMock = new MockAdapter(networkClient);
-      const hostID = faker.string.uuid();
-      const packageIDs = Array.from(new Array(160)).map(() =>
-        faker.number.int()
-      );
-      const patches = patchForPackageFactory.buildList(3);
-      const response = {
-        patches: [{ package_id: packageIDs[0], patches }],
-      };
-
-      axiosMock.onGet(`/api/v1/software_updates/packages`).reply(200, response);
-
-      const dispatched = await recordSaga(fetchUpgradablePackagesPatches, {
-        payload: { hostID, packageIDs },
-      });
-
-      expect(dispatched).toEqual([
-        setPatchesForPackages({
-          hostID,
-          patches: [
-            ...response.patches,
-            ...response.patches,
-            ...response.patches,
-            ...response.patches,
-          ],
-        }),
-        setSettingsConfigured(),
-      ]);
-    });
-
     it('should set settings not configured when 422 with relevant error message', async () => {
       const axiosMock = new MockAdapter(networkClient);
       const hostID = faker.string.uuid();

lib/trento/infrastructure/software_updates/adapter/mock_suma.ex

@@ -149,7 +149,14 @@ defmodule Trento.Infrastructure.SoftwareUpdates.MockSuma do
       {:ok,
        [
          %{
-           name: "kernel",
+           name: "elixir",
+           version: "6.9.7",
+           release: "2",
+           arch_label: "x86_64",
+           epoch: "0"
+         },
+         %{
+           name: "systemd",
            version: "6.9.7",
            release: "2",
            arch_label: "x86_64",

lib/trento/software_updates.ex

@@ -58,19 +58,37 @@ defmodule Trento.SoftwareUpdates do
              :settings_not_configured
              | :error_getting_patches
              | :max_login_retries_reached}
-  def get_packages_patches(package_ids) do
+  def get_packages_patches(host_id) do
     with {:ok, _} <- Settings.get_suse_manager_settings() do
-      result =
-        package_ids
-        |> ParallelStream.map(fn package_id ->
-          {package_id, Discovery.get_patches_for_package(package_id)}
+      {:ok, relevant_patches, upgradable_packages} = Discovery.get_discovery_result(host_id)
+
+      affected_packages_for_patches =
+        relevant_patches
+        |> ParallelStream.map(fn %{advisory_name: advisory_name} = advisory ->
+          {advisory, Discovery.get_affected_packages(advisory_name)}
         end)
         |> Enum.map(fn
-          {package_id, {:ok, patches}} -> %{package_id: package_id, patches: patches}
-          {package_id, _} -> %{package_id: package_id, patches: []}
+          {advisory, {:ok, packages}} -> Map.put(advisory, :packages, packages)
+          {advisory, _} -> Map.put(advisory, :packages, [])
         end)
 
+      result = group_patches(upgradable_packages, affected_packages_for_patches)
+
       {:ok, result}
     end
   end
+
+  defp group_patches(upgradable_packages, affected_packages_for_patches),
+    do:
+      Enum.map(upgradable_packages, fn %{to_package_id: to_package_id, name: package_name} ->
+        patches = filter_affected_packages(affected_packages_for_patches, package_name)
+
+        %{package_id: to_package_id, patches: patches}
+      end)
+
+  defp filter_affected_packages(affected_packages_for_patches, package_name),
+    do:
+      Enum.filter(affected_packages_for_patches, fn %{packages: packages} ->
+        Enum.find(packages, fn %{name: name} -> name === package_name end)
+      end)
 end

lib/trento_web/controllers/v1/suse_manager_controller.ex

@@ -56,15 +56,10 @@ defmodule TrentoWeb.V1.SUSEManagerController do
     tags: ["Platform"],
     description: "Endpoint to fetch relevant patches covered by package upgrades in SUSE Manager",
     parameters: [
-      package_ids: [
+      host_id: [
         in: :query,
         required: true,
-        type: %OpenApiSpex.Schema{
-          type: :array,
-          items: %OpenApiSpex.Schema{
-            type: :string
-          }
-        }
+        type: %OpenApiSpex.Schema{type: :string, format: :uuid}
       ]
     ],
     responses: [
@@ -74,8 +69,8 @@ defmodule TrentoWeb.V1.SUSEManagerController do
     ]
 
   @spec patches_for_packages(Plug.Conn.t(), any) :: Plug.Conn.t()
-  def patches_for_packages(conn, %{package_ids: package_ids}) do
-    with {:ok, packages_patches} <- SoftwareUpdates.get_packages_patches(package_ids) do
+  def patches_for_packages(conn, %{host_id: host_id}) do
+    with {:ok, packages_patches} <- SoftwareUpdates.get_packages_patches(host_id) do
       render(conn, %{patches: packages_patches})
     end
   end

lib/trento_web/views/v1/suse_manager_view.ex

@@ -63,7 +63,29 @@ defmodule TrentoWeb.V1.SUSEManagerView do
         to_package_id: to_package_id
       }
 
-  def render("patches_for_packages.json", %{patches: patches}), do: %{patches: patches}
+  def render("patches_for_packages.json", %{patches: patches}),
+    do: %{patches: render_many(patches, __MODULE__, "package.json", as: :package)}
+
+  def render("package.json", %{package: %{package_id: package_id, patches: patches}}) do
+    %{package_id: package_id, patches: render_many(patches, __MODULE__, "patch.json", as: :patch)}
+  end
+
+  def render("patch.json", %{
+        patch: %{
+          date: date,
+          advisory_name: advisory_name,
+          advisory_type: advisory_type,
+          advisory_synopsis: advisory_synopsis,
+          update_date: update_date
+        }
+      }),
+      do: %{
+        issue_date: date,
+        advisory: advisory_name,
+        advisory_type: advisory_type,
+        synopsis: advisory_synopsis,
+        last_modified_date: update_date
+      }
 
   def render("errata_details.json", %{
         errata_details: errata_details = %{errataFrom: errataFrom},

test/e2e/cypress/e2e/suse_manager_overviews.cy.js

@@ -59,7 +59,7 @@ context('SUSE Manager overviews', () => {
 
       cy.contains('CVEs').next().should('contain', 'SUSE-15-SP4');
 
-      cy.contains('Affected Packages').next().should('contain', 'kernel');
+      cy.contains('Affected Packages').next().should('contain', 'elixir');
       cy.contains('Affected Systems')
         .next()
         .should('contain', 'vmdrbddev01')

test/trento/software_updates_test.exs

@@ -78,30 +78,60 @@ defmodule Trento.SoftwareUpdates.SettingsTest do
     test "should return an aggregated list of packages and related patches" do
       insert_software_updates_settings()
 
-      [first_package_id, second_package_id, _] =
-        packages_ids = [Faker.UUID.v4(), Faker.UUID.v4(), Faker.UUID.v4()]
+      [first_package_id, second_package_id] =
+        [Faker.UUID.v4(), Faker.UUID.v4()]
 
-      first_patch = [build(:patch_for_package)]
-      second_patch = [build(:patch_for_package)]
+      [%{advisory_name: first_patch_name}, %{advisory_name: second_patch_name}] =
+        relevant_patches = [
+          build(:relevant_patch, id: 4182),
+          build(:relevant_patch, id: 4174)
+        ]
 
-      expect(Trento.SoftwareUpdates.Discovery.Mock, :get_patches_for_package, 3, fn
-        ^first_package_id ->
-          {:ok, first_patch}
+      upgradable_packages = [
+        build(:upgradable_package, name: "elixir", to_package_id: first_package_id),
+        build(:upgradable_package, name: "systemd", to_package_id: second_package_id)
+      ]
 
-        ^second_package_id ->
-          {:ok, second_patch}
+      affected_packages = [
+        build(:affected_package, name: "elixir"),
+        build(:affected_package, name: "systemd")
+      ]
+
+      %{host_id: host_id} =
+        insert(:software_updates_discovery_result,
+          relevant_patches: relevant_patches,
+          upgradable_packages: upgradable_packages
+        )
+
+      expect(Trento.SoftwareUpdates.Discovery.Mock, :get_affected_packages, 2, fn
+        ^first_patch_name ->
+          {:ok, affected_packages}
+
+        ^second_patch_name ->
+          {:ok, affected_packages}
 
         _ ->
           {:error, :some_error}
       end)
 
       assert {:ok,
               [
-                %{package_id: ^first_package_id, patches: ^first_patch},
-                %{package_id: ^second_package_id, patches: ^second_patch},
-                %{package_id: _, patches: []}
+                %{
+                  package_id: ^first_package_id,
+                  patches: [
+                    %{advisory_name: ^first_patch_name},
+                    %{advisory_name: ^second_patch_name}
+                  ]
+                },
+                %{
+                  package_id: ^second_package_id,
+                  patches: [
+                    %{advisory_name: ^first_patch_name},
+                    %{advisory_name: ^second_patch_name}
+                  ]
+                }
               ]} =
-               SoftwareUpdates.get_packages_patches(packages_ids)
+               SoftwareUpdates.get_packages_patches(host_id)
     end
   end