Merge branch 'main' into actity-log-settings-e2e

trento-project · Jul 8, 2024 · 124ad3d · 124ad3d
2 parents d1b0830 + 6d2730a
commit 124ad3d
Show file tree

Hide file tree

Showing 10 changed files with 164 additions and 42 deletions.
diff --git a/assets/js/common/ActivityLogsConfig/ActivityLogsConfig.jsx b/assets/js/common/ActivityLogsConfig/ActivityLogsConfig.jsx
@@ -35,17 +35,14 @@ function ActivityLogsConfig({ retentionTime, onEditClick = noop }) {
       <div>
         <h2 className="text-2xl font-bold inline-block">Activity Logs</h2>
         <span className="float-right">
-          <Button
-            className="mr-2"
-            type="primary-white-fit"
-            size="small"
-            onClick={onEditClick}
-          >
+          <Button type="primary-white-fit" size="small" onClick={onEditClick}>
             Edit Settings
           </Button>
         </span>
       </div>
-      <p className="mt-3 mb-3 text-gray-500" />
+      <p className="mt-3 mb-3 text-gray-500">
+        Configure data retention times for log entries.
+      </p>
 
       <div className="grid grid-cols-6 mt-5 items-center">
         <div className="font-bold mb-3">Retention Time</div>

diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx
@@ -7,7 +7,7 @@ import { InputNumber } from '@common/Input';
 import Select from '@common/Select';
 import Label from '@common/Label';
 
-import { getError } from '@lib/api/validationErrors';
+import { getError, getGlobalError } from '@lib/api/validationErrors';
 
 const defaultErrors = [];
 
@@ -22,16 +22,14 @@ const toRetentionTimeErrorMessage = (errors) =>
     .filter(Boolean)
     .join('; ');
 
-const toGenericErrorMessage = (errors) =>
-  // the first error of type string is considered the generic error
-  errors.find((error) => typeof error === 'string');
+const toGlobalErrorMessage = (errors) => capitalize(getGlobalError(errors));
 
 function TimeSpan({ time: initialTime, error = false, onChange = noop }) {
   const [time, setTime] = useState(initialTime);
 
   return (
-    <div className="flex  items-center space-x-2">
-      <div className="w-2/4 pb-4">
+    <div className="flex items-center space-x-2">
+      <div className="w-1/4 pt-1">
         <InputNumber
           value={time.value}
           className="!h-8"
@@ -45,7 +43,7 @@ function TimeSpan({ time: initialTime, error = false, onChange = noop }) {
           }}
         />
       </div>
-      <div className="w-2/4 ">
+      <div className="flex w-1/4">
         <Select
           optionsName=""
           options={timeUnitOptions}
@@ -90,10 +88,18 @@ function ActivityLogsSettingsModal({
   const [retentionTime, setRetentionTime] = useState(initialRetentionTime);
 
   const retentionTimeError = toRetentionTimeErrorMessage(errors);
-  const genericError = toGenericErrorMessage(errors);
+  const genericError = toGlobalErrorMessage(errors);
 
   return (
-    <Modal title="Enter Activity Logs Settings" open={open} onClose={onCancel}>
+    <Modal
+      className="!w-3/4 !max-w-3xl"
+      title="Enter Activity Logs Settings"
+      open={open}
+      onClose={onCancel}
+    >
+      <div className="text-gray-500">
+        Set the data retention times for log entries.
+      </div>
       <div className="grid grid-cols-6 my-5 gap-6">
         <Label className="col-span-2" required>
           Retention Time

diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx
@@ -64,7 +64,7 @@ export const WithCompositeFieldValidationError = {
   },
 };
 
-export const WithGenericError = {
+export const WithGlobalError = {
   args: {
     open: false,
     initialRetentionTime: { value: 1, unit: 'month' },

diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx
@@ -4,6 +4,7 @@ import { render, screen, act } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import '@testing-library/jest-dom';
 
+import { defaultGlobalError } from '@lib/api/validationErrors';
 import ActivityLogsSettingsModal from '.';
 
 const positiveInt = () => faker.number.int({ min: 1 });
@@ -125,8 +126,8 @@ describe('ActivityLogsSettingsModal component', () => {
     ${'unit error'}                        | ${[unitError]}                     | ${unitError.detail}
     ${'value and unit errors (1)'}         | ${[valueError, unitError]}         | ${valueError.detail}
     ${'value and unit errors (2)'}         | ${[valueError, unitError]}         | ${unitError.detail}
-    ${'generic error'}                     | ${['a generic error']}             | ${'a generic error'}
-    ${'generic error and value error (1)'} | ${['a generic error', valueError]} | ${'a generic error'}
+    ${'generic error'}                     | ${['a generic error']}             | ${defaultGlobalError.detail}
+    ${'generic error and value error (1)'} | ${['a generic error', valueError]} | ${defaultGlobalError.detail}
     ${'generic error and value error (2)'} | ${['a generic error', valueError]} | ${valueError.detail}
   `(
     'should display errors on $scenario',

diff --git a/assets/js/common/Tags/Tags.jsx b/assets/js/common/Tags/Tags.jsx
@@ -16,7 +16,7 @@ const tagValidationDefaultMessage = (
   </>
 );
 
-function ExistingTag({ onClick, disabled, tag }) {
+function DeleteTagButton({ onClick, disabled, tag }) {
   return (
     <span
       data-test-id={`tag-${tag}`}
@@ -102,7 +102,7 @@ function Tags({
             permitted={tagDeletionPermittedFor}
             tooltipWrap
           >
-            <ExistingTag
+            <DeleteTagButton
               tag={tag}
               onClick={() => {
                 const newTagsList = renderedTags.reduce(

diff --git a/assets/js/lib/api/validationErrors.js b/assets/js/lib/api/validationErrors.js
@@ -1,20 +1,32 @@
 import { flow, first } from 'lodash';
-import { get, filter } from 'lodash/fp';
+import { get, filter, map } from 'lodash/fp';
 
-export const hasError = (keyword, errors) =>
-  errors.some((error) => {
-    const pointer = get(['source', 'pointer'], error);
+const selectField = (keyword) => (error) => {
+  const pointer = get(['source', 'pointer'], error);
 
-    return pointer === `/${keyword}`;
-  });
+  return pointer === `/${keyword}`;
+};
+
+export const hasError = (keyword, errors) => errors.some(selectField(keyword));
 
 export const getError = (keyword, errors) =>
-  flow([
-    filter((error) => {
-      const pointer = get(['source', 'pointer'], error);
+  flow([filter(selectField(keyword)), first, get('detail')])(errors);
 
-      return pointer === `/${keyword}`;
-    }),
+export const defaultGlobalError = {
+  title: 'Unexpected error',
+  detail: 'Something went wrong.',
+};
+
+export const getGlobalError = (errors) =>
+  flow([
+    filter(
+      (error) => !(typeof error === 'object' && error && 'source' in error)
+    ),
+    map((error) =>
+      typeof error === 'object' && error && 'detail' in error
+        ? error
+        : defaultGlobalError
+    ),
     first,
     get('detail'),
   ])(errors);
diff --git a/assets/js/lib/api/validationErrors.test.js b/assets/js/lib/api/validationErrors.test.js
@@ -1,4 +1,9 @@
-import { hasError, getError } from './validationErrors';
+import {
+  hasError,
+  getError,
+  getGlobalError,
+  defaultGlobalError,
+} from './validationErrors';
 
 describe('hasError', () => {
   it('should tell that a list contains an error about a specific field', () => {
@@ -91,3 +96,58 @@ describe('getError', () => {
     expect(getError('url', errors)).toBe(undefined);
   });
 });
+
+describe('getGlobalError', () => {
+  it('should return the first global error', () => {
+    const errors = [
+      {
+        detail: 'a detail',
+        title: 'a title',
+      },
+      {
+        detail: 'another detail',
+        source: { pointer: '/some_field' },
+        title: 'another title',
+      },
+      {
+        detail: 'do not return this detail',
+        title: 'do not return this title',
+      },
+    ];
+
+    expect(getGlobalError(errors)).toBe('a detail');
+  });
+
+  it('should return undefined when there is no global error', () => {
+    const errors = [
+      {
+        detail: "can't be blank",
+        source: { pointer: '/some_value' },
+        title: 'Invalid value',
+      },
+    ];
+
+    expect(getGlobalError(errors)).not.toBeDefined();
+  });
+
+  it('should return undefined when no error', () => {
+    const errors = [];
+
+    expect(getGlobalError(errors)).not.toBeDefined();
+  });
+
+  it.each`
+    input
+    ${{ malformed: true }}
+    ${undefined}
+    ${null}
+    ${'string'}
+    ${1234 /* number */}
+    ${[12, 34] /* array */}
+  `(
+    'should return the default error if the received error is malformed',
+    ({ input }) => {
+      expect(getGlobalError([input])).toBe(defaultGlobalError.detail);
+    }
+  );
+});
diff --git a/assets/js/pages/AdvisoryDetails/AdvisoryDetails.stories.jsx b/assets/js/pages/AdvisoryDetails/AdvisoryDetails.stories.jsx
@@ -13,15 +13,21 @@ export const Default = {
   args: {
     advisoryName: 'SUSE-15-SP4-2023-3369',
     errata: {
-      issue_date: Date.now(),
-      update_date: Date.now(),
-      synopsis: 'I think my Geekos ate my quiche 🦎🦎',
-      advisory_status: 'stable',
-      type: 'security_advisory',
-      description: `My Geekos really love the cakes I order from the crab bakery.
+      errata_details: {
+        issue_date: Date.now(),
+        update_date: Date.now(),
+        synopsis: 'I think my Geekos ate my quiche 🦎🦎',
+        advisory_status: 'stable',
+        type: 'security_advisory',
+        description: `My Geekos really love the cakes I order from the crab bakery.
 Yesterday, I left before the post arrived. Normally, the post just delivers my packages the next day.
 However, the post didn't come by today, and I am starting to wonder, if my Geekos ate my quiche. AITA? 😟`,
-      reboot_suggested: true,
+        reboot_suggested: true,
+      },
+      fixes: {
+        4815162342: 'Geekos unexpectedly eating quiches',
+      },
+      cves: ['CVE-2024-35938'],
     },
     packages: undefined,
     affectsPackageMaintanaceStack: false,

diff --git a/assets/js/state/sagas/activityLogsSettings.js b/assets/js/state/sagas/activityLogsSettings.js
@@ -11,6 +11,7 @@ import {
   setEditingActivityLogsSettings,
   setNetworkError,
 } from '@state/activityLogsSettings';
+import { defaultGlobalError } from '@lib/api/validationErrors';
 
 export function* fetchActivityLogsSettings() {
   yield put(startLoadingActivityLogsSettings());
@@ -34,7 +35,7 @@ export function* updateActivityLogsSettings({ payload }) {
     const errors = get(
       error,
       ['response', 'data', 'errors'],
-      ['An error occurred while saving the settings']
+      [defaultGlobalError]
     );
     yield put(setActivityLogsSettingsErrors(errors));
   }

diff --git a/assets/js/state/sagas/activityLogsSettings.test.js b/assets/js/state/sagas/activityLogsSettings.test.js
@@ -13,6 +13,7 @@ import {
   setNetworkError,
 } from '@state/activityLogsSettings';
 
+import { defaultGlobalError } from '@lib/api/validationErrors';
 import {
   fetchActivityLogsSettings,
   updateActivityLogsSettings,
@@ -93,6 +94,44 @@ describe('Activity Logs Settings saga', () => {
       ]);
     });
 
+    it('should have generic errors on update (receiving empty body)', async () => {
+      const axiosMock = new MockAdapter(networkClient);
+      const payload = activityLogsSettingsFactory.build();
+
+      axiosMock.onPut('/settings/activity_log', payload).reply(500);
+
+      const dispatched = await recordSaga(updateActivityLogsSettings, {
+        payload,
+      });
+
+      expect(dispatched).toEqual([
+        startLoadingActivityLogsSettings(),
+        setActivityLogsSettingsErrors([defaultGlobalError]),
+      ]);
+    });
+
+    it('should have generic errors on update', async () => {
+      const axiosMock = new MockAdapter(networkClient);
+      const payload = activityLogsSettingsFactory.build();
+
+      axiosMock.onPut('/settings/activity_log', payload).reply(500, {
+        errors: [
+          { title: 'Internal Server Error', detail: 'Something went wrong.' },
+        ],
+      });
+
+      const dispatched = await recordSaga(updateActivityLogsSettings, {
+        payload,
+      });
+
+      expect(dispatched).toEqual([
+        startLoadingActivityLogsSettings(),
+        setActivityLogsSettingsErrors([
+          { title: 'Internal Server Error', detail: 'Something went wrong.' },
+        ]),
+      ]);
+    });
+
     it.each([403, 404, 500, 502, 504])(
       'should put a network error flag on failed saving',
       async (status) => {
@@ -107,7 +146,7 @@ describe('Activity Logs Settings saga', () => {
 
         expect(dispatched).toEqual([
           startLoadingActivityLogsSettings(),
-          setActivityLogsSettingsErrors([expect.any(String)]),
+          setActivityLogsSettingsErrors([defaultGlobalError]),
         ]);
       }
     );