From da82582eb6b16410fa8228d1d4f2ddaf0856e066 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:24:14 +0200 Subject: [PATCH 01/25] replace SuseManagerSettingsContainer Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 9b31730b78..650766f25c 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,6 +35,9 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; +import SettingsLoader, { + calculateStatus as calculateSettingsLoaderStatus, +} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 85cde85fb665c8a564ba01de8b1977ddbf080b6f Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:23:48 +0200 Subject: [PATCH 02/25] add SettingsLoader component Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 650766f25c..9b31730b78 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,9 +35,6 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; -import SettingsLoader, { - calculateStatus as calculateSettingsLoaderStatus, -} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 3ee0c36fba2e41e8e2a60ee106996adda6110937 Mon Sep 17 00:00:00 2001 From: balanza Date: Fri, 21 Jun 2024 17:14:24 +0200 Subject: [PATCH 03/25] refactor: move errors utility Signed-off-by: balanza --- .../SuseManagerSettingsModal.jsx | 2 +- assets/js/lib/input/errors.js | 20 +++++ assets/js/lib/input/errors.test.js | 81 +++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 assets/js/lib/input/errors.js create mode 100644 assets/js/lib/input/errors.test.js diff --git a/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx b/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx index 144b7954d0..4731c087ce 100644 --- a/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx +++ b/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx @@ -8,7 +8,7 @@ import Modal from '@common/Modal'; import Input, { Password, Textarea } from '@common/Input'; import Label from '@common/Label'; -import { hasError, getError } from '@lib/api/validationErrors'; +import { hasError, getError } from '@lib/input/errors'; const defaultErrors = []; diff --git a/assets/js/lib/input/errors.js b/assets/js/lib/input/errors.js new file mode 100644 index 0000000000..de19dcebcf --- /dev/null +++ b/assets/js/lib/input/errors.js @@ -0,0 +1,20 @@ +import { flow, first } from 'lodash'; +import { get, filter } from 'lodash/fp'; + +export const hasError = (keyword, errors) => + errors.some((error) => { + const pointer = get(['source', 'pointer'], error); + + return pointer === `/${keyword}`; + }); + +export const getError = (keyword, errors) => + flow([ + filter((error) => { + const pointer = get(['source', 'pointer'], error); + + return pointer === `/${keyword}`; + }), + first, + get('detail'), + ])(errors); diff --git a/assets/js/lib/input/errors.test.js b/assets/js/lib/input/errors.test.js new file mode 100644 index 0000000000..a043de21cd --- /dev/null +++ b/assets/js/lib/input/errors.test.js @@ -0,0 +1,81 @@ +import { hasError, getError } from './errors'; + +describe('hasError', () => { + it('should tell that a list contains an error about a specific field', () => { + const errors = [ + { + detail: "can't be blank", + source: { pointer: '/url' }, + title: 'Invalid value', + }, + { + detail: "can't be blank", + source: { pointer: '/ca_cert' }, + title: 'Invalid value', + }, + ]; + + expect(hasError('url', errors)).toBe(true); + }); + + it('should spot nothing in an empty list', () => { + expect(hasError('url', [])).toBe(false); + }); + + it('should spot nothing when the keyword is not in the list', () => { + const errors = [ + { + detail: "can't be blank", + source: { pointer: '/username' }, + title: 'Invalid value', + }, + { + detail: "can't be blank", + source: { pointer: '/ca_cert' }, + title: 'Invalid value', + }, + ]; + + expect(hasError('url', errors)).toBe(false); + }); +}); + +describe('getError', () => { + it('should spot nothing in an empty list', () => { + expect(getError('url', [])).toBe(undefined); + }); + + it('should get nothing when the keyword is not in the list', () => { + const errors = [ + { + detail: "can't be blank", + source: { pointer: '/username' }, + title: 'Invalid value', + }, + { + detail: "can't be blank", + source: { pointer: '/ca_cert' }, + title: 'Invalid value', + }, + ]; + + expect(getError('username', errors)).toBe("can't be blank"); + }); + + it('should get nothing when the keyword is not in the list', () => { + const errors = [ + { + detail: "can't be blank", + source: { pointer: '/username' }, + title: 'Invalid value', + }, + { + detail: "can't be blank", + source: { pointer: '/ca_cert' }, + title: 'Invalid value', + }, + ]; + + expect(getError('url', errors)).toBe(undefined); + }); +}); From 73139dc0cc12aad319c83a9a551ea2e0369c1ef1 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:24:14 +0200 Subject: [PATCH 04/25] replace SuseManagerSettingsContainer Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 9b31730b78..650766f25c 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,6 +35,9 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; +import SettingsLoader, { + calculateStatus as calculateSettingsLoaderStatus, +} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 42c19a564bfd3fbe1cc18d3192ceba9becc6e06f Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:23:48 +0200 Subject: [PATCH 05/25] add SettingsLoader component Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 650766f25c..9b31730b78 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,9 +35,6 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; -import SettingsLoader, { - calculateStatus as calculateSettingsLoaderStatus, -} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From f7c731ed5561ef57b8c66cf5d234b81ebccf9fe7 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:24:14 +0200 Subject: [PATCH 06/25] replace SuseManagerSettingsContainer Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 9b31730b78..650766f25c 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,6 +35,9 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; +import SettingsLoader, { + calculateStatus as calculateSettingsLoaderStatus, +} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 270304f41ff203b409d74729367ae180de42f112 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:23:48 +0200 Subject: [PATCH 07/25] add SettingsLoader component Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 650766f25c..9b31730b78 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,9 +35,6 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; -import SettingsLoader, { - calculateStatus as calculateSettingsLoaderStatus, -} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 9b4f577af372c1882580a7a466bedaf9c601148f Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:24:14 +0200 Subject: [PATCH 08/25] replace SuseManagerSettingsContainer Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 9b31730b78..650766f25c 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,6 +35,9 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; +import SettingsLoader, { + calculateStatus as calculateSettingsLoaderStatus, +} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 3c88f35c732bf7003cb48bbf6e891f3cd251e5d2 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:23:48 +0200 Subject: [PATCH 09/25] add SettingsLoader component Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 650766f25c..9b31730b78 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,9 +35,6 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; -import SettingsLoader, { - calculateStatus as calculateSettingsLoaderStatus, -} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From a4043d2e8c3e7558ac3b04ff4d9ea5fd44763006 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:24:14 +0200 Subject: [PATCH 10/25] replace SuseManagerSettingsContainer Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 9b31730b78..650766f25c 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,6 +35,9 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; +import SettingsLoader, { + calculateStatus as calculateSettingsLoaderStatus, +} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 10fee272c43c9f0e72c35d08c809adc550a481a0 Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:23:48 +0200 Subject: [PATCH 11/25] add SettingsLoader component Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 650766f25c..9b31730b78 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,9 +35,6 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; -import SettingsLoader, { - calculateStatus as calculateSettingsLoaderStatus, -} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 4668beed5f515f3ca870130877d01391eaa699dd Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:24:14 +0200 Subject: [PATCH 12/25] replace SuseManagerSettingsContainer Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 9b31730b78..650766f25c 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,6 +35,9 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; +import SettingsLoader, { + calculateStatus as calculateSettingsLoaderStatus, +} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 9f05a2c5d4b94240e6352594f369955d76e5b07d Mon Sep 17 00:00:00 2001 From: balanza Date: Wed, 19 Jun 2024 16:23:48 +0200 Subject: [PATCH 13/25] add SettingsLoader component Signed-off-by: balanza --- assets/js/pages/SettingsPage/SettingsPage.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 650766f25c..9b31730b78 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.jsx @@ -35,9 +35,6 @@ import { import { dismissNotification } from '@state/notifications'; import { API_KEY_EXPIRATION_NOTIFICATION_ID } from '@state/sagas/settings'; -import SettingsLoader, { - calculateStatus as calculateSettingsLoaderStatus, -} from '@common/SettingsLoader'; function ApiKeyExpireInfo({ apiKeyExpiration }) { const expirationLabel = () => { From 45bad2535208fb3d656eb1bfcbb9c4515051a466 Mon Sep 17 00:00:00 2001 From: balanza Date: Thu, 27 Jun 2024 12:35:11 +0200 Subject: [PATCH 14/25] remove duplicate module --- .../SuseManagerSettingsModal.jsx | 2 +- assets/js/lib/input/errors.js | 20 ----- assets/js/lib/input/errors.test.js | 81 ------------------- 3 files changed, 1 insertion(+), 102 deletions(-) delete mode 100644 assets/js/lib/input/errors.js delete mode 100644 assets/js/lib/input/errors.test.js diff --git a/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx b/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx index 4731c087ce..144b7954d0 100644 --- a/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx +++ b/assets/js/common/SuseManagerSettingsDialog/SuseManagerSettingsModal.jsx @@ -8,7 +8,7 @@ import Modal from '@common/Modal'; import Input, { Password, Textarea } from '@common/Input'; import Label from '@common/Label'; -import { hasError, getError } from '@lib/input/errors'; +import { hasError, getError } from '@lib/api/validationErrors'; const defaultErrors = []; diff --git a/assets/js/lib/input/errors.js b/assets/js/lib/input/errors.js deleted file mode 100644 index de19dcebcf..0000000000 --- a/assets/js/lib/input/errors.js +++ /dev/null @@ -1,20 +0,0 @@ -import { flow, first } from 'lodash'; -import { get, filter } from 'lodash/fp'; - -export const hasError = (keyword, errors) => - errors.some((error) => { - const pointer = get(['source', 'pointer'], error); - - return pointer === `/${keyword}`; - }); - -export const getError = (keyword, errors) => - flow([ - filter((error) => { - const pointer = get(['source', 'pointer'], error); - - return pointer === `/${keyword}`; - }), - first, - get('detail'), - ])(errors); diff --git a/assets/js/lib/input/errors.test.js b/assets/js/lib/input/errors.test.js deleted file mode 100644 index a043de21cd..0000000000 --- a/assets/js/lib/input/errors.test.js +++ /dev/null @@ -1,81 +0,0 @@ -import { hasError, getError } from './errors'; - -describe('hasError', () => { - it('should tell that a list contains an error about a specific field', () => { - const errors = [ - { - detail: "can't be blank", - source: { pointer: '/url' }, - title: 'Invalid value', - }, - { - detail: "can't be blank", - source: { pointer: '/ca_cert' }, - title: 'Invalid value', - }, - ]; - - expect(hasError('url', errors)).toBe(true); - }); - - it('should spot nothing in an empty list', () => { - expect(hasError('url', [])).toBe(false); - }); - - it('should spot nothing when the keyword is not in the list', () => { - const errors = [ - { - detail: "can't be blank", - source: { pointer: '/username' }, - title: 'Invalid value', - }, - { - detail: "can't be blank", - source: { pointer: '/ca_cert' }, - title: 'Invalid value', - }, - ]; - - expect(hasError('url', errors)).toBe(false); - }); -}); - -describe('getError', () => { - it('should spot nothing in an empty list', () => { - expect(getError('url', [])).toBe(undefined); - }); - - it('should get nothing when the keyword is not in the list', () => { - const errors = [ - { - detail: "can't be blank", - source: { pointer: '/username' }, - title: 'Invalid value', - }, - { - detail: "can't be blank", - source: { pointer: '/ca_cert' }, - title: 'Invalid value', - }, - ]; - - expect(getError('username', errors)).toBe("can't be blank"); - }); - - it('should get nothing when the keyword is not in the list', () => { - const errors = [ - { - detail: "can't be blank", - source: { pointer: '/username' }, - title: 'Invalid value', - }, - { - detail: "can't be blank", - source: { pointer: '/ca_cert' }, - title: 'Invalid value', - }, - ]; - - expect(getError('url', errors)).toBe(undefined); - }); -}); From c0b61a46acc8ed40fa143b06ae780a8c1dc18cfd Mon Sep 17 00:00:00 2001 From: balanza Date: Fri, 28 Jun 2024 16:31:01 +0200 Subject: [PATCH 15/25] mount audit log section --- .../ActivityLogsSettingsModal.jsx | 20 +- .../ActivityLogsSettingsModal.test.jsx | 69 ++-- assets/js/lib/api/activityLogsSettings.js | 6 + assets/js/lib/api/validationErrors.test.js | 12 + .../factories/activityLogsSettings.js | 9 + assets/js/pages/SettingsPage/SettingsPage.jsx | 55 ++++ .../pages/SettingsPage/SettingsPage.test.jsx | 304 +++++++++++++----- assets/js/state/activityLogsSettings.js | 59 ++++ assets/js/state/activityLogsSettings.test.js | 129 ++++++++ assets/js/state/index.js | 2 + assets/js/state/sagas/activityLogsSettings.js | 45 +++ .../state/sagas/activityLogsSettings.test.js | 97 ++++++ assets/js/state/sagas/index.js | 2 + .../state/selectors/activityLogsSettings.js | 36 +++ .../selectors/activityLogsSettings.test.js | 118 +++++++ 15 files changed, 855 insertions(+), 108 deletions(-) create mode 100644 assets/js/lib/api/activityLogsSettings.js create mode 100644 assets/js/lib/test-utils/factories/activityLogsSettings.js create mode 100644 assets/js/state/activityLogsSettings.js create mode 100644 assets/js/state/activityLogsSettings.test.js create mode 100644 assets/js/state/sagas/activityLogsSettings.js create mode 100644 assets/js/state/sagas/activityLogsSettings.test.js create mode 100644 assets/js/state/selectors/activityLogsSettings.js create mode 100644 assets/js/state/selectors/activityLogsSettings.test.js diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx index 8889a87f14..2ba9bbae50 100644 --- a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx +++ b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx @@ -7,13 +7,21 @@ 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 toErrorMessage = (errors) => + [ + capitalize(getError('retention_time/value', errors)), + capitalize(getError('retention_time/unit', errors)), + ] + .filter(Boolean) + .join(', '); + function TimeSpan({ time: initialTime, error = false, onChange = noop }) { const [time, setTime] = useState(initialTime); @@ -64,6 +72,8 @@ function ActivityLogsSettingsModal({ }) { const [retentionTime, setRetentionTime] = useState(initialRetentionTime); + const errorMessage = toErrorMessage(errors); + return (
@@ -73,18 +83,18 @@ function ActivityLogsSettingsModal({
{ setRetentionTime(time); onClearErrors(); }} /> - {hasError('retentionTime', errors) && ( + {errorMessage && (

- {capitalize(getError('retentionTime', errors))} + {errorMessage}

)}
@@ -98,7 +108,7 @@ function ActivityLogsSettingsModal({ disabled={loading} onClick={() => { const payload = { - retentionTime, + retention_time: retentionTime, }; onSave(payload); }} diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx index 7bd9771e40..4f05f516ea 100644 --- a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx +++ b/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,47 @@ 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( - {}} - 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} + `( + 'should display errors on $scenario', + async ({ errors, expectedErrorMessage }) => { + const initialRetentionTime = { value: positiveInt(), unit: 'month' }; + + await act(async () => { + render( + {}} + onCancel={() => {}} + /> + ); + }); - expect(screen.getAllByText(detail)).toHaveLength(1); - }); + expect( + screen.getAllByText(expectedErrorMessage, { exact: false }) + ).toHaveLength(1); + } + ); }); diff --git a/assets/js/lib/api/activityLogsSettings.js b/assets/js/lib/api/activityLogsSettings.js new file mode 100644 index 0000000000..e4633b1b05 --- /dev/null +++ b/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); diff --git a/assets/js/lib/api/validationErrors.test.js b/assets/js/lib/api/validationErrors.test.js index b764214999..d197bb0d8d 100644 --- a/assets/js/lib/api/validationErrors.test.js +++ b/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); }); diff --git a/assets/js/lib/test-utils/factories/activityLogsSettings.js b/assets/js/lib/test-utils/factories/activityLogsSettings.js new file mode 100644 index 0000000000..a3b1123643 --- /dev/null +++ b/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']), + }, +})); diff --git a/assets/js/pages/SettingsPage/SettingsPage.jsx b/assets/js/pages/SettingsPage/SettingsPage.jsx index 9b31730b78..2160711f6a 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.jsx +++ b/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() { />
)} + + dispatch(fetchActivityLogsSettings())} + > + dispatch(setEditingActivityLogsSettings(true))} + /> + + { + dispatch(updateActivityLogsSettings(payload)); + }} + onCancel={() => { + dispatch(setActivityLogsSettingsErrors([])); + dispatch(setEditingActivityLogsSettings(false)); + }} + /> ); } diff --git a/assets/js/pages/SettingsPage/SettingsPage.test.jsx b/assets/js/pages/SettingsPage/SettingsPage.test.jsx index 98b76007be..e119df69c2 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.test.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.test.jsx @@ -29,105 +29,261 @@ describe('Settings Page', () => { }); }); - it('should render the api key with copy button', async () => { - const [StatefulSettings] = withState(, { - ...defaultInitialState, - softwareUpdatesSettings: { - loading: true, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - }, + describe('API Key Section', () => { + it('should render the api key with copy button', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: true, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + settings: { retention_time: { value: 1, unit: 'day' } }, + }, + }); + + await act(async () => { + renderWithRouter(StatefulSettings); + }); + + expect(screen.getByText('Key will never expire')).toBeVisible(); + expect(screen.getByText('api_key')).toBeVisible(); + expect( + screen.getByRole('button', { name: 'copy to clipboard' }) + ).toBeVisible(); }); + }); + + describe('Software Updates Section', () => { + it('should render a loading box while fetching settings', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: true, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + settings: { retention_time: { value: 1, unit: 'day' } }, + }, + }); - await act(async () => { renderWithRouter(StatefulSettings); + + expect( + screen.getByText('Loading SUSE Manager Settings...') + ).toBeVisible(); }); - expect(screen.getByText('Key will never expire')).toBeVisible(); - expect(screen.getByText('api_key')).toBeVisible(); - expect( - screen.getByRole('button', { name: 'copy to clipboard' }) - ).toBeVisible(); - }); + it('should render an empty SUSE Manager Config Section', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: false, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + error: null, + }, + activityLogsSettings: { + settings: { retention_time: { value: 1, unit: 'day' } }, + }, + }); - it('should render a loading box while fetching settings', async () => { - const [StatefulSettings] = withState(, { - ...defaultInitialState, - softwareUpdatesSettings: { - loading: true, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - }, + renderWithRouter(StatefulSettings); + + expect(screen.getByText('SUSE Manager URL')).toBeVisible(); + expect(screen.getByText('https://')).toBeVisible(); + + expect(screen.getByText('CA Certificate')).toBeVisible(); + expect(screen.getByText('-')).toBeVisible(); + + expect(screen.getByText('Username')).toBeVisible(); + expect(screen.getByText('Password')).toBeVisible(); + + expect(screen.queryAllByText('.....')).toHaveLength(2); }); - renderWithRouter(StatefulSettings); + it('should render SUSE Manager Config Section with configured settings', async () => { + const settings = softwareUpdatesSettingsFactory.build(); + + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: false, + settings, + error: null, + }, + activityLogsSettings: { + settings: { retention_time: { value: 1, unit: 'day' } }, + }, + }); + + const { url, username, ca_uploaded_at } = settings; + + renderWithRouter(StatefulSettings); + + expect(screen.getByText('SUSE Manager URL')).toBeVisible(); + expect(screen.getByText(url)).toBeVisible(); + + expect(screen.getByText('CA Certificate')).toBeVisible(); + expect(screen.getByText('Certificate Uploaded')).toBeVisible(); + expect( + screen.getByText(format(ca_uploaded_at, "'Uploaded:' dd MMM y")) + ).toBeVisible(); + + expect(screen.getByText('Username')).toBeVisible(); + expect(screen.getByText(username)).toBeVisible(); - expect(screen.getByText('Loading SUSE Manager Settings...')).toBeVisible(); + expect(screen.getByText('Password')).toBeVisible(); + expect(screen.getByText('•••••')).toBeVisible(); + }); }); - it('should render an empty SUSE Manager Config Section', async () => { - const [StatefulSettings] = withState(, { - ...defaultInitialState, - softwareUpdatesSettings: { - loading: false, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - error: null, - }, + describe('Activity Logs Section', () => { + it('should render activity logs section', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: true, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + loading: false, + settings: { retention_time: { value: 1, unit: 'day' } }, + }, + }); + + await act(async () => { + renderWithRouter(StatefulSettings); + }); + + expect(screen.getByText('Activity Logs')).toBeVisible(); }); - renderWithRouter(StatefulSettings); + it('should render loader on activity logs section', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: true, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + loading: true, + settings: { retention_time: { value: 1, unit: 'day' } }, + }, + }); - expect(screen.getByText('SUSE Manager URL')).toBeVisible(); - expect(screen.getByText('https://')).toBeVisible(); + await act(async () => { + renderWithRouter(StatefulSettings); + }); - expect(screen.getByText('CA Certificate')).toBeVisible(); - expect(screen.getByText('-')).toBeVisible(); + expect( + screen.getByText('Loading Activity Logs Settings...') + ).toBeVisible(); + }); - expect(screen.getByText('Username')).toBeVisible(); - expect(screen.getByText('Password')).toBeVisible(); + it('should render edit modal', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: true, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + editing: true, + settings: { retention_time: { value: 1, unit: 'day' } }, + }, + }); - expect(screen.queryAllByText('.....')).toHaveLength(2); - }); + await act(async () => { + renderWithRouter(StatefulSettings); + }); + + expect(screen.getByText('Enter Activity Logs Settings')).toBeVisible(); - it('should render SUSE Manager Config Section with configured settings', async () => { - const settings = softwareUpdatesSettingsFactory.build(); + expect(screen.getByText('1')).toBeVisible(); - const [StatefulSettings] = withState(, { - ...defaultInitialState, - softwareUpdatesSettings: { - loading: false, - settings, - error: null, - }, + expect(screen.getByText('day')).toBeVisible(); + + expect(screen.getByText('Save Settings')).toBeVisible(); + + expect(screen.getByText('Cancel')).toBeVisible(); }); - const { url, username, ca_uploaded_at } = settings; + it('should render saving errors', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: true, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + editing: true, + settings: { retention_time: { value: 1, unit: 'day' } }, + errors: [ + { + detail: 'Invalid data provided', + source: { pointer: '/retention_time/value' }, + title: 'Invalid data', + }, + ], + }, + }); - renderWithRouter(StatefulSettings); + await act(async () => { + renderWithRouter(StatefulSettings); + }); - expect(screen.getByText('SUSE Manager URL')).toBeVisible(); - expect(screen.getByText(url)).toBeVisible(); + expect(screen.getByText('Invalid data provided')).toBeVisible(); + }); - expect(screen.getByText('CA Certificate')).toBeVisible(); - expect(screen.getByText('Certificate Uploaded')).toBeVisible(); - expect( - screen.getByText(format(ca_uploaded_at, "'Uploaded:' dd MMM y")) - ).toBeVisible(); + it('should render saved configuration', async () => { + const [StatefulSettings] = withState(, { + ...defaultInitialState, + softwareUpdatesSettings: { + loading: true, + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + settings: { retention_time: { value: 2, unit: 'day' } }, + }, + }); - expect(screen.getByText('Username')).toBeVisible(); - expect(screen.getByText(username)).toBeVisible(); + await act(async () => { + renderWithRouter(StatefulSettings); + }); - expect(screen.getByText('Password')).toBeVisible(); - expect(screen.getByText('•••••')).toBeVisible(); + expect(screen.getByText('2 days')).toBeVisible(); + }); }); }); diff --git a/assets/js/state/activityLogsSettings.js b/assets/js/state/activityLogsSettings.js new file mode 100644 index 0000000000..56b01d35b3 --- /dev/null +++ b/assets/js/state/activityLogsSettings.js @@ -0,0 +1,59 @@ +import { createAction, createSlice } from '@reduxjs/toolkit'; + +const emptySettings = { + retention_time: undefined, +}; + +const initialState = { + loading: true, + settings: emptySettings, + networkError: false, + editing: false, + errors: [], +}; +export const activityLogsSettingsSlice = createSlice({ + name: 'activityLogsSettings', + initialState, + reducers: { + startLoadingActivityLogsSettings: (state) => { + state.loading = true; + }, + setActivityLogsSettings: (state, { payload: settings }) => { + state.loading = false; + state.networkError = false; + state.settings = settings; + }, + setActivityLogsSettingsErrors: (state, { payload: errors }) => { + state.loading = false; + state.networkError = false; + state.errors = errors; + }, + setEditingActivityLogsSettings: (state, { payload }) => { + state.editing = payload; + }, + + setNetworkError: (state, { payload }) => { + state.loading = false; + state.networkError = payload; + }, + }, +}); + +export const FETCH_ACTIVITY_LOGS_SETTINGS = 'FETCH_ACTIVITY_LOGS_SETTINGS'; +export const UPDATE_ACTIVITY_LOGS_SETTINGS = 'UPDATE_ACTIVITY_LOGS_SETTINGS'; + +export const fetchActivityLogsSettings = createAction( + FETCH_ACTIVITY_LOGS_SETTINGS +); +export const updateActivityLogsSettings = createAction( + UPDATE_ACTIVITY_LOGS_SETTINGS +); +export const { + startLoadingActivityLogsSettings, + setActivityLogsSettings, + setActivityLogsSettingsErrors, + setEditingActivityLogsSettings, + setNetworkError, +} = activityLogsSettingsSlice.actions; + +export default activityLogsSettingsSlice.reducer; diff --git a/assets/js/state/activityLogsSettings.test.js b/assets/js/state/activityLogsSettings.test.js new file mode 100644 index 0000000000..800afa6578 --- /dev/null +++ b/assets/js/state/activityLogsSettings.test.js @@ -0,0 +1,129 @@ +import activityLogsSettingsReducer, { + startLoadingActivityLogsSettings, + setActivityLogsSettings, + setActivityLogsSettingsErrors, + setEditingActivityLogsSettings, + setNetworkError, +} from './activityLogsSettings'; + +describe('ActivityLogsSettings reducer', () => { + it('should mark software updates settings on loading state', () => { + const initialState = { + loading: false, + }; + + const action = startLoadingActivityLogsSettings(); + + const expectedState = { + loading: true, + }; + + expect(activityLogsSettingsReducer(initialState, action)).toEqual( + expectedState + ); + }); + + it('should set software updates settings', () => { + const initialState = { + loading: true, + settings: { + retentionTime: undefined, + }, + networkError: false, + errors: [], + }; + + const settings = { + retentionTime: { value: 2, unit: 'week' }, + }; + + const action = setActivityLogsSettings(settings); + + const actual = activityLogsSettingsReducer(initialState, action); + + expect(actual).toEqual({ + loading: false, + settings, + networkError: false, + errors: [], + }); + }); + + it('should set errors upon validation failed', () => { + const initialState = { + loading: false, + settings: { + retentionTime: { value: 2, unit: 'week' }, + }, + networkError: false, + errors: [], + }; + + const errors = [ + { + detail: "can't be blank", + source: { pointer: '/retentionTime' }, + title: 'Invalid value', + }, + ]; + + const action = setActivityLogsSettingsErrors(errors); + + const actual = activityLogsSettingsReducer(initialState, action); + + expect(actual).toEqual({ + loading: false, + settings: { + retentionTime: { value: 2, unit: 'week' }, + }, + networkError: false, + errors, + }); + }); + + it('should set the editing field to true', () => { + const initialState = { + loading: false, + settings: { + retentionTime: { value: 2, unit: 'week' }, + }, + networkError: false, + errors: [], + editing: false, + }; + + const action = setEditingActivityLogsSettings(true); + + const actual = activityLogsSettingsReducer(initialState, action); + + expect(actual).toEqual({ + ...initialState, + editing: true, + }); + }); + + it('should mark that the connection is being tested', () => { + const initialState = { + loading: true, + settings: { + retentionTime: { value: 2, unit: 'week' }, + }, + networkError: false, + errors: [], + editing: false, + testingConnection: false, + }; + + [true, false].forEach((hasNetworkError) => { + const action = setNetworkError(hasNetworkError); + + const actual = activityLogsSettingsReducer(initialState, action); + + expect(actual).toEqual({ + ...initialState, + loading: false, + networkError: hasNetworkError, + }); + }); + }); +}); diff --git a/assets/js/state/index.js b/assets/js/state/index.js index 008e765aca..117b5f9e0e 100644 --- a/assets/js/state/index.js +++ b/assets/js/state/index.js @@ -14,6 +14,7 @@ import settingsReducer from './settings'; import userReducer from './user'; import softwareUpdatesReducer from './softwareUpdates'; import softwareUpdatesSettingsReducer from './softwareUpdatesSettings'; +import activityLogsSettingsReducer from './activityLogsSettings'; import rootSaga from './sagas'; export const createStore = (router) => { @@ -38,6 +39,7 @@ export const createStore = (router) => { user: userReducer, softwareUpdates: softwareUpdatesReducer, softwareUpdatesSettings: softwareUpdatesSettingsReducer, + activityLogsSettings: activityLogsSettingsReducer, }, middleware: (getDefaultMiddleware) => getDefaultMiddleware().concat(sagaMiddleware), diff --git a/assets/js/state/sagas/activityLogsSettings.js b/assets/js/state/sagas/activityLogsSettings.js new file mode 100644 index 0000000000..42b1498865 --- /dev/null +++ b/assets/js/state/sagas/activityLogsSettings.js @@ -0,0 +1,45 @@ +import { get } from 'lodash'; +import { put, call, takeEvery, debounce } from 'redux-saga/effects'; +import { getSettings, updateSettings } from '@lib/api/activityLogsSettings'; + +import { + FETCH_ACTIVITY_LOGS_SETTINGS, + UPDATE_ACTIVITY_LOGS_SETTINGS, + startLoadingActivityLogsSettings, + setActivityLogsSettings, + setActivityLogsSettingsErrors, + setEditingActivityLogsSettings, + setNetworkError, +} from '@state/activityLogsSettings'; + +export function* fetchActivityLogsSettings() { + yield put(startLoadingActivityLogsSettings()); + + try { + const response = yield call(getSettings); + yield put(setActivityLogsSettings(response.data)); + } catch (error) { + const errorCode = get(error, ['response', 'status']); + if (errorCode !== 403) { + yield put(setNetworkError(true)); + } + } +} + +export function* updateActivityLogsSettings({ payload }) { + yield put(startLoadingActivityLogsSettings()); + try { + const response = yield call(updateSettings, payload); + yield put(setActivityLogsSettings(response.data)); + yield put(setEditingActivityLogsSettings(false)); + yield put(setActivityLogsSettingsErrors([])); + } catch (error) { + const errors = get(error, ['response', 'data', 'errors'], []); + yield put(setActivityLogsSettingsErrors(errors)); + } +} + +export function* watchActivityLogsSettings() { + yield debounce(1000, FETCH_ACTIVITY_LOGS_SETTINGS, fetchActivityLogsSettings); + yield takeEvery(UPDATE_ACTIVITY_LOGS_SETTINGS, updateActivityLogsSettings); +} diff --git a/assets/js/state/sagas/activityLogsSettings.test.js b/assets/js/state/sagas/activityLogsSettings.test.js new file mode 100644 index 0000000000..a46eb3e30b --- /dev/null +++ b/assets/js/state/sagas/activityLogsSettings.test.js @@ -0,0 +1,97 @@ +import { recordSaga } from '@lib/test-utils'; + +import { networkClient } from '@lib/network'; +import MockAdapter from 'axios-mock-adapter'; + +import { activityLogsSettingsFactory } from '@lib/test-utils/factories/activityLogsSettings'; + +import { + startLoadingActivityLogsSettings, + setActivityLogsSettings, + setActivityLogsSettingsErrors, + setEditingActivityLogsSettings, + setNetworkError, +} from '@state/activityLogsSettings'; + +import { + fetchActivityLogsSettings, + updateActivityLogsSettings, +} from './activityLogsSettings'; + +describe('Activity Logs Settings saga', () => { + describe('Fetching Activity Logs Settings', () => { + it('should successfully fetch activity logs settings', async () => { + const axiosMock = new MockAdapter(networkClient); + const successfulResponse = activityLogsSettingsFactory.build(); + + axiosMock.onGet('/settings/activity_log').reply(200, successfulResponse); + + const dispatched = await recordSaga(fetchActivityLogsSettings); + + expect(dispatched).toEqual([ + startLoadingActivityLogsSettings(), + setActivityLogsSettings(successfulResponse), + ]); + }); + + + it.each([403, 404, 500, 502, 504])( + 'should put a network error flag on failed fetching', + async (status) => { + const axiosMock = new MockAdapter(networkClient); + axiosMock.onGet('/settings/activity_logs').reply(status); + + const dispatched = await recordSaga(fetchActivityLogsSettings); + + expect(dispatched).toEqual([ + startLoadingActivityLogsSettings(), + setNetworkError(true), + ]); + } + ); + }); + + describe('Updating Activity Logs settings', () => { + it('should successfully change activity logs settings', async () => { + const axiosMock = new MockAdapter(networkClient); + const payload = activityLogsSettingsFactory.build(); + + axiosMock.onPut('/settings/activity_log').reply(200, payload); + + const dispatched = await recordSaga(updateActivityLogsSettings, payload); + + expect(dispatched).toEqual([ + startLoadingActivityLogsSettings(), + setActivityLogsSettings(payload), + setEditingActivityLogsSettings(false), + setActivityLogsSettingsErrors([]), + ]); + }); + + it('should have errors on failed update', async () => { + const axiosMock = new MockAdapter(networkClient); + const payload = activityLogsSettingsFactory.build(); + + const errors = [ + { + detail: "can't be blank", + source: { pointer: '/retention_time/value' }, + title: 'Invalid value', + }, + ]; + + axiosMock.onPut('/settings/activity_log', payload).reply(422, { + errors, + }); + + const dispatched = await recordSaga(updateActivityLogsSettings, { + payload, + }); + + expect(dispatched).toEqual([ + startLoadingActivityLogsSettings(), + setActivityLogsSettingsErrors(errors), + ]); + }); + }); +}); diff --git a/assets/js/state/sagas/index.js b/assets/js/state/sagas/index.js index 1b8c10f87f..d969a03746 100644 --- a/assets/js/state/sagas/index.js +++ b/assets/js/state/sagas/index.js @@ -77,6 +77,7 @@ import { } from '@state/sagas/user'; import { watchChecksSelectionEvents } from '@state/sagas/checksSelection'; import { watchSoftwareUpdateSettings } from '@state/sagas/softwareUpdatesSettings'; +import { watchActivityLogsSettings } from '@state/sagas/activityLogsSettings'; import { watchSoftwareUpdates } from '@state/sagas/softwareUpdates'; import { watchSocketEvents } from '@state/sagas/channels'; @@ -248,6 +249,7 @@ export default function* rootSaga() { watchSapSystemEvents(), watchUserLoggedIn(), watchSoftwareUpdateSettings(), + watchActivityLogsSettings(), watchSoftwareUpdates(), ]); } diff --git a/assets/js/state/selectors/activityLogsSettings.js b/assets/js/state/selectors/activityLogsSettings.js new file mode 100644 index 0000000000..4b16eea613 --- /dev/null +++ b/assets/js/state/selectors/activityLogsSettings.js @@ -0,0 +1,36 @@ +import { createSelector } from '@reduxjs/toolkit'; + +export const getActivityLogsSettings = createSelector( + [({ activityLogsSettings }) => activityLogsSettings], + ({ + settings, + errors, + loading, + editing, + networkError, + testingConnection, + }) => ({ + settings, + errors, + loading, + editing, + networkError, + testingConnection, + }) +); + +export const getActivityLogsSettingsLoading = createSelector( + [getActivityLogsSettings], + ({ loading }) => loading +); + +export const getActivityLogsSettingsSaved = createSelector( + [getActivityLogsSettings], + ({ settings: { retention_time } }) => + !!(retention_time && retention_time.value && retention_time.unit) +); + +export const getActivityLogsSettingsErrors = createSelector( + [getActivityLogsSettings], + ({ errors }) => errors +); diff --git a/assets/js/state/selectors/activityLogsSettings.test.js b/assets/js/state/selectors/activityLogsSettings.test.js new file mode 100644 index 0000000000..dc010cad54 --- /dev/null +++ b/assets/js/state/selectors/activityLogsSettings.test.js @@ -0,0 +1,118 @@ +import { + getActivityLogsSettings, + getActivityLogsSettingsLoading, + getActivityLogsSettingsSaved, +} from './activityLogsSettings'; + +describe('Activity Logs Settings selector', () => { + describe('get activity logs settings', () => { + const stateScenarios = [ + { + loading: false, + settings: { + retention_time: { value: 2, unit: 'week' }, + }, + errors: null, + editing: false, + }, + { + loading: true, + settings: { + retention_time: undefined, + }, + errors: null, + editing: false, + testingConnection: false, + }, + ]; + + it.each(stateScenarios)( + 'should return the correct catalog state', + (activityLogsSettings) => { + expect(getActivityLogsSettings({ activityLogsSettings })).toEqual( + activityLogsSettings + ); + } + ); + }); + + describe('get activity logs settings loading', () => { + const scenarios = [ + { + state: { loading: false }, + result: false, + }, + { + state: { loading: true }, + result: true, + }, + ]; + + it.each(scenarios)( + 'should return if the activity logs settings are loading', + ({ state, result }) => { + expect( + getActivityLogsSettingsLoading({ activityLogsSettings: state }) + ).toEqual(result); + } + ); + }); + + describe('get if activity logs settings saved', () => { + const scenarios = [ + { + state: { + loading: false, + settings: { + retention_time: { value: 2, unit: 'week' }, + }, + errors: null, + editing: false, + }, + result: true, + }, + { + state: { + loading: false, + settings: { + retention_time: undefined, + }, + errors: null, + editing: false, + }, + result: false, + }, + { + state: { + loading: false, + settings: { + retention_time: {} /* invalid */, + }, + errors: null, + editing: false, + }, + result: false, + }, + { + state: { + loading: true, + settings: { + retention_time: undefined, + }, + errors: null, + editing: false, + }, + result: false, + }, + ]; + + it.each(scenarios)( + 'should return if the activity logs settings are saved', + ({ state, result }) => { + expect( + getActivityLogsSettingsSaved({ activityLogsSettings: state }) + ).toEqual(result); + } + ); + }); +}); From 37e69a4fe597812b327c331465be1936267ca48d Mon Sep 17 00:00:00 2001 From: balanza Date: Fri, 28 Jun 2024 16:57:23 +0200 Subject: [PATCH 16/25] fix formatter --- assets/js/state/sagas/activityLogsSettings.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/assets/js/state/sagas/activityLogsSettings.test.js b/assets/js/state/sagas/activityLogsSettings.test.js index a46eb3e30b..1cfbe4b283 100644 --- a/assets/js/state/sagas/activityLogsSettings.test.js +++ b/assets/js/state/sagas/activityLogsSettings.test.js @@ -34,7 +34,6 @@ describe('Activity Logs Settings saga', () => { ]); }); - it.each([403, 404, 500, 502, 504])( 'should put a network error flag on failed fetching', async (status) => { From 2203b5f1e305c558d293f4285ed2101e7fca896d Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 09:36:07 +0200 Subject: [PATCH 17/25] typo --- assets/js/state/selectors/activityLogsSettings.js | 10 +--------- assets/js/state/selectors/activityLogsSettings.test.js | 1 - 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/assets/js/state/selectors/activityLogsSettings.js b/assets/js/state/selectors/activityLogsSettings.js index 4b16eea613..3c52f35c1b 100644 --- a/assets/js/state/selectors/activityLogsSettings.js +++ b/assets/js/state/selectors/activityLogsSettings.js @@ -2,20 +2,12 @@ import { createSelector } from '@reduxjs/toolkit'; export const getActivityLogsSettings = createSelector( [({ activityLogsSettings }) => activityLogsSettings], - ({ + ({ settings, errors, loading, editing, networkError }) => ({ settings, errors, loading, editing, networkError, - testingConnection, - }) => ({ - settings, - errors, - loading, - editing, - networkError, - testingConnection, }) ); diff --git a/assets/js/state/selectors/activityLogsSettings.test.js b/assets/js/state/selectors/activityLogsSettings.test.js index dc010cad54..c457134818 100644 --- a/assets/js/state/selectors/activityLogsSettings.test.js +++ b/assets/js/state/selectors/activityLogsSettings.test.js @@ -22,7 +22,6 @@ describe('Activity Logs Settings selector', () => { }, errors: null, editing: false, - testingConnection: false, }, ]; From 19d183a7a405bf90c454fde92c59c5463a5012b2 Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 09:44:16 +0200 Subject: [PATCH 18/25] typo --- assets/js/state/activityLogsSettings.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/assets/js/state/activityLogsSettings.test.js b/assets/js/state/activityLogsSettings.test.js index 800afa6578..abad3a19c6 100644 --- a/assets/js/state/activityLogsSettings.test.js +++ b/assets/js/state/activityLogsSettings.test.js @@ -27,14 +27,14 @@ describe('ActivityLogsSettings reducer', () => { const initialState = { loading: true, settings: { - retentionTime: undefined, + retention_time: undefined, }, networkError: false, errors: [], }; const settings = { - retentionTime: { value: 2, unit: 'week' }, + retention_time: { value: 2, unit: 'week' }, }; const action = setActivityLogsSettings(settings); @@ -53,7 +53,7 @@ describe('ActivityLogsSettings reducer', () => { const initialState = { loading: false, settings: { - retentionTime: { value: 2, unit: 'week' }, + retention_time: { value: 2, unit: 'week' }, }, networkError: false, errors: [], @@ -62,7 +62,7 @@ describe('ActivityLogsSettings reducer', () => { const errors = [ { detail: "can't be blank", - source: { pointer: '/retentionTime' }, + source: { pointer: '/retention_time' }, title: 'Invalid value', }, ]; @@ -74,7 +74,7 @@ describe('ActivityLogsSettings reducer', () => { expect(actual).toEqual({ loading: false, settings: { - retentionTime: { value: 2, unit: 'week' }, + retention_time: { value: 2, unit: 'week' }, }, networkError: false, errors, @@ -85,7 +85,7 @@ describe('ActivityLogsSettings reducer', () => { const initialState = { loading: false, settings: { - retentionTime: { value: 2, unit: 'week' }, + retention_time: { value: 2, unit: 'week' }, }, networkError: false, errors: [], @@ -106,7 +106,7 @@ describe('ActivityLogsSettings reducer', () => { const initialState = { loading: true, settings: { - retentionTime: { value: 2, unit: 'week' }, + retention_time: { value: 2, unit: 'week' }, }, networkError: false, errors: [], From 565ee4fe5b93c3be1939a756a9b13f3d9299cdc7 Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 10:03:04 +0200 Subject: [PATCH 19/25] refactor --- .../pages/SettingsPage/SettingsPage.test.jsx | 70 +++++-------------- 1 file changed, 17 insertions(+), 53 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.test.jsx b/assets/js/pages/SettingsPage/SettingsPage.test.jsx index e119df69c2..feb83670e1 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.test.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.test.jsx @@ -6,7 +6,7 @@ import '@testing-library/jest-dom'; import { withState, - defaultInitialState, + defaultInitialState as defaultInitialStateBase, renderWithRouter, } from '@lib/test-utils'; import { softwareUpdatesSettingsFactory } from '@lib/test-utils/factories/softwareUpdatesSettings'; @@ -17,6 +17,22 @@ import SettingsPage from './SettingsPage'; const axiosMock = new MockAdapter(networkClient); +const defaultInitialState = { + ...defaultInitialStateBase, + softwareUpdatesSettings: { + settings: { + url: undefined, + username: undefined, + ca_uploaded_at: undefined, + }, + }, + activityLogsSettings: { + settings: { + retention_time: { value: 1, unit: 'day' }, + }, + }, +}; + describe('Settings Page', () => { afterEach(() => { axiosMock.reset(); @@ -41,9 +57,6 @@ describe('Settings Page', () => { ca_uploaded_at: undefined, }, }, - activityLogsSettings: { - settings: { retention_time: { value: 1, unit: 'day' } }, - }, }); await act(async () => { @@ -70,9 +83,6 @@ describe('Settings Page', () => { ca_uploaded_at: undefined, }, }, - activityLogsSettings: { - settings: { retention_time: { value: 1, unit: 'day' } }, - }, }); renderWithRouter(StatefulSettings); @@ -94,9 +104,6 @@ describe('Settings Page', () => { }, error: null, }, - activityLogsSettings: { - settings: { retention_time: { value: 1, unit: 'day' } }, - }, }); renderWithRouter(StatefulSettings); @@ -123,9 +130,6 @@ describe('Settings Page', () => { settings, error: null, }, - activityLogsSettings: { - settings: { retention_time: { value: 1, unit: 'day' } }, - }, }); const { url, username, ca_uploaded_at } = settings; @@ -153,14 +157,6 @@ describe('Settings Page', () => { it('should render activity logs section', async () => { const [StatefulSettings] = withState(, { ...defaultInitialState, - softwareUpdatesSettings: { - loading: true, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - }, activityLogsSettings: { loading: false, settings: { retention_time: { value: 1, unit: 'day' } }, @@ -177,14 +173,6 @@ describe('Settings Page', () => { it('should render loader on activity logs section', async () => { const [StatefulSettings] = withState(, { ...defaultInitialState, - softwareUpdatesSettings: { - loading: true, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - }, activityLogsSettings: { loading: true, settings: { retention_time: { value: 1, unit: 'day' } }, @@ -203,14 +191,6 @@ describe('Settings Page', () => { it('should render edit modal', async () => { const [StatefulSettings] = withState(, { ...defaultInitialState, - softwareUpdatesSettings: { - loading: true, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - }, activityLogsSettings: { editing: true, settings: { retention_time: { value: 1, unit: 'day' } }, @@ -235,14 +215,6 @@ describe('Settings Page', () => { it('should render saving errors', async () => { const [StatefulSettings] = withState(, { ...defaultInitialState, - softwareUpdatesSettings: { - loading: true, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - }, activityLogsSettings: { editing: true, settings: { retention_time: { value: 1, unit: 'day' } }, @@ -266,14 +238,6 @@ describe('Settings Page', () => { it('should render saved configuration', async () => { const [StatefulSettings] = withState(, { ...defaultInitialState, - softwareUpdatesSettings: { - loading: true, - settings: { - url: undefined, - username: undefined, - ca_uploaded_at: undefined, - }, - }, activityLogsSettings: { settings: { retention_time: { value: 2, unit: 'day' } }, }, From 253efb20c20bd41c8c393e0c54aa35bd0be8d8ff Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 10:06:05 +0200 Subject: [PATCH 20/25] remove duplicated test --- .../js/pages/SettingsPage/SettingsPage.test.jsx | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/assets/js/pages/SettingsPage/SettingsPage.test.jsx b/assets/js/pages/SettingsPage/SettingsPage.test.jsx index feb83670e1..2e75a3f313 100644 --- a/assets/js/pages/SettingsPage/SettingsPage.test.jsx +++ b/assets/js/pages/SettingsPage/SettingsPage.test.jsx @@ -234,20 +234,5 @@ describe('Settings Page', () => { expect(screen.getByText('Invalid data provided')).toBeVisible(); }); - - it('should render saved configuration', async () => { - const [StatefulSettings] = withState(, { - ...defaultInitialState, - activityLogsSettings: { - settings: { retention_time: { value: 2, unit: 'day' } }, - }, - }); - - await act(async () => { - renderWithRouter(StatefulSettings); - }); - - expect(screen.getByText('2 days')).toBeVisible(); - }); }); }); From c918490a88ca425f5df99c9b7379e4408509559f Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 10:10:47 +0200 Subject: [PATCH 21/25] refactor test --- assets/js/state/activityLogsSettings.test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/assets/js/state/activityLogsSettings.test.js b/assets/js/state/activityLogsSettings.test.js index abad3a19c6..7b83a1f350 100644 --- a/assets/js/state/activityLogsSettings.test.js +++ b/assets/js/state/activityLogsSettings.test.js @@ -102,7 +102,7 @@ describe('ActivityLogsSettings reducer', () => { }); }); - it('should mark that the connection is being tested', () => { + it('should mark loading false on received network error', () => { const initialState = { loading: true, settings: { @@ -110,8 +110,7 @@ describe('ActivityLogsSettings reducer', () => { }, networkError: false, errors: [], - editing: false, - testingConnection: false, + editing: false }; [true, false].forEach((hasNetworkError) => { From 700c914675299b23ca9652d690272b773786a4f0 Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 13:08:44 +0200 Subject: [PATCH 22/25] refactor errors --- .../ActivityLogsSettingsModal.jsx | 40 +++++++++++++------ .../ActivityLogsSettingsModal.stories.jsx | 33 +++++++++++++-- .../ActivityLogsSettingsModal.test.jsx | 14 ++++--- assets/js/state/activityLogsSettings.test.js | 2 +- 4 files changed, 68 insertions(+), 21 deletions(-) diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx index 2ba9bbae50..822b5f6916 100644 --- a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx +++ b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx @@ -14,13 +14,19 @@ const defaultErrors = []; const timeUnitOptions = ['day', 'week', 'month', 'year']; const defaultTimeUnit = timeUnitOptions[0]; -const toErrorMessage = (errors) => +const toRetentionTimeErrorMessage = (errors) => [ capitalize(getError('retention_time/value', errors)), capitalize(getError('retention_time/unit', errors)), ] .filter(Boolean) - .join(', '); + .join('; '); + +const toGenericErrorMessage = (errors) => + // If there is at lease one error not related to a specific field, return a generic error message + errors.some((error) => !error.source) + ? 'Something went wrong while saving' + : ''; function TimeSpan({ time: initialTime, error = false, onChange = noop }) { const [time, setTime] = useState(initialTime); @@ -58,6 +64,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 ? ( +

{text}

+ ) : ( +

 

+ ); +} + /** * Modal to edit Activity Logs settings */ @@ -72,7 +91,8 @@ function ActivityLogsSettingsModal({ }) { const [retentionTime, setRetentionTime] = useState(initialRetentionTime); - const errorMessage = toErrorMessage(errors); + const retentionTimeError = toRetentionTimeErrorMessage(errors); + const genericError = toGenericErrorMessage(errors); return ( @@ -83,20 +103,13 @@ function ActivityLogsSettingsModal({
{ setRetentionTime(time); onClearErrors(); }} /> - {errorMessage && ( -

- {errorMessage} -

- )} +

@@ -119,6 +132,9 @@ function ActivityLogsSettingsModal({ Cancel +

+ +
); } diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx index 90760212d8..547ada2041 100644 --- a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.stories.jsx +++ b/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'], + }, +}; diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx index 4f05f516ea..98d3c2d22f 100644 --- a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx +++ b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx @@ -118,13 +118,17 @@ describe('ActivityLogsSettingsModal component', () => { source: { pointer: '/retention_time/unit' }, title: faker.lorem.words(10), }; + const anyPayloadError = { foo: 'bar' }; 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} + 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'} | ${[anyPayloadError]} | ${'Something went wrong while saving'} + ${'generic error and value error (1)'} | ${[anyPayloadError, valueError]} | ${'Something went wrong while saving'} + ${'generic error and value error (2)'} | ${[anyPayloadError, valueError]} | ${valueError.detail} `( 'should display errors on $scenario', async ({ errors, expectedErrorMessage }) => { diff --git a/assets/js/state/activityLogsSettings.test.js b/assets/js/state/activityLogsSettings.test.js index 7b83a1f350..3bdc0c9e38 100644 --- a/assets/js/state/activityLogsSettings.test.js +++ b/assets/js/state/activityLogsSettings.test.js @@ -110,7 +110,7 @@ describe('ActivityLogsSettings reducer', () => { }, networkError: false, errors: [], - editing: false + editing: false, }; [true, false].forEach((hasNetworkError) => { From c05896150c36569ffe9f3e9c1612de0fb920124b Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 13:14:21 +0200 Subject: [PATCH 23/25] remove unused selectors --- .../state/selectors/activityLogsSettings.js | 11 --- .../selectors/activityLogsSettings.test.js | 86 +------------------ 2 files changed, 1 insertion(+), 96 deletions(-) diff --git a/assets/js/state/selectors/activityLogsSettings.js b/assets/js/state/selectors/activityLogsSettings.js index 3c52f35c1b..3e97eda7c1 100644 --- a/assets/js/state/selectors/activityLogsSettings.js +++ b/assets/js/state/selectors/activityLogsSettings.js @@ -11,17 +11,6 @@ export const getActivityLogsSettings = createSelector( }) ); -export const getActivityLogsSettingsLoading = createSelector( - [getActivityLogsSettings], - ({ loading }) => loading -); - -export const getActivityLogsSettingsSaved = createSelector( - [getActivityLogsSettings], - ({ settings: { retention_time } }) => - !!(retention_time && retention_time.value && retention_time.unit) -); - export const getActivityLogsSettingsErrors = createSelector( [getActivityLogsSettings], ({ errors }) => errors diff --git a/assets/js/state/selectors/activityLogsSettings.test.js b/assets/js/state/selectors/activityLogsSettings.test.js index c457134818..fb00ffdb87 100644 --- a/assets/js/state/selectors/activityLogsSettings.test.js +++ b/assets/js/state/selectors/activityLogsSettings.test.js @@ -1,8 +1,4 @@ -import { - getActivityLogsSettings, - getActivityLogsSettingsLoading, - getActivityLogsSettingsSaved, -} from './activityLogsSettings'; +import { getActivityLogsSettings } from './activityLogsSettings'; describe('Activity Logs Settings selector', () => { describe('get activity logs settings', () => { @@ -34,84 +30,4 @@ describe('Activity Logs Settings selector', () => { } ); }); - - describe('get activity logs settings loading', () => { - const scenarios = [ - { - state: { loading: false }, - result: false, - }, - { - state: { loading: true }, - result: true, - }, - ]; - - it.each(scenarios)( - 'should return if the activity logs settings are loading', - ({ state, result }) => { - expect( - getActivityLogsSettingsLoading({ activityLogsSettings: state }) - ).toEqual(result); - } - ); - }); - - describe('get if activity logs settings saved', () => { - const scenarios = [ - { - state: { - loading: false, - settings: { - retention_time: { value: 2, unit: 'week' }, - }, - errors: null, - editing: false, - }, - result: true, - }, - { - state: { - loading: false, - settings: { - retention_time: undefined, - }, - errors: null, - editing: false, - }, - result: false, - }, - { - state: { - loading: false, - settings: { - retention_time: {} /* invalid */, - }, - errors: null, - editing: false, - }, - result: false, - }, - { - state: { - loading: true, - settings: { - retention_time: undefined, - }, - errors: null, - editing: false, - }, - result: false, - }, - ]; - - it.each(scenarios)( - 'should return if the activity logs settings are saved', - ({ state, result }) => { - expect( - getActivityLogsSettingsSaved({ activityLogsSettings: state }) - ).toEqual(result); - } - ); - }); }); From 69a2e23acf4888c40f1785b88671bf8c6be1a8b9 Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 13:56:13 +0200 Subject: [PATCH 24/25] fix error --- assets/js/state/sagas/activityLogsSettings.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/assets/js/state/sagas/activityLogsSettings.js b/assets/js/state/sagas/activityLogsSettings.js index 42b1498865..ee61ca613e 100644 --- a/assets/js/state/sagas/activityLogsSettings.js +++ b/assets/js/state/sagas/activityLogsSettings.js @@ -19,10 +19,7 @@ export function* fetchActivityLogsSettings() { const response = yield call(getSettings); yield put(setActivityLogsSettings(response.data)); } catch (error) { - const errorCode = get(error, ['response', 'status']); - if (errorCode !== 403) { - yield put(setNetworkError(true)); - } + yield put(setNetworkError(true)); } } From 52303cac574bda7f2b54edbb8574cd63f3089ca2 Mon Sep 17 00:00:00 2001 From: balanza Date: Tue, 2 Jul 2024 16:04:40 +0200 Subject: [PATCH 25/25] fixes --- .../ActivityLogsSettingsModal.jsx | 8 +++---- .../ActivityLogsSettingsModal.test.jsx | 17 +++++++-------- assets/js/state/sagas/activityLogsSettings.js | 10 ++++++--- .../state/sagas/activityLogsSettings.test.js | 21 ++++++++++++++++++- 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx index 822b5f6916..ee6a157b38 100644 --- a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx +++ b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.jsx @@ -23,10 +23,8 @@ const toRetentionTimeErrorMessage = (errors) => .join('; '); const toGenericErrorMessage = (errors) => - // If there is at lease one error not related to a specific field, return a generic error message - errors.some((error) => !error.source) - ? 'Something went wrong while saving' - : ''; + // 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); @@ -38,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 }; diff --git a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx index 98d3c2d22f..3aa6553430 100644 --- a/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx +++ b/assets/js/common/ActivityLogsSettingsModal/ActivityLogsSettingsModal.test.jsx @@ -118,17 +118,16 @@ describe('ActivityLogsSettingsModal component', () => { source: { pointer: '/retention_time/unit' }, title: faker.lorem.words(10), }; - const anyPayloadError = { foo: 'bar' }; 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'} | ${[anyPayloadError]} | ${'Something went wrong while saving'} - ${'generic error and value error (1)'} | ${[anyPayloadError, valueError]} | ${'Something went wrong while saving'} - ${'generic error and value error (2)'} | ${[anyPayloadError, valueError]} | ${valueError.detail} + 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 }) => { diff --git a/assets/js/state/sagas/activityLogsSettings.js b/assets/js/state/sagas/activityLogsSettings.js index ee61ca613e..6b9e0d5e25 100644 --- a/assets/js/state/sagas/activityLogsSettings.js +++ b/assets/js/state/sagas/activityLogsSettings.js @@ -1,5 +1,5 @@ import { get } from 'lodash'; -import { put, call, takeEvery, debounce } from 'redux-saga/effects'; +import { put, call, takeEvery } from 'redux-saga/effects'; import { getSettings, updateSettings } from '@lib/api/activityLogsSettings'; import { @@ -31,12 +31,16 @@ export function* updateActivityLogsSettings({ payload }) { yield put(setEditingActivityLogsSettings(false)); yield put(setActivityLogsSettingsErrors([])); } catch (error) { - const errors = get(error, ['response', 'data', 'errors'], []); + const errors = get( + error, + ['response', 'data', 'errors'], + ['An error occurred while saving the settings'] + ); yield put(setActivityLogsSettingsErrors(errors)); } } export function* watchActivityLogsSettings() { - yield debounce(1000, FETCH_ACTIVITY_LOGS_SETTINGS, fetchActivityLogsSettings); + yield takeEvery(FETCH_ACTIVITY_LOGS_SETTINGS, fetchActivityLogsSettings); yield takeEvery(UPDATE_ACTIVITY_LOGS_SETTINGS, updateActivityLogsSettings); } diff --git a/assets/js/state/sagas/activityLogsSettings.test.js b/assets/js/state/sagas/activityLogsSettings.test.js index 1cfbe4b283..32dd163e05 100644 --- a/assets/js/state/sagas/activityLogsSettings.test.js +++ b/assets/js/state/sagas/activityLogsSettings.test.js @@ -38,7 +38,7 @@ describe('Activity Logs Settings saga', () => { 'should put a network error flag on failed fetching', async (status) => { const axiosMock = new MockAdapter(networkClient); - axiosMock.onGet('/settings/activity_logs').reply(status); + axiosMock.onGet('/settings/activity_log').reply(status); const dispatched = await recordSaga(fetchActivityLogsSettings); @@ -92,5 +92,24 @@ describe('Activity Logs Settings saga', () => { setActivityLogsSettingsErrors(errors), ]); }); + + it.each([403, 404, 500, 502, 504])( + 'should put a network error flag on failed saving', + async (status) => { + const axiosMock = new MockAdapter(networkClient); + axiosMock.onPut('/settings/activity_log').reply(status); + + const payload = activityLogsSettingsFactory.build(); + + const dispatched = await recordSaga(updateActivityLogsSettings, { + payload, + }); + + expect(dispatched).toEqual([ + startLoadingActivityLogsSettings(), + setActivityLogsSettingsErrors([expect.any(String)]), + ]); + } + ); }); });