Extend AvailableSoftwareUpdates to display arbitrary errors (#2821)

* Extend `AvailableSoftwareUpdates` to display arbitrary errors * Fix tests * Strip loading prop off of Indicator * Fix wording * Add test for SUSE Manager error messages --------- Co-authored-by: Alessio Biancalana <[email protected]>
trento-project · Jul 29, 2024 · 30ba3c7 · 30ba3c7
1 parent a1215c6
commit 30ba3c7
Show file tree

Hide file tree

Showing 11 changed files with 271 additions and 46 deletions.
diff --git a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.jsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import classNames from 'classnames';
 import { EOS_HEALING, EOS_PACKAGE_UPGRADE_OUTLINED } from 'eos-icons-react';
-import { noop, gt } from 'lodash';
+import { noop, gt, gte } from 'lodash';
 
 import Loading from './Loading';
 import SumaNotConfigured from './SumaNotConfigured';
@@ -24,6 +24,7 @@ function AvailableSoftwareUpdates({
   loading,
   relevantPatches,
   upgradablePackages,
+  errorMessage,
   tooltip,
   onBackToSettings = noop,
   onNavigateToPatches = noop,
@@ -58,21 +59,21 @@ function AvailableSoftwareUpdates({
       <Indicator
         title="Relevant Patches"
         critical={gt(relevantPatches, 0)}
+        isError={relevantPatches === undefined}
+        message={gte(relevantPatches, 0) ? relevantPatches : errorMessage}
         tooltip={tooltip}
         icon={<EOS_HEALING size="xl" />}
         onNavigate={onNavigateToPatches}
-      >
-        {relevantPatches}
-      </Indicator>
+      />
 
       <Indicator
         title="Upgradable Packages"
+        isError={upgradablePackages === undefined}
+        message={gte(upgradablePackages, 0) ? upgradablePackages : errorMessage}
         tooltip={tooltip}
         icon={<EOS_PACKAGE_UPGRADE_OUTLINED size="xl" />}
         onNavigate={onNavigateToPackages}
-      >
-        {upgradablePackages}
-      </Indicator>
+      />
     </div>
   );
 }

diff --git a/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx b/assets/js/common/AvailableSoftwareUpdates/AvailableSoftwareUpdates.test.jsx
@@ -41,15 +41,22 @@ describe('AvailableSoftwareUpdates component', () => {
     expect(screen.getAllByTestId('eos-svg-component')).toHaveLength(5);
   });
 
-  it('renders Unknown status', async () => {
+  it('renders error status', async () => {
     const user = userEvent.setup();
     const tooltip = faker.lorem.words({ min: 3, max: 5 });
-    render(<AvailableSoftwareUpdates settingsConfigured tooltip={tooltip} />);
+    const errorMessage = faker.person.jobType();
+    render(
+      <AvailableSoftwareUpdates
+        settingsConfigured
+        tooltip={tooltip}
+        errorMessage={errorMessage}
+      />
+    );
 
-    expect(screen.getAllByText('Unknown')).toHaveLength(2);
+    expect(screen.getAllByText(errorMessage)).toHaveLength(2);
     expect(screen.getAllByTestId('eos-svg-component')).toHaveLength(4);
 
-    await user.hover(screen.getAllByText('Unknown')[0]);
+    await user.hover(screen.getAllByText(errorMessage)[0]);
     expect(screen.getByText(tooltip)).toBeInTheDocument();
   });
 

diff --git a/assets/js/common/AvailableSoftwareUpdates/Indicator.jsx b/assets/js/common/AvailableSoftwareUpdates/Indicator.jsx
@@ -11,33 +11,19 @@ function Indicator({
   title,
   critical,
   tooltip,
+  message,
   icon,
-  loading,
-  children,
+  isError,
   onNavigate,
 }) {
-  const unknown = children === undefined;
-
-  if (loading) {
-    return (
-      <div className="flex flex-row items-center border border-gray-200 p-2 rounded-md grow">
-        <div className="px-2">{icon}</div>
-        <div>
-          <p className="font-bold">{title}</p>
-          <div className="text-gray-500">Loading...</div>
-        </div>
-      </div>
-    );
-  }
-
   return (
-    <Tooltip isEnabled={unknown} content={tooltip} wrap={false}>
+    <Tooltip isEnabled={isError} content={tooltip} wrap={false}>
       <div
         role="button"
         tabIndex={0}
         className={classNames(
           'flex flex-row items-center border border-gray-200 p-2 rounded-md grow',
-          { 'cursor-pointer': !unknown }
+          { 'cursor-pointer': !isError }
         )}
         onClick={onNavigate}
         onKeyDown={({ code }) => {
@@ -51,27 +37,27 @@ function Indicator({
           <p className="font-bold">{title}</p>
           <div
             className={classNames({
-              'text-green-600': !unknown,
-              'text-gray-600': unknown,
+              'text-green-600': !isError,
+              'text-gray-600': isError,
             })}
           >
-            {critical || unknown ? (
+            {critical || isError ? (
               <div>
                 <EOS_ERROR_OUTLINED
                   size="l"
                   className={`inline align-bottom${
                     critical && ' fill-red-500'
                   }`}
                 />{' '}
-                {critical && children ? children : 'Unknown'}
+                {message}
               </div>
             ) : (
-              children
+              message
             )}
           </div>
         </div>
         <div className="flex grow justify-end">
-          {!unknown && (
+          {!isError && (
             <div>
               <EOS_KEYBOARD_ARROW_RIGHT_FILLED
                 size="l"

diff --git a/assets/js/pages/HostDetailsPage/HostDetails.jsx b/assets/js/pages/HostDetailsPage/HostDetails.jsx
@@ -81,6 +81,7 @@ function HostDetails({
   upgradablePackages,
   softwareUpdatesLoading,
   softwareUpdatesSettingsSaved,
+  softwareUpdatesErrorMessage,
   softwareUpdatesTooltip,
   userAbilities,
   cleanUpHost,
@@ -251,6 +252,7 @@ function HostDetails({
             settingsConfigured={softwareUpdatesSettingsSaved}
             relevantPatches={relevantPatches}
             upgradablePackages={upgradablePackages}
+            errorMessage={softwareUpdatesErrorMessage}
             tooltip={softwareUpdatesTooltip}
             loading={softwareUpdatesLoading}
             onBackToSettings={() => navigate(`/settings`)}

diff --git a/assets/js/pages/HostDetailsPage/HostDetails.test.jsx b/assets/js/pages/HostDetailsPage/HostDetails.test.jsx
@@ -300,7 +300,7 @@ describe('HostDetails component', () => {
       );
     });
 
-    it("should display software updates as 'Unknown' when no SUMA updates data is available", () => {
+    it('should display software updates showing an error message when no SUMA updates data is available', () => {
       const relevantPatches = undefined;
       const upgradablePackages = undefined;
 
@@ -309,6 +309,7 @@ describe('HostDetails component', () => {
           agentVersion="2.0.0"
           suseManagerEnabled
           softwareUpdatesSettingsSaved
+          softwareUpdatesErrorMessage="An error message"
           relevantPatches={relevantPatches}
           upgradablePackages={upgradablePackages}
           userAbilities={userAbilities}
@@ -322,8 +323,8 @@ describe('HostDetails component', () => {
         .getByText(/Upgradable Packages/)
         .closest('div');
 
-      expect(relevantPatchesElement).toHaveTextContent('Unknown');
-      expect(upgradablePackagesElement).toHaveTextContent('Unknown');
+      expect(relevantPatchesElement).toHaveTextContent('An error message');
+      expect(upgradablePackagesElement).toHaveTextContent('An error message');
     });
 
     it('should show the summary of SUMA software updates in a loading state', () => {

diff --git a/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx b/assets/js/pages/HostDetailsPage/HostDetailsPage.jsx
@@ -17,6 +17,7 @@ import {
   getSoftwareUpdatesSettingsConfigured,
   getSoftwareUpdatesLoading,
   getSoftwareUpdatesStats,
+  getSoftwareUpdatesErrors,
 } from '@state/selectors/softwareUpdates';
 
 import { getHost, getHostSelectedChecks } from '@state/selectors/host';
@@ -34,6 +35,42 @@ import HostDetails from './HostDetails';
 const chartsEnabled = getFromConfig('chartsEnabled');
 const suseManagerEnabled = getFromConfig('suseManagerEnabled');
 
+const getSoftwareUpdatesErrorMessage = (errors) => {
+  const hostNotFoundInSUMA = errors.some(
+    ({ detail }) =>
+      detail === 'The requested resource cannot be found.' ||
+      detail === 'No system ID was found on SUSE Manager for this host.'
+  );
+
+  if (hostNotFoundInSUMA) {
+    return 'Host not found in SUSE Manager';
+  }
+
+  if (errors.length) {
+    return 'Connection to SUMA not working';
+  }
+
+  return 'Unknown';
+};
+
+const getSoftwareUpdatesErrorTooltip = (errors) => {
+  const hostNotFoundInSUMA = errors.some(
+    ({ detail }) =>
+      detail === 'The requested resource cannot be found.' ||
+      detail === 'No system ID was found on SUSE Manager for this host.'
+  );
+
+  if (hostNotFoundInSUMA) {
+    return 'Contact your SUSE Manager admin to ensure the host is managed by SUSE Manager';
+  }
+
+  if (errors.length) {
+    return 'Please review SUSE Manager settings';
+  }
+
+  return undefined;
+};
+
 function HostDetailsPage() {
   const { hostID } = useParams();
   const dispatch = useDispatch();
@@ -68,6 +105,17 @@ function HostDetailsPage() {
     getSoftwareUpdatesLoading(state, hostID)
   );
 
+  const softwareUpdatesErrors = useSelector((state) =>
+    getSoftwareUpdatesErrors(state, hostID)
+  );
+
+  const softwareUpdatesErrorMessage = getSoftwareUpdatesErrorMessage(
+    softwareUpdatesErrors
+  );
+  const softwareUpdatesTooltip = getSoftwareUpdatesErrorTooltip(
+    softwareUpdatesErrors
+  );
+
   const getExportersStatus = async () => {
     const { data } = await networkClient.get(
       `/hosts/${hostID}/exporters_status`
@@ -121,11 +169,8 @@ function HostDetailsPage() {
       upgradablePackages={numUpgradablePackages}
       softwareUpdatesSettingsSaved={settingsConfigured}
       softwareUpdatesLoading={softwareUpdatesLoading}
-      softwareUpdatesTooltip={
-        numRelevantPatches === undefined && numUpgradablePackages === undefined
-          ? 'Trento was not able to retrieve the requested data'
-          : undefined
-      }
+      softwareUpdatesErrorMessage={softwareUpdatesErrorMessage}
+      softwareUpdatesTooltip={softwareUpdatesTooltip}
       userAbilities={abilities}
       cleanUpHost={() => {
         dispatch(deregisterHost({ id: hostID, hostname: host.hostname }));