Fix flickering in Available Software Updates panel

trento-project · Jul 17, 2024 · 1d79813 · 1d79813
1 parent c9c0024
commit 1d79813
Show file tree

Hide file tree

Showing 13 changed files with 248 additions and 219 deletions.
diff --git a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx
@@ -21,16 +21,15 @@ const containerClassNames = classNames(
 function AvailableSoftwareUpdates({
   className,
   settingsConfigured = false,
-  softwareUpdatesSettingsLoading,
-  softwareUpdatesLoading,
+  loading,
   relevantPatches,
   upgradablePackages,
   tooltip,
   onBackToSettings = noop,
   onNavigateToPatches = noop,
   onNavigateToPackages = noop,
 }) {
-  if (softwareUpdatesSettingsLoading) {
+  if (loading) {
     return <Loading className={containerClassNames} />;
   }
 
@@ -60,7 +59,6 @@ function AvailableSoftwareUpdates({
         title="Relevant Patches"
         critical={gt(relevantPatches, 0)}
         tooltip={tooltip}
-        loading={softwareUpdatesLoading}
         icon={<EOS_HEALING size="xl" />}
         onNavigate={onNavigateToPatches}
       >
@@ -70,7 +68,6 @@ function AvailableSoftwareUpdates({
       <Indicator
         title="Upgradable Packages"
         tooltip={tooltip}
-        loading={softwareUpdatesLoading}
         icon={<EOS_PACKAGE_UPGRADE_OUTLINED size="xl" />}
         onNavigate={onNavigateToPackages}
       >

diff --git a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx
@@ -54,24 +54,11 @@ describe('AvailableSoftwareUpdates component', () => {
   });
 
   it('renders Software Updates Settings Loading status', () => {
-    render(
-      <AvailableSoftwareUpdates
-        settingsConfigured
-        softwareUpdatesSettingsLoading
-      />
-    );
+    render(<AvailableSoftwareUpdates settingsConfigured loading />);
 
     expect(screen.getAllByLabelText('Loading')).toHaveLength(1);
   });
 
-  it('renders Software Updates Loading status', () => {
-    render(
-      <AvailableSoftwareUpdates settingsConfigured softwareUpdatesLoading />
-    );
-
-    expect(screen.getAllByText('Loading...')).toHaveLength(2);
-  });
-
   it('renders "SUSE Manager is not configured"', () => {
     render(<AvailableSoftwareUpdates settingsConfigured={false} />);
 

diff --git a/assets/js/pages/HostDetailsPage/HostDetails.jsx b/assets/js/pages/HostDetailsPage/HostDetails.jsx
@@ -73,7 +73,6 @@ function HostDetails({
   upgradablePackages,
   softwareUpdatesLoading,
   softwareUpdatesSettingsSaved,
-  softwareUpdatesSettingsLoading,
   softwareUpdatesTooltip,
   userAbilities,
   cleanUpHost,
@@ -245,8 +244,7 @@ function HostDetails({
             relevantPatches={relevantPatches}
             upgradablePackages={upgradablePackages}
             tooltip={softwareUpdatesTooltip}
-            softwareUpdatesSettingsLoading={softwareUpdatesSettingsLoading}
-            softwareUpdatesLoading={softwareUpdatesLoading}
+            loading={softwareUpdatesLoading}
             onBackToSettings={() => navigate(`/settings`)}
             onNavigateToPatches={() => navigate(`/hosts/${hostID}/patches`)}
             onNavigateToPackages={() => navigate(`/hosts/${hostID}/packages`)}

diff --git a/assets/js/pages/HostDetailsPage/HostDetails.test.jsx b/assets/js/pages/HostDetailsPage/HostDetails.test.jsx
@@ -310,19 +310,7 @@ describe('HostDetails component', () => {
         />
       );
 
-      const relevantPatchesElement = screen
-        .getByText(/Relevant Patches/)
-        .closest('div');
-      const upgradablePackagesElement = screen
-        .getByText(/Upgradable Packages/)
-        .closest('div');
-
-      expect(
-        within(relevantPatchesElement).getByText('Loading...')
-      ).toBeVisible();
-      expect(
-        within(upgradablePackagesElement).getByText('Loading...')
-      ).toBeVisible();
+      expect(screen.getAllByLabelText('Loading')).toHaveLength(1);
     });
 
     it('should a SUMA software updates when a connection error occurred', () => {

diff --git a/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx b/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx
@@ -14,10 +14,7 @@ import { getInstancesOnHost } from '@state/selectors/sapSystem';
 import { getCatalog } from '@state/selectors/catalog';
 import { getLastExecution } from '@state/selectors/lastExecutions';
 import {
-  getSoftwareUpdatesSettingsLoading,
-  getSoftwareUpdatesSettingsSaved,
-} from '@state/selectors/softwareUpdatesSettings';
-import {
+  getSoftwareUpdatesSettingsConfigured,
   getSoftwareUpdatesLoading,
   getSoftwareUpdatesStats,
 } from '@state/selectors/softwareUpdates';
@@ -31,7 +28,6 @@ import {
 } from '@state/lastExecutions';
 
 import { deregisterHost } from '@state/hosts';
-import { fetchSoftwareUpdatesSettings } from '@state/softwareUpdatesSettings';
 import { fetchSoftwareUpdates } from '@state/softwareUpdates';
 import HostDetails from './HostDetails';
 
@@ -60,12 +56,10 @@ function HostDetailsPage() {
 
   const [exportersStatus, setExportersStatus] = useState([]);
 
-  const softwareUpdatesSettingsLoading = useSelector((state) =>
-    getSoftwareUpdatesSettingsLoading(state)
-  );
-  const softwareUpdatesConnectionSaved = useSelector((state) =>
-    getSoftwareUpdatesSettingsSaved(state)
+  const settingsConfigured = useSelector((state) =>
+    getSoftwareUpdatesSettingsConfigured(state)
   );
+
   const { numRelevantPatches, numUpgradablePackages } = useSelector((state) =>
     getSoftwareUpdatesStats(state, hostID)
   );
@@ -90,11 +84,10 @@ function HostDetailsPage() {
     );
 
   useEffect(() => {
+    dispatch(fetchSoftwareUpdates(hostID));
     getExportersStatus();
     refreshCatalog();
     dispatch(updateLastExecution(hostID));
-    dispatch(fetchSoftwareUpdates(hostID));
-    dispatch(fetchSoftwareUpdatesSettings());
   }, []);
 
   if (!host) {
@@ -125,8 +118,7 @@ function HostDetailsPage() {
       suseManagerEnabled={suseManagerEnabled}
       relevantPatches={numRelevantPatches}
       upgradablePackages={numUpgradablePackages}
-      softwareUpdatesSettingsSaved={softwareUpdatesConnectionSaved}
-      softwareUpdatesSettingsLoading={softwareUpdatesSettingsLoading}
+      softwareUpdatesSettingsSaved={settingsConfigured}
       softwareUpdatesLoading={softwareUpdatesLoading}
       softwareUpdatesTooltip={
         numRelevantPatches === undefined && numUpgradablePackages === undefined

diff --git a/assets/js/state/sagas/softwareUpdates.js b/assets/js/state/sagas/softwareUpdates.js
@@ -8,6 +8,8 @@ import {
 import {
   FETCH_UPGRADABLE_PACKAGES_PATCHES,
   FETCH_SOFTWARE_UPDATES,
+  setSettingsConfigured,
+  setSettingsNotConfigured,
   startLoadingSoftwareUpdates,
   setSoftwareUpdates,
   setEmptySoftwareUpdates,
@@ -21,9 +23,16 @@ export function* fetchSoftwareUpdates({ payload: hostID }) {
   try {
     const response = yield call(getSoftwareUpdates, hostID);
     yield put(setSoftwareUpdates({ hostID, ...response.data }));
+    yield put(setSettingsConfigured());
   } catch (error) {
     yield put(setEmptySoftwareUpdates({ hostID }));
 
+    const errorCode = get(error, ['response', 'status']);
+
+    if (errorCode === 403) {
+      yield put(setSettingsNotConfigured());
+    }
+
     const errors = get(error, ['response', 'data'], []);
     yield put(setSoftwareUpdatesErrors({ hostID, errors }));
   }
@@ -37,7 +46,13 @@ export function* fetchUpgradablePackagesPatches({
       data: { patches },
     } = yield call(getPatchesForPackages, packageIDs);
     yield put(setPatchesForPackages({ hostID, patches }));
+    yield put(setSettingsConfigured());
   } catch (error) {
+    const errorCode = get(error, ['response', 'status']);
+
+    if (errorCode === 403) {
+      yield put(setSettingsNotConfigured());
+    }
     yield put(setPatchesForPackages({ hostID, patches: [] }));
   }
 }

diff --git a/assets/js/state/sagas/softwareUpdates.test.js b/assets/js/state/sagas/softwareUpdates.test.js
@@ -7,6 +7,8 @@ import { patchForPackageFactory } from '@lib/test-utils/factories';
 import { networkClient } from '@lib/network';
 
 import {
+  setSettingsConfigured,
+  setSettingsNotConfigured,
   startLoadingSoftwareUpdates,
   setSoftwareUpdates,
   setSoftwareUpdatesErrors,
@@ -64,6 +66,7 @@ describe('Software Updates saga', () => {
       expect(dispatched).toEqual([
         startLoadingSoftwareUpdates({ hostID }),
         setSoftwareUpdates({ hostID, ...response }),
+        setSettingsConfigured(),
       ]);
     });
 
@@ -91,6 +94,32 @@ describe('Software Updates saga', () => {
         ]);
       }
     );
+
+    it('should set settings not configured when 403', async () => {
+      const axiosMock = new MockAdapter(networkClient);
+      const hostID = faker.string.uuid();
+
+      const errorBody = {
+        errors: [
+          { title: 'Forbidden', detail: 'SUSE Manager authentication error.' },
+        ],
+      };
+
+      axiosMock
+        .onGet(`/api/v1/hosts/${hostID}/software_updates`)
+        .reply(403, errorBody);
+
+      const dispatched = await recordSaga(fetchSoftwareUpdates, {
+        payload: hostID,
+      });
+
+      expect(dispatched).toEqual([
+        startLoadingSoftwareUpdates({ hostID }),
+        setEmptySoftwareUpdates({ hostID }),
+        setSettingsNotConfigured(),
+        setSoftwareUpdatesErrors({ hostID, errors: errorBody }),
+      ]);
+    });
   });
 
   describe('Fetching patches for packages', () => {
@@ -114,6 +143,33 @@ describe('Software Updates saga', () => {
 
       expect(dispatched).toEqual([
         setPatchesForPackages({ hostID, patches: response.patches }),
+        setSettingsConfigured(),
+      ]);
+    });
+
+    it('should set settings not configured when 403', async () => {
+      const axiosMock = new MockAdapter(networkClient);
+      const hostID = faker.string.uuid();
+
+      const errorBody = {
+        errors: [
+          { title: 'Forbidden', detail: 'SUSE Manager authentication error.' },
+        ],
+      };
+
+      axiosMock
+        .onGet(`/api/v1/hosts/${hostID}/software_updates`)
+        .reply(403, errorBody);
+
+      const dispatched = await recordSaga(fetchSoftwareUpdates, {
+        payload: hostID,
+      });
+
+      expect(dispatched).toEqual([
+        startLoadingSoftwareUpdates({ hostID }),
+        setEmptySoftwareUpdates({ hostID }),
+        setSettingsNotConfigured(),
+        setSoftwareUpdatesErrors({ hostID, errors: errorBody }),
       ]);
     });
   });

diff --git a/assets/js/state/selectors/softwareUpdates.js b/assets/js/state/selectors/softwareUpdates.js
@@ -17,6 +17,11 @@ export const getSoftwareUpdatesStats = createSelector(
   })
 );
 
+export const getSoftwareUpdatesSettingsConfigured = createSelector(
+  [(state) => getSoftwareUpdates(state)],
+  (softwareUpdates) => get(softwareUpdates, ['settingsConfigured'], false)
+);
+
 export const getSoftwareUpdatesPatches = createSelector(
   [(state, id) => getSoftwareUpdatesForHost(id)(state)],
   (softwareUpdates) => get(softwareUpdates, ['relevant_patches'], [])

diff --git a/assets/js/state/softwareUpdates.js b/assets/js/state/softwareUpdates.js
@@ -2,6 +2,7 @@ import { createAction, createSlice } from '@reduxjs/toolkit';
 import { find, get, isEmpty } from 'lodash';
 
 const initialState = {
+  settingsConfigured: true,
   softwareUpdates: {},
 };
 
@@ -14,6 +15,12 @@ export const softwareUpdatesSlice = createSlice({
   name: 'softwareUpdates',
   initialState,
   reducers: {
+    setSettingsConfigured: (state) => {
+      state.settingsConfigured = true;
+    },
+    setSettingsNotConfigured: (state) => {
+      state.settingsConfigured = false;
+    },
     startLoadingSoftwareUpdates: (state, { payload: { hostID } }) => {
       state.softwareUpdates = {
         ...state.softwareUpdates,
@@ -81,6 +88,8 @@ export const fetchUpgradablePackagesPatches = createAction(
 );
 
 export const {
+  setSettingsConfigured,
+  setSettingsNotConfigured,
   startLoadingSoftwareUpdates,
   setSoftwareUpdates,
   setEmptySoftwareUpdates,

diff --git a/lib/trento/infrastructure/software_updates/suma.ex b/lib/trento/infrastructure/software_updates/suma.ex
@@ -62,19 +62,19 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
       {:ok, new_state} ->
         request
         |> do_handle(new_state)
-        |> handle_authentication_error(request)
+        |> handle_suma_authentication_error(request)
 
       error ->
         error
     end
   end
 
-  defp handle_authentication_error({:error, :authentication_error}, request) do
+  defp handle_suma_authentication_error({:error, :suma_authentication_error}, request) do
     clear()
     handle_request(request)
   end
 
-  defp handle_authentication_error(result, _), do: result
+  defp handle_suma_authentication_error(result, _), do: result
 
   defp do_handle({:get_system_id, fully_qualified_domain_name}, %State{
          url: url,