Activity log settings UI state

assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx

@@ -7,13 +7,25 @@ import { InputNumber } from '@common/Input';
 import Select from '@common/Select';
 import Label from '@common/Label';
 
-import { hasError, getError } from '@lib/api/validationErrors';
+import { getError } from '@lib/api/validationErrors';
 
 const defaultErrors = [];
 
 const timeUnitOptions = ['day', 'week', 'month', 'year'];
 const defaultTimeUnit = timeUnitOptions[0];
 
+const toRetentionTimeErrorMessage = (errors) =>
+  [
+    capitalize(getError('retention_time/value', errors)),
+    capitalize(getError('retention_time/unit', errors)),
+  ]
+    .filter(Boolean)
+    .join('; ');
+
+const toGenericErrorMessage = (errors) =>
+  // the first error of type string is considered the generic error
+  errors.find((error) => typeof error === 'string');
+
 function TimeSpan({ time: initialTime, error = false, onChange = noop }) {
   const [time, setTime] = useState(initialTime);
 
@@ -24,7 +36,7 @@ function TimeSpan({ time: initialTime, error = false, onChange = noop }) {
           value={time.value}
           className="!h-8"
           type="number"
-          min="0"
+          min="1"
           error={error}
           onChange={(value) => {
             const newTime = { ...time, value };
@@ -50,6 +62,19 @@ function TimeSpan({ time: initialTime, error = false, onChange = noop }) {
   );
 }
 
+/**
+ * Display an error message. If no error is provided, an empty space is displayed to keep the layout stable
+ * @param {string} text The error message to display
+ * @returns {JSX.Element}
+ */
+function Error({ text }) {
+  return text ? (
+    <p className="text-red-500 mt-1">{text}</p>
+  ) : (
+    <p className="mt-1">&nbsp;</p>
+  );
+}
+
 /**
  * Modal to edit Activity Logs settings
  */
@@ -64,6 +89,9 @@ function ActivityLogsSettingsModal({
 }) {
   const [retentionTime, setRetentionTime] = useState(initialRetentionTime);
 
+  const retentionTimeError = toRetentionTimeErrorMessage(errors);
+  const genericError = toGenericErrorMessage(errors);
+
   return (
     <Modal title="Enter Activity Logs Settings" open={open} onClose={onCancel}>
       <div className="grid grid-cols-6 my-5 gap-6">
@@ -73,20 +101,13 @@ function ActivityLogsSettingsModal({
         <div className="col-span-4">
           <TimeSpan
             time={retentionTime}
-            error={hasError('retentionTime', errors)}
+            error={Boolean(retentionTimeError)}
             onChange={(time) => {
               setRetentionTime(time);
               onClearErrors();
             }}
           />
-          {hasError('retentionTime', errors) && (
-            <p
-              aria-label="retention-time-input-error"
-              className="text-red-500 mt-1"
-            >
-              {capitalize(getError('retentionTime', errors))}
-            </p>
-          )}
+          <Error text={retentionTimeError} />
         </div>
 
         <p className="col-span-6">
@@ -98,7 +119,7 @@ function ActivityLogsSettingsModal({
           disabled={loading}
           onClick={() => {
             const payload = {
-              retentionTime,
+              retention_time: retentionTime,
             };
             onSave(payload);
           }}
@@ -109,6 +130,9 @@ function ActivityLogsSettingsModal({
           Cancel
         </Button>
       </div>
+      <div className="flex flex-row w-80 space-x-2">
+        <Error text={genericError} />
+      </div>
     </Modal>
   );
 }

assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx

@@ -31,16 +31,43 @@ export const Default = {
   },
 };
 
-export const WithErrors = {
+export const WithFieldValidationError = {
   args: {
     open: false,
     initialRetentionTime: { value: 1, unit: 'month' },
     errors: [
       {
-        detail: "can't be blank",
-        source: { pointer: '/retentionTime' },
+        detail: 'must be greater than or equal to 1',
+        source: { pointer: '/retention_time/value' },
         title: 'Invalid value',
       },
     ],
   },
 };
+
+export const WithCompositeFieldValidationError = {
+  args: {
+    open: false,
+    initialRetentionTime: { value: 1, unit: 'month' },
+    errors: [
+      {
+        detail: 'must be greater than or equal to 1',
+        source: { pointer: '/retention_time/value' },
+        title: 'Invalid value',
+      },
+      {
+        detail: 'invalid time unit',
+        source: { pointer: '/retention_time/unit' },
+        title: 'Invalid unit',
+      },
+    ],
+  },
+};
+
+export const WithGenericError = {
+  args: {
+    open: false,
+    initialRetentionTime: { value: 1, unit: 'month' },
+    errors: ['any error'],
+  },
+};

assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx

@@ -4,8 +4,6 @@ import { render, screen, act } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import '@testing-library/jest-dom';
 
-import { capitalize } from 'lodash';
-
 import ActivityLogsSettingsModal from '.';
 
 const positiveInt = () => faker.number.int({ min: 1 });
@@ -83,7 +81,7 @@ describe('ActivityLogsSettingsModal component', () => {
     await user.click(screen.getByText('Save Settings'));
 
     expect(onSave).toHaveBeenCalledWith({
-      retentionTime: expectedRetentionTime,
+      retention_time: expectedRetentionTime,
     });
   });
 
@@ -106,34 +104,50 @@ describe('ActivityLogsSettingsModal component', () => {
     await user.click(screen.getByText('Save Settings'));
 
     expect(onSave).toHaveBeenCalledWith({
-      retentionTime: initialRetentionTime,
+      retention_time: initialRetentionTime,
     });
   });
 
-  it('should display errors', async () => {
-    const detail = capitalize(faker.lorem.words(5));
-    const initialRetentionTime = { value: positiveInt(), unit: 'month' };
-
-    const errors = [
-      {
-        detail,
-        source: { pointer: '/retentionTime' },
-        title: 'Invalid value',
-      },
-    ];
-
-    await act(async () => {
-      render(
-        <ActivityLogsSettingsModal
-          errors={errors}
-          initialRetentionTime={initialRetentionTime}
-          open
-          onSave={() => {}}
-          onCancel={() => {}}
-        />
-      );
-    });
+  const valueError = {
+    detail: faker.lorem.words(20),
+    source: { pointer: '/retention_time/value' },
+    title: faker.lorem.words(10),
+  };
+  const unitError = {
+    detail: faker.lorem.words(20),
+    source: { pointer: '/retention_time/unit' },
+    title: faker.lorem.words(10),
+  };
+
+  it.each`
+    scenario                               | errors                             | expectedErrorMessage
+    ${'value error'}                       | ${[valueError]}                    | ${valueError.detail}
+    ${'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 and value error (2)'} | ${['a generic error', valueError]} | ${valueError.detail}
+  `(
+    'should display errors on $scenario',
+    async ({ errors, expectedErrorMessage }) => {
+      const initialRetentionTime = { value: positiveInt(), unit: 'month' };
+
+      await act(async () => {
+        render(
+          <ActivityLogsSettingsModal
+            errors={errors}
+            initialRetentionTime={initialRetentionTime}
+            open
+            onSave={() => {}}
+            onCancel={() => {}}
+          />
+        );
+      });
 
-    expect(screen.getAllByText(detail)).toHaveLength(1);
-  });
+      expect(
+        screen.getAllByText(expectedErrorMessage, { exact: false })
+      ).toHaveLength(1);
+    }
+  );
 });

assets/js/lib/api/activityLogsSettings.js

@@ -0,0 +1,6 @@
+import { networkClient } from '@lib/network';
+
+export const getSettings = () => networkClient.get(`/settings/activity_log`);
+
+export const updateSettings = (settings) =>
+  networkClient.put(`/settings/activity_log`, settings);

assets/js/lib/api/validationErrors.test.js

@@ -18,6 +18,18 @@ describe('hasError', () => {
     expect(hasError('url', errors)).toBe(true);
   });
 
+  it('should match subfields', () => {
+    const errors = [
+      {
+        detail: "can't be blank",
+        source: { pointer: '/date/year' },
+        title: 'Invalid value',
+      },
+    ];
+
+    expect(hasError('date/year', errors)).toBe(true);
+  });
+
   it('should spot nothing in an empty list', () => {
     expect(hasError('url', [])).toBe(false);
   });

assets/js/lib/test-utils/factories/activityLogsSettings.js

@@ -0,0 +1,9 @@
+import { faker } from '@faker-js/faker';
+import { Factory } from 'fishery';
+
+export const activityLogsSettingsFactory = Factory.define(() => ({
+  retention_time: {
+    value: faker.number.int({ min: 1, max: 30 }),
+    unit: faker.helpers.arrayElement(['day', 'week', 'month', 'year']),
+  },
+}));

assets/js/pages/SettingsPage/SettingsPage.jsx

@@ -33,6 +33,19 @@ import {
   getSoftwareUpdatesSettingsErrors,
 } from '@state/selectors/softwareUpdatesSettings';
 
+import ActivityLogsConfig from '@common/ActivityLogsConfig';
+import ActivityLogsSettingsModal from '@common/ActivityLogsSettingsModal';
+import {
+  fetchActivityLogsSettings,
+  updateActivityLogsSettings,
+  setEditingActivityLogsSettings,
+  setActivityLogsSettingsErrors,
+} from '@state/activityLogsSettings';
+import {
+  getActivityLogsSettings,
+  getActivityLogsSettingsErrors,
+} from '@state/selectors/activityLogsSettings';
+
 import { dismissNotification } from '@state/notifications';
 import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings';
 
@@ -102,6 +115,7 @@ function SettingsPage() {
     setLoading(true);
     fetchApiKeySettings();
     dispatch(fetchSoftwareUpdatesSettings());
+    dispatch(fetchActivityLogsSettings());
   }, []);
 
   const {
@@ -120,6 +134,17 @@ function SettingsPage() {
     (value) => !isUndefined(value)
   );
 
+  const {
+    settings: activityLogsSettings = {},
+    loading: activityLogsSettingsLoading,
+    editing: editingActivityLogsSettings,
+    networkError: activityLogsSettingsNetworkError,
+  } = useSelector(getActivityLogsSettings);
+
+  const activityLogsValidationErrors = useSelector(
+    getActivityLogsSettingsErrors
+  );
+
   const hasApiKey = Boolean(apiKey);
 
   return (
@@ -305,6 +330,36 @@ function SettingsPage() {
           />
         </div>
       )}
+
+      <SettingsLoader
+        sectionName="Activity Logs"
+        status={calculateSettingsLoaderStatus(
+          activityLogsSettingsLoading,
+          activityLogsSettingsNetworkError
+        )}
+        onRetry={() => dispatch(fetchActivityLogsSettings())}
+      >
+        <ActivityLogsConfig
+          retentionTime={activityLogsSettings.retention_time}
+          onEditClick={() => dispatch(setEditingActivityLogsSettings(true))}
+        />
+      </SettingsLoader>
+      <ActivityLogsSettingsModal
+        key={`${JSON.stringify(
+          activityLogsSettings
+        )}-${editingActivityLogsSettings}`}
+        open={editingActivityLogsSettings}
+        errors={activityLogsValidationErrors}
+        loading={activityLogsSettingsLoading}
+        initialRetentionTime={activityLogsSettings.retention_time}
+        onSave={(payload) => {
+          dispatch(updateActivityLogsSettings(payload));
+        }}
+        onCancel={() => {
+          dispatch(setActivityLogsSettingsErrors([]));
+          dispatch(setEditingActivityLogsSettings(false));
+        }}
+      />
     </section>
   );
 }