Skip to content

Commit

Permalink
Fix flickering in Available Software Updates panel (#2789)
Browse files Browse the repository at this point in the history
  • Loading branch information
jamie-suse authored Jul 18, 2024
1 parent a89dfa2 commit cecd0f3
Show file tree
Hide file tree
Showing 13 changed files with 283 additions and 255 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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} />;
}

Expand Down Expand Up @@ -60,7 +59,6 @@ function AvailableSoftwareUpdates({
title="Relevant Patches"
critical={gt(relevantPatches, 0)}
tooltip={tooltip}
loading={softwareUpdatesLoading}
icon={<EOS_HEALING size="xl" />}
onNavigate={onNavigateToPatches}
>
Expand All @@ -70,7 +68,6 @@ function AvailableSoftwareUpdates({
<Indicator
title="Upgradable Packages"
tooltip={tooltip}
loading={softwareUpdatesLoading}
icon={<EOS_PACKAGE_UPGRADE_OUTLINED size="xl" />}
onNavigate={onNavigateToPackages}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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} />);

Expand Down
4 changes: 1 addition & 3 deletions assets/js/pages/HostDetailsPage/HostDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ function HostDetails({
upgradablePackages,
softwareUpdatesLoading,
softwareUpdatesSettingsSaved,
softwareUpdatesSettingsLoading,
softwareUpdatesTooltip,
userAbilities,
cleanUpHost,
Expand Down Expand Up @@ -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`)}
Expand Down
47 changes: 2 additions & 45 deletions assets/js/pages/HostDetailsPage/HostDetails.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { screen, within } from '@testing-library/react';
import { screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import 'intersection-observer';
import '@testing-library/jest-dom';
Expand Down Expand Up @@ -310,50 +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();
});

it('should a SUMA software updates when a connection error occurred', () => {
const relevantPatches = faker.number.int(100);
const upgradablePackages = faker.number.int(100);

renderWithRouter(
<HostDetails
agentVersion="2.0.0"
suseManagerEnabled
softwareUpdatesSettingsSaved
softwareUpdatesLoading
relevantPatches={relevantPatches}
upgradablePackages={upgradablePackages}
userAbilities={userAbilities}
/>
);

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);
});
});

Expand Down
20 changes: 6 additions & 14 deletions assets/js/pages/HostDetailsPage/HostDetailsPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';

Expand Down Expand Up @@ -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)
);
Expand All @@ -90,11 +84,10 @@ function HostDetailsPage() {
);

useEffect(() => {
dispatch(fetchSoftwareUpdates(hostID));
getExportersStatus();
refreshCatalog();
dispatch(updateLastExecution(hostID));
dispatch(fetchSoftwareUpdates(hostID));
dispatch(fetchSoftwareUpdatesSettings());
}, []);

if (!host) {
Expand Down Expand Up @@ -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
Expand Down
24 changes: 23 additions & 1 deletion assets/js/state/sagas/softwareUpdates.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
import {
FETCH_UPGRADABLE_PACKAGES_PATCHES,
FETCH_SOFTWARE_UPDATES,
setSettingsConfigured,
setSettingsNotConfigured,
startLoadingSoftwareUpdates,
setSoftwareUpdates,
setEmptySoftwareUpdates,
Expand All @@ -21,10 +23,20 @@ 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 errors = get(error, ['response', 'data'], []);
const errorCode = get(error, ['response', 'status']);
const { errors } = get(error, ['response', 'data'], []);
const suma_unauthorized = errors.some(
({ detail }) => detail === 'SUSE Manager authentication error.'
);

if (errorCode === 422 && suma_unauthorized) {
yield put(setSettingsNotConfigured());
}

yield put(setSoftwareUpdatesErrors({ hostID, errors }));
}
}
Expand All @@ -37,7 +49,17 @@ 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']);
const { errors } = get(error, ['response', 'data'], []);
const suma_unauthorized = errors.some(
({ detail }) => detail === 'SUSE Manager authentication error.'
);

if (errorCode === 422 && suma_unauthorized) {
yield put(setSettingsNotConfigured());
}
yield put(setPatchesForPackages({ hostID, patches: [] }));
}
}
Expand Down
85 changes: 82 additions & 3 deletions assets/js/state/sagas/softwareUpdates.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { patchForPackageFactory } from '@lib/test-utils/factories';
import { networkClient } from '@lib/network';

import {
setSettingsConfigured,
setSettingsNotConfigured,
startLoadingSoftwareUpdates,
setSoftwareUpdates,
setSoftwareUpdatesErrors,
Expand Down Expand Up @@ -64,12 +66,30 @@ describe('Software Updates saga', () => {
expect(dispatched).toEqual([
startLoadingSoftwareUpdates({ hostID }),
setSoftwareUpdates({ hostID, ...response }),
setSettingsConfigured(),
]);
});

it.each([
{ status: 404, body: { message: '404 Not found' } },
{ status: 500, body: { message: 'java.lang.NullPointerException' } },
{
status: 404,
body: {
errors: [
{
title: 'Not Found',
detail: 'The requested resource cannot be found.',
},
],
},
},
{
status: 500,
body: {
errors: [
{ title: 'Internal Server Error', detail: 'Something went wrong.' },
],
},
},
])(
'should empty software updates settings on failed fetching',
async ({ status, body }) => {
Expand All @@ -87,10 +107,39 @@ describe('Software Updates saga', () => {
expect(dispatched).toEqual([
startLoadingSoftwareUpdates({ hostID }),
setEmptySoftwareUpdates({ hostID }),
setSoftwareUpdatesErrors({ hostID, errors: body }),
setSoftwareUpdatesErrors({ hostID, errors: body.errors }),
]);
}
);

it('should set settings not configured when 422 with relevant error message', async () => {
const axiosMock = new MockAdapter(networkClient);
const hostID = faker.string.uuid();

const errorBody = {
errors: [
{
title: 'Unprocessable Entity',
detail: 'SUSE Manager authentication error.',
},
],
};

axiosMock
.onGet(`/api/v1/hosts/${hostID}/software_updates`)
.reply(422, errorBody);

const dispatched = await recordSaga(fetchSoftwareUpdates, {
payload: hostID,
});

expect(dispatched).toEqual([
startLoadingSoftwareUpdates({ hostID }),
setEmptySoftwareUpdates({ hostID }),
setSettingsNotConfigured(),
setSoftwareUpdatesErrors({ hostID, errors: errorBody.errors }),
]);
});
});

describe('Fetching patches for packages', () => {
Expand All @@ -114,6 +163,36 @@ describe('Software Updates saga', () => {

expect(dispatched).toEqual([
setPatchesForPackages({ hostID, 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();

const errorBody = {
errors: [
{
title: 'Unprocessable Entity',
detail: 'SUSE Manager authentication error.',
},
],
};

axiosMock
.onGet(`/api/v1/hosts/${hostID}/software_updates`)
.reply(422, errorBody);

const dispatched = await recordSaga(fetchSoftwareUpdates, {
payload: hostID,
});

expect(dispatched).toEqual([
startLoadingSoftwareUpdates({ hostID }),
setEmptySoftwareUpdates({ hostID }),
setSettingsNotConfigured(),
setSoftwareUpdatesErrors({ hostID, errors: errorBody.errors }),
]);
});
});
Expand Down
5 changes: 5 additions & 0 deletions assets/js/state/selectors/softwareUpdates.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'], [])
Expand Down
Loading

0 comments on commit cecd0f3

Please sign in to comment.