Handle network error for SUSE Manager settings (#2470)

* Handle network error for SUSE Manager settings * Render loading and error cases through a container in settings * Reorder tests * Use `false` instead of `null` to indicate absence of network error * Properly handle scenario --------- Co-authored-by: Jamie Rodriguez <jamie.rodriguez@suse.com> Co-authored-by: Nelson Kopliku <nelson.kopliku@suse.com>
trento-project · Mar 27, 2024 · 1f10bf6 · 1f10bf6
1 parent 8469773
commit 1f10bf6
Show file tree

Hide file tree

Showing 9 changed files with 184 additions and 28 deletions.
diff --git a/assets/js/common/SuseManagerConfig/SuseManagerSettingsContainer.jsx b/assets/js/common/SuseManagerConfig/SuseManagerSettingsContainer.jsx
@@ -0,0 +1,39 @@
+import React from 'react';
+
+import ConnectionErrorAntenna from '@static/connection-error-antenna.svg';
+
+import LoadingBox from '@common/LoadingBox';
+import NotificationBox from '@common/NotificationBox';
+
+function SuseManagerSettingsContainer({ error, loading, children, onRetry }) {
+  if (loading) {
+    return (
+      <LoadingBox
+        className="shadow-none rounded-lg"
+        text="Loading Settings..."
+      />
+    );
+  }
+
+  if (error) {
+    return (
+      <NotificationBox
+        icon={
+          <img
+            src={ConnectionErrorAntenna}
+            className="m-auto w-48"
+            alt="Connection error"
+          />
+        }
+        title="Connection Error"
+        text="Unable to load SUSE Manager configuration. Please try reloading this section."
+        buttonText="Reload"
+        buttonOnClick={onRetry}
+      />
+    );
+  }
+
+  return children;
+}
+
+export default SuseManagerSettingsContainer;
diff --git a/assets/js/common/SuseManagerConfig/SuseManagerSettingsContainer.test.jsx b/assets/js/common/SuseManagerConfig/SuseManagerSettingsContainer.test.jsx
@@ -0,0 +1,39 @@
+import React from 'react';
+import { faker } from '@faker-js/faker';
+
+import { screen, render } from '@testing-library/react';
+import '@testing-library/jest-dom';
+
+import SuseManagerSettingsContainer from './SuseManagerSettingsContainer';
+
+describe('ChecksCatalog CatalogContainer component', () => {
+  it('should render the loading box', () => {
+    const text = faker.lorem.paragraph();
+
+    render(
+      <SuseManagerSettingsContainer>
+        <p>{text}</p>
+      </SuseManagerSettingsContainer>
+    );
+
+    expect(screen.getByText(text)).toBeVisible();
+  });
+
+  it('should render the notification box in loading state', () => {
+    render(<SuseManagerSettingsContainer loading />);
+
+    expect(screen.getByText('Loading Settings...')).toBeVisible();
+  });
+
+  it('should render the notification box with connection error', () => {
+    render(<SuseManagerSettingsContainer error />);
+
+    expect(screen.getByText('Connection Error')).toBeVisible();
+    expect(
+      screen.getByText(
+        'Unable to load SUSE Manager configuration. Please try reloading this section.'
+      )
+    ).toBeVisible();
+    expect(screen.getByRole('button')).toHaveTextContent('Reload');
+  });
+});
diff --git a/assets/js/common/SuseManagerConfig/index.js b/assets/js/common/SuseManagerConfig/index.js
@@ -1,3 +1,6 @@
 import SuseManagerConfig from './SuseManagerConfig';
+import SuseManagerSettingsContainer from './SuseManagerSettingsContainer';
 
 export default SuseManagerConfig;
+
+export { SuseManagerSettingsContainer };
diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx
@@ -9,9 +9,11 @@ import { logError } from '@lib/log';
 import { get, patch } from '@lib/network';
 import { getFromConfig } from '@lib/config';
 
-import LoadingBox from '@common/LoadingBox';
 import PageHeader from '@common/PageHeader';
 import Button from '@common/Button';
+import SuseManagerConfig, {
+  SuseManagerSettingsContainer,
+} from '@common/SuseManagerConfig';
 import SuseManagerSettingsModal from '@common/SuseManagerSettingsDialog';
 import ApiKeySettingsModal from '@common/ApiKeySettingsModal';
 
@@ -32,8 +34,6 @@ import {
 import { dismissNotification } from '@state/notifications';
 import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings';
 
-import SuseManagerConfig from '@common/SuseManagerConfig';
-
 function SettingsPage() {
   const dispatch = useDispatch();
 
@@ -84,6 +84,7 @@ function SettingsPage() {
     settings,
     loading: softwareUpdatesSettingsLoading,
     editing: editingSoftwareUpdatesSettings,
+    networkError: softwareUpdatesSettingsNetworkError,
     testingConnection: testingSoftwareUpdatesConnection,
   } = useSelector(getSoftwareUpdatesSettings);
 
@@ -253,12 +254,11 @@ function SettingsPage() {
       />
       {getFromConfig('suseManagerEnabled') && (
         <div className="py-4">
-          {softwareUpdatesSettingsLoading ? (
-            <LoadingBox
-              className="shadow-none rounded-lg"
-              text="Loading Settings..."
-            />
-          ) : (
+          <SuseManagerSettingsContainer
+            loading={softwareUpdatesSettingsLoading}
+            error={softwareUpdatesSettingsNetworkError}
+            onRetry={() => dispatch(fetchSoftwareUpdatesSettings())}
+          >
             <SuseManagerConfig
               url={settings.url}
               username={settings.username}
@@ -280,7 +280,7 @@ function SettingsPage() {
               }}
               onCancel={() => setClearingSoftwareUpdatesSettings(false)}
             />
-          )}
+          </SuseManagerSettingsContainer>
           <SuseManagerSettingsModal
             key={`${settings.url}-${settings.username}-${settings.ca_uploaded_at}-${editingSoftwareUpdatesSettings}`}
             open={editingSoftwareUpdatesSettings}

diff --git a/assets/js/state/sagas/softwareUpdatesSettings.js b/assets/js/state/sagas/softwareUpdatesSettings.js
@@ -21,6 +21,7 @@ import {
   setSoftwareUpdatesSettingsErrors,
   setEditingSoftwareUpdatesSettings,
   setTestingSoftwareUpdatesConnection,
+  setNetworkError,
 } from '@state/softwareUpdatesSettings';
 
 export function* fetchSoftwareUpdatesSettings() {
@@ -30,7 +31,11 @@ export function* fetchSoftwareUpdatesSettings() {
     const response = yield call(getSettings);
     yield put(setSoftwareUpdatesSettings(response.data));
   } catch (error) {
+    const errorCode = get(error, ['response', 'status']);
     yield put(setEmptySoftwareUpdatesSettings());
+    if (errorCode !== 401) {
+      yield put(setNetworkError(true));
+    }
   }
 }
 

diff --git a/assets/js/state/sagas/softwareUpdatesSettings.test.js b/assets/js/state/sagas/softwareUpdatesSettings.test.js
@@ -3,6 +3,7 @@ import { faker } from '@faker-js/faker';
 import { recordSaga } from '@lib/test-utils';
 
 import { networkClient } from '@lib/network';
+import { authClient, storeRefreshToken } from '@lib/auth';
 import MockAdapter from 'axios-mock-adapter';
 
 import { softwareUpdatesSettingsFactory } from '@lib/test-utils/factories/softwareUpdatesSettings';
@@ -14,6 +15,7 @@ import {
   setEmptySoftwareUpdatesSettings,
   setEditingSoftwareUpdatesSettings,
   setTestingSoftwareUpdatesConnection,
+  setNetworkError,
 } from '@state/softwareUpdatesSettings';
 
 import {
@@ -42,19 +44,46 @@ describe('Software Updates Settings saga', () => {
       ]);
     });
 
-    it('should empty software updates settings on failed fetching', async () => {
+    it('should empty software updates settings when no configured settings were found', async () => {
       const axiosMock = new MockAdapter(networkClient);
-      [404, 500].forEach(async (errorStatus) => {
-        axiosMock.onGet('/settings/suma_credentials').reply(errorStatus);
+      const mockAuthClient = new MockAdapter(authClient);
+      storeRefreshToken('refresh-token');
+
+      mockAuthClient.onPost('/api/session/refresh').reply(200, {
+        access_token: 'new_token',
+      });
+
+      axiosMock
+        .onGet('/settings/suma_credentials')
+        .replyOnce(401, {
+          error: 'unauthorized',
+        })
+        .onGet('/settings/suma_credentials')
+        .reply(401);
+
+      const dispatched = await recordSaga(fetchSoftwareUpdatesSettings);
+
+      expect(dispatched).toEqual([
+        startLoadingSoftwareUpdatesSettings(),
+        setEmptySoftwareUpdatesSettings(),
+      ]);
+    });
+
+    it.each([403, 404, 500, 502, 504])(
+      'should empty software updates settings and put a network error flag on failed fetching',
+      async (status) => {
+        const axiosMock = new MockAdapter(networkClient);
+        axiosMock.onGet('/settings/suma_credentials').reply(status);
 
         const dispatched = await recordSaga(fetchSoftwareUpdatesSettings);
 
         expect(dispatched).toEqual([
           startLoadingSoftwareUpdatesSettings(),
           setEmptySoftwareUpdatesSettings(),
+          setNetworkError(true),
         ]);
-      });
-    });
+      }
+    );
   });
 
   describe('Saving Software Updates settings', () => {

diff --git a/assets/js/state/selectors/softwareUpdatesSettings.js b/assets/js/state/selectors/softwareUpdatesSettings.js
@@ -2,11 +2,19 @@ import { createSelector } from '@reduxjs/toolkit';
 
 export const getSoftwareUpdatesSettings = createSelector(
   [({ softwareUpdatesSettings }) => softwareUpdatesSettings],
-  ({ settings, errors, loading, editing, testingConnection }) => ({
+  ({
     settings,
     errors,
     loading,
     editing,
+    networkError,
+    testingConnection,
+  }) => ({
+    settings,
+    errors,
+    loading,
+    editing,
+    networkError,
     testingConnection,
   })
 );

diff --git a/assets/js/state/softwareUpdatesSettings.js b/assets/js/state/softwareUpdatesSettings.js
@@ -12,7 +12,7 @@ const emptySettings = {
 const initialState = {
   loading: false,
   settings: emptySettings,
-  networkError: null,
+  networkError: false,
   editing: false,
   testingConnection: false,
   errors: [],
@@ -27,17 +27,17 @@ export const softwareUpdatesSettingsSlice = createSlice({
     },
     setSoftwareUpdatesSettings: (state, { payload: settings }) => {
       state.loading = false;
-      state.networkError = null;
+      state.networkError = false;
       state.settings = settings;
     },
     setEmptySoftwareUpdatesSettings: (state) => {
       state.loading = false;
-      state.networkError = null;
+      state.networkError = false;
       state.settings = emptySettings;
     },
     setSoftwareUpdatesSettingsErrors: (state, { payload: errors }) => {
       state.loading = false;
-      state.networkError = null;
+      state.networkError = false;
       state.errors = errors;
     },
     setEditingSoftwareUpdatesSettings: (state, { payload }) => {
@@ -46,6 +46,10 @@ export const softwareUpdatesSettingsSlice = createSlice({
     setTestingSoftwareUpdatesConnection: (state, { payload }) => {
       state.testingConnection = payload;
     },
+    setNetworkError: (state, { payload }) => {
+      state.loading = false;
+      state.networkError = payload;
+    },
   },
 });
 
@@ -83,6 +87,7 @@ export const {
   setSoftwareUpdatesSettingsErrors,
   setEditingSoftwareUpdatesSettings,
   setTestingSoftwareUpdatesConnection,
+  setNetworkError,
 } = softwareUpdatesSettingsSlice.actions;
 
 export default softwareUpdatesSettingsSlice.reducer;