diff --git a/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.jsx b/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.jsx index 34dbb8d52d..98e8e86f9d 100644 --- a/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.jsx +++ b/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.jsx @@ -3,13 +3,14 @@ import { noop } from 'lodash'; import Button from '@common/Button'; import Table, { createStringSortingPredicate } from '@common/Table'; +import { EOS_LOADING_ANIMATED } from 'eos-icons-react'; const upgradablePackagesDefault = []; function UpgradablePackagesList({ upgradablePackages = upgradablePackagesDefault, + patchesLoading, onPatchClick = noop, - onLoad = noop, }) { const [sortDirection, setSortDirection] = useState('asc'); @@ -29,7 +30,6 @@ function UpgradablePackagesList({ const config = { pagination: true, usePadding: false, - onPageChange: onLoad, columns: [ { title: 'Installed Packages', @@ -47,22 +47,31 @@ function UpgradablePackagesList({ { title: 'Related Patches', key: 'patches', - render: (content, { to_package_id }) => ( -
- {content && - content.map(({ advisory }) => ( -
- -
- ))} -
- ), + render: (content, { to_package_id }) => { + if (patchesLoading) { + return ( +
+ +
+ ); + } + return ( +
+ {content && + content.map(({ advisory }) => ( +
+ +
+ ))} +
+ ); + }, }, ], }; diff --git a/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.stories.jsx b/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.stories.jsx index 2fcfcec8e1..1643073347 100644 --- a/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.stories.jsx +++ b/assets/js/common/UpgradablePackagesList/UpgradablePackagesList.stories.jsx @@ -16,6 +16,9 @@ export default { }, description: 'List of upgradable packages', }, + patchesLoading: { + description: 'Are patches loading?', + }, }, }; @@ -25,3 +28,11 @@ export const Default = { upgradablePackages: upgradablePackageFactory.buildList(4), }, }; + +export const PatchesLoading = { + args: { + hostname: 'Example Host', + patchesLoading: true, + upgradablePackages: upgradablePackageFactory.buildList(4), + }, +}; diff --git a/assets/js/lib/api/softwareUpdates.js b/assets/js/lib/api/softwareUpdates.js index fa5b86381c..94c70513bd 100644 --- a/assets/js/lib/api/softwareUpdates.js +++ b/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) => diff --git a/assets/js/pages/UpgradablePackagesPage/UpgradablePackages.jsx b/assets/js/pages/UpgradablePackagesPage/UpgradablePackages.jsx index 074345cbe9..6efd21823c 100644 --- a/assets/js/pages/UpgradablePackagesPage/UpgradablePackages.jsx +++ b/assets/js/pages/UpgradablePackagesPage/UpgradablePackages.jsx @@ -10,6 +10,7 @@ import { containsSubstring } from '@lib/filter'; export default function UpgradablePackages({ hostName, upgradablePackages, + patchesLoading, onPatchClick = noop, onLoad = noop, }) { @@ -41,6 +42,7 @@ export default function UpgradablePackages({ + getPatchesLoading(state, hostID) + ); + + useEffect(() => { + if (upgradablePackages.length > 0) { + dispatch(fetchUpgradablePackagesPatches({ hostID })); + } + }, [upgradablePackages.length]); + return ( <> Back to Host Details navigate(`/hosts/${hostID}/patches/${advisoryID}`) } - onLoad={(items) => { - if (items.length) { - const packageIDs = items.map( - ({ to_package_id: packageID }) => packageID - ); - - dispatch(fetchUpgradablePackagesPatches({ hostID, packageIDs })); - } - }} /> ); diff --git a/assets/js/state/sagas/softwareUpdates.js b/assets/js/state/sagas/softwareUpdates.js index 5b598d25b3..109123594c 100644 --- a/assets/js/state/sagas/softwareUpdates.js +++ b/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']); diff --git a/assets/js/state/sagas/softwareUpdates.test.js b/assets/js/state/sagas/softwareUpdates.test.js index 724f10c98c..c7f8bb51eb 100644 --- a/assets/js/state/sagas/softwareUpdates.test.js +++ b/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(); diff --git a/assets/js/state/selectors/softwareUpdates.js b/assets/js/state/selectors/softwareUpdates.js index 5837f54999..f462c9bcb6 100644 --- a/assets/js/state/selectors/softwareUpdates.js +++ b/assets/js/state/selectors/softwareUpdates.js @@ -37,6 +37,11 @@ export const getSoftwareUpdatesLoading = createSelector( (softwareUpdates) => get(softwareUpdates, ['loading'], false) ); +export const getPatchesLoading = createSelector( + [(state, id) => getSoftwareUpdatesForHost(id)(state)], + (softwareUpdates) => get(softwareUpdates, ['loadingPatches'], false) +); + export const getSoftwareUpdatesErrors = createSelector( [(state, id) => getSoftwareUpdatesForHost(id)(state)], (softwareUpdates) => get(softwareUpdates, ['errors'], []) diff --git a/assets/js/state/softwareUpdates.js b/assets/js/state/softwareUpdates.js index 9557eb42a6..5dcfbb0816 100644 --- a/assets/js/state/softwareUpdates.js +++ b/assets/js/state/softwareUpdates.js @@ -8,6 +8,7 @@ const initialState = { const initialHostState = { loading: false, + loadingPatches: false, errors: [], }; @@ -24,7 +25,7 @@ export const softwareUpdatesSlice = createSlice({ startLoadingSoftwareUpdates: (state, { payload: { hostID } }) => { state.softwareUpdates = { ...state.softwareUpdates, - [hostID]: { ...initialHostState, loading: true }, + [hostID]: { ...initialHostState, loading: true, loadingPatches: true }, }; }, setSoftwareUpdates: ( @@ -71,6 +72,10 @@ export const softwareUpdatesSlice = createSlice({ }; }); + if (state.softwareUpdates[hostID]) { + state.softwareUpdates[hostID].loadingPatches = false; + } + if (!isEmpty(packages)) { state.softwareUpdates[hostID].upgradable_packages = newPackages; } diff --git a/assets/js/state/softwareUpdates.test.js b/assets/js/state/softwareUpdates.test.js index 64cf3b6043..908fb3d9e0 100644 --- a/assets/js/state/softwareUpdates.test.js +++ b/assets/js/state/softwareUpdates.test.js @@ -18,7 +18,9 @@ describe('SoftwareUpdates reducer', () => { const action = startLoadingSoftwareUpdates({ hostID }); const expectedState = { - softwareUpdates: { [hostID]: { loading: true, errors: [] } }, + softwareUpdates: { + [hostID]: { loading: true, loadingPatches: true, errors: [] }, + }, }; expect(softwareUpdatesReducer(initialState, action)).toEqual(expectedState); @@ -34,6 +36,7 @@ describe('SoftwareUpdates reducer', () => { relevant_patches: [], upgradable_packages: [], loading: false, + loadingPatches: false, errors: [], }, [host2]: { @@ -64,6 +67,7 @@ describe('SoftwareUpdates reducer', () => { }, ], loading: false, + loadingPatches: false, errors: [], }, }, @@ -116,10 +120,12 @@ describe('SoftwareUpdates reducer', () => { relevant_patches: [], upgradable_packages: [], loading: false, + loadingPatches: false, errors: [], }, [host2]: { ...newSoftwareUpdates, + loadingPatches: false, loading: false, errors: [], }, @@ -170,7 +176,7 @@ describe('SoftwareUpdates reducer', () => { errors: [], loading: false, }, - [host2]: { errors: [], loading: false }, + [host2]: { errors: [], loading: false, loadingPatches: false }, }, }); }); @@ -230,7 +236,12 @@ describe('SoftwareUpdates reducer', () => { expect(actual).toEqual({ softwareUpdates: { - [host1]: { ...initialState[host1], loading: false, errors }, + [host1]: { + ...initialState[host1], + loading: false, + loadingPatches: false, + errors, + }, }, }); }); diff --git a/lib/trento/infrastructure/software_updates/adapter/mock_suma.ex b/lib/trento/infrastructure/software_updates/adapter/mock_suma.ex index fc31f2ee31..373990f6c6 100644 --- a/lib/trento/infrastructure/software_updates/adapter/mock_suma.ex +++ b/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", diff --git a/lib/trento/software_updates.ex b/lib/trento/software_updates.ex index 07da2a6d62..c57d10d491 100644 --- a/lib/trento/software_updates.ex +++ b/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 diff --git a/lib/trento_web/controllers/v1/suse_manager_controller.ex b/lib/trento_web/controllers/v1/suse_manager_controller.ex index 15763a97bd..cbfbe18fc8 100644 --- a/lib/trento_web/controllers/v1/suse_manager_controller.ex +++ b/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 diff --git a/lib/trento_web/views/v1/suse_manager_view.ex b/lib/trento_web/views/v1/suse_manager_view.ex index 425f9ee741..723a6be6e1 100644 --- a/lib/trento_web/views/v1/suse_manager_view.ex +++ b/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}, diff --git a/test/e2e/cypress/e2e/suse_manager_overviews.cy.js b/test/e2e/cypress/e2e/suse_manager_overviews.cy.js index 1accf80d64..e53dd25872 100644 --- a/test/e2e/cypress/e2e/suse_manager_overviews.cy.js +++ b/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') diff --git a/test/trento/software_updates_test.exs b/test/trento/software_updates_test.exs index 44a3bd89fa..3fcce9f957 100644 --- a/test/trento/software_updates_test.exs +++ b/test/trento/software_updates_test.exs @@ -78,18 +78,37 @@ 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} @@ -97,11 +116,22 @@ defmodule Trento.SoftwareUpdates.SettingsTest do 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 diff --git a/test/trento_web/controllers/v1/suse_manager_controller_test.exs b/test/trento_web/controllers/v1/suse_manager_controller_test.exs index c25a39e8a8..e4671033a5 100644 --- a/test/trento_web/controllers/v1/suse_manager_controller_test.exs +++ b/test/trento_web/controllers/v1/suse_manager_controller_test.exs @@ -92,6 +92,22 @@ defmodule TrentoWeb.V1.SUSEManagerControllerTest do test "should return relevant patches grouped by package id", %{conn: conn, api_spec: api_spec} do insert_software_updates_settings() + relevant_patches = [ + build(:relevant_patch, id: 4182), + build(:relevant_patch, id: 4174) + ] + + upgradable_packages = [ + build(:upgradable_package, name: "elixir"), + build(:upgradable_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_patches_for_package, 3, fn _ -> {:ok, build_list(10, :patch_for_package)} end) @@ -103,9 +119,7 @@ defmodule TrentoWeb.V1.SUSEManagerControllerTest do ] } = conn - |> get( - "/api/v1/software_updates/packages?package_ids[]=#{Faker.UUID.v4()}&package_ids[]=#{Faker.UUID.v4()}" - ) + |> get("/api/v1/software_updates/packages?host_id=#{host_id}") |> json_response(:ok) |> assert_schema("PatchesForPackagesResponse", api_spec) end @@ -113,7 +127,23 @@ defmodule TrentoWeb.V1.SUSEManagerControllerTest do test "should return an empty list if every call errors", %{conn: conn, api_spec: api_spec} do insert_software_updates_settings() - expect(Trento.SoftwareUpdates.Discovery.Mock, :get_patches_for_package, 3, fn _ -> + relevant_patches = [ + build(:relevant_patch, id: 4182), + build(:relevant_patch, id: 4174) + ] + + upgradable_packages = [ + build(:upgradable_package, name: "elixir"), + build(:upgradable_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, 3, fn _ -> {:error, :error_getting_patches} end) @@ -124,9 +154,7 @@ defmodule TrentoWeb.V1.SUSEManagerControllerTest do ] } = conn - |> get( - "/api/v1/software_updates/packages?package_ids[]=#{Faker.UUID.v4()}&package_ids[]=#{Faker.UUID.v4()}" - ) + |> get("/api/v1/software_updates/packages?host_id=#{host_id}") |> json_response(:ok) |> assert_schema("PatchesForPackagesResponse", api_spec) end