From 39cf8a058872ef4d2ca0db8c95e53bd81a836344 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Tue, 18 Jul 2023 14:54:58 +0200 Subject: [PATCH 1/9] Add getClusterCheckSelection selector on top of the centralized checks selection state --- assets/js/state/selectors/checksSelection.js | 7 ++- .../state/selectors/checksSelection.test.js | 43 ++++++++++++++++--- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/assets/js/state/selectors/checksSelection.js b/assets/js/state/selectors/checksSelection.js index d14f6450df..bf008c851c 100644 --- a/assets/js/state/selectors/checksSelection.js +++ b/assets/js/state/selectors/checksSelection.js @@ -1,6 +1,11 @@ -import { TARGET_HOST } from '@lib/model'; +import { TARGET_HOST, TARGET_CLUSTER } from '@lib/model'; export const getHostCheckSelection = (hostID) => ({ checksSelection }) => checksSelection?.[TARGET_HOST][hostID] || {}; + +export const getClusterCheckSelection = + (clusterID) => + ({ checksSelection }) => + checksSelection?.[TARGET_CLUSTER][clusterID] || {}; diff --git a/assets/js/state/selectors/checksSelection.test.js b/assets/js/state/selectors/checksSelection.test.js index 00182079c6..8b4ab3b7af 100644 --- a/assets/js/state/selectors/checksSelection.test.js +++ b/assets/js/state/selectors/checksSelection.test.js @@ -1,5 +1,8 @@ import { faker } from '@faker-js/faker'; -import { getHostCheckSelection } from './checksSelection'; +import { + getHostCheckSelection, + getClusterCheckSelection, +} from './checksSelection'; describe('Checks Selection selector', () => { it(`should get a host's check selection state`, () => { @@ -8,12 +11,12 @@ describe('Checks Selection selector', () => { getHostCheckSelection(hostID)({ checksSelection: { host: { - [faker.datatype.uuid()]: { status: 'successfully_saved' }, - [hostID]: { status: 'saving' }, + [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, + [hostID]: { status: 'SAVING' }, }, }, }) - ).toEqual({ status: 'saving' }); + ).toEqual({ status: 'SAVING' }); }); it(`should not get a host's check selection state if not present`, () => { @@ -22,8 +25,36 @@ describe('Checks Selection selector', () => { getHostCheckSelection(hostWithoutSelection)({ checksSelection: { host: { - [faker.datatype.uuid()]: { status: 'successfully_saved' }, - [faker.datatype.uuid()]: { status: 'saving' }, + [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, + [faker.datatype.uuid()]: { status: 'SAVING' }, + }, + }, + }) + ).toEqual({}); + }); + + it(`should get a cluster's check selection state`, () => { + const clusterID = faker.datatype.uuid(); + expect( + getClusterCheckSelection(clusterID)({ + checksSelection: { + cluster: { + [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, + [clusterID]: { status: 'SAVING' }, + }, + }, + }) + ).toEqual({ status: 'SAVING' }); + }); + + it(`should not get a cluster's check selection state if not present`, () => { + const clustertWithoutSelection = faker.datatype.uuid(); + expect( + getClusterCheckSelection(clustertWithoutSelection)({ + checksSelection: { + cluster: { + [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, + [faker.datatype.uuid()]: { status: 'SAVING' }, }, }, }) From b5aa3a343782c94880ca1fc054f99f15494b592f Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Tue, 18 Jul 2023 14:56:31 +0200 Subject: [PATCH 2/9] Add cluster checks selection saga to centralized checks selection --- assets/js/state/sagas/checksSelection.js | 34 ++++++++- assets/js/state/sagas/checksSelection.test.js | 69 ++++++++++++++++++- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/assets/js/state/sagas/checksSelection.js b/assets/js/state/sagas/checksSelection.js index 2a9cb8ff01..cd60b3c576 100644 --- a/assets/js/state/sagas/checksSelection.js +++ b/assets/js/state/sagas/checksSelection.js @@ -1,18 +1,20 @@ import { put, call, takeEvery } from 'redux-saga/effects'; import { post } from '@lib/network'; -import { TARGET_HOST } from '@lib/model'; +import { TARGET_HOST, TARGET_CLUSTER } from '@lib/model'; import { notify } from '@state/actions/notifications'; import { HOST_CHECKS_SELECTED, + CLUSTER_CHECKS_SELECTED, startSavingChecksSelection, setSavingSuccessful, setSavingFailed, } from '@state/checksSelection'; import { updateSelectedChecks as updateHostSelectedChecks } from '@state/hosts'; +import { updateSelectedChecks as updateClusterSelectedChecks } from '@state/clusters'; function* saveHostChecksSelection({ hostID, checks }) { yield call(post, `/hosts/${hostID}/checks`, { @@ -26,6 +28,18 @@ function* saveHostChecksSelection({ hostID, checks }) { ); } +function* saveClusterChecksSelection({ clusterID, checks }) { + yield call(post, `/clusters/${clusterID}/checks`, { + checks, + }); + yield put( + updateClusterSelectedChecks({ + clusterID, + checks, + }) + ); +} + function* saveChecksSelection(targetID, targetType, checks) { switch (targetType) { case TARGET_HOST: @@ -34,6 +48,12 @@ function* saveChecksSelection(targetID, targetType, checks) { checks, }); break; + case TARGET_CLUSTER: + yield saveClusterChecksSelection({ + clusterID: targetID, + checks, + }); + break; default: } } @@ -71,6 +91,18 @@ export function* selectHostChecks({ payload: { hostID, hostName, checks } }) { }); } +export function* selectClusterChecks({ + payload: { clusterID, clusterName, checks }, +}) { + yield checksSelected({ + targetID: clusterID, + targetType: TARGET_CLUSTER, + targetName: clusterName, + checks, + }); +} + export function* watchChecksSelection() { yield takeEvery(HOST_CHECKS_SELECTED, selectHostChecks); + yield takeEvery(CLUSTER_CHECKS_SELECTED, selectClusterChecks); } diff --git a/assets/js/state/sagas/checksSelection.test.js b/assets/js/state/sagas/checksSelection.test.js index 93ec26667a..d5b9117228 100644 --- a/assets/js/state/sagas/checksSelection.test.js +++ b/assets/js/state/sagas/checksSelection.test.js @@ -4,9 +4,10 @@ import { recordSaga } from '@lib/test-utils'; import { networkClient } from '@lib/network'; import MockAdapter from 'axios-mock-adapter'; -import { hostFactory } from '@lib/test-utils/factories'; +import { hostFactory, clusterFactory } from '@lib/test-utils/factories'; import { updateSelectedChecks as updateHostSelectedChecks } from '@state/hosts'; +import { updateSelectedChecks as updateClusterSelectedChecks } from '@state/clusters'; import { startSavingChecksSelection, @@ -16,7 +17,7 @@ import { import { notify } from '@state/actions/notifications'; -import { selectHostChecks } from './checksSelection'; +import { selectHostChecks, selectClusterChecks } from './checksSelection'; const axiosMock = new MockAdapter(networkClient); @@ -84,4 +85,68 @@ describe('Checks Selection saga', () => { ]); }); }); + + describe('Cluster Checks Selection', () => { + it('should successfully save check selection for a cluster', async () => { + const { id: clusterID, name: clusterName } = clusterFactory.build(); + const checks = [faker.datatype.uuid(), faker.datatype.uuid()]; + + axiosMock.onPost(`/clusters/${clusterID}/checks`).reply(202, {}); + + const dispatched = await recordSaga(selectClusterChecks, { + payload: { + clusterID, + clusterName, + checks, + }, + }); + + const payload = { + targetID: clusterID, + targetType: 'cluster', + }; + + expect(dispatched).toEqual([ + startSavingChecksSelection(payload), + updateClusterSelectedChecks({ + clusterID, + checks, + }), + setSavingSuccessful(payload), + notify({ + text: `Checks selection for ${clusterName} saved`, + icon: '💾', + }), + ]); + }); + + it('should not save check selection for a cluster on request failure', async () => { + const { id: clusterID, name: clusterName } = clusterFactory.build(); + const checks = [faker.datatype.uuid(), faker.datatype.uuid()]; + + axiosMock.onPost(`/clusters/${clusterID}/checks`).reply(400, {}); + + const dispatched = await recordSaga(selectClusterChecks, { + payload: { + clusterID, + clusterName, + checks, + }, + }); + + const payload = { + targetID: clusterID, + targetType: 'cluster', + }; + + expect(dispatched).toEqual([ + startSavingChecksSelection(payload), + setSavingFailed(payload), + notify({ + text: `Unable to save selection for ${clusterName}`, + icon: '❌', + }), + ]); + }); + }); }); From ca65ec9123ff0374dd0a33f4d52972fbbb034949 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Tue, 18 Jul 2023 15:00:30 +0200 Subject: [PATCH 3/9] Remove legacy cluster and host checks selections sagas --- assets/js/state/sagas/clusters.js | 51 +---------------- assets/js/state/sagas/clusters.test.js | 79 +------------------------- assets/js/state/sagas/hosts.js | 40 +------------ assets/js/state/sagas/hosts.test.js | 57 ------------------- 4 files changed, 5 insertions(+), 222 deletions(-) diff --git a/assets/js/state/sagas/clusters.js b/assets/js/state/sagas/clusters.js index ae2bcf8e50..083e2d4eb8 100644 --- a/assets/js/state/sagas/clusters.js +++ b/assets/js/state/sagas/clusters.js @@ -1,50 +1,7 @@ -import { call, put, select, takeEvery } from 'redux-saga/effects'; -import { post } from '@lib/network'; +import { put, takeEvery } from 'redux-saga/effects'; import { notify } from '@state/actions/notifications'; -import { - CLUSTER_DEREGISTERED, - CLUSTER_CHECKS_SELECTED, - removeCluster, - updateSelectedChecks, -} from '@state/clusters'; -import { - setClusterChecksSelectionSavingError, - setClusterChecksSelectionSavingSuccess, - startSavingClusterChecksSelection, - stopSavingClusterChecksSelection, -} from '@state/clusterChecksSelection'; -import { getClusterName } from '@state/selectors/cluster'; - -export function* checksSelected({ payload }) { - yield put(startSavingClusterChecksSelection()); - - const clusterName = yield select(getClusterName(payload.clusterID)); - - try { - yield call(post, `/clusters/${payload.clusterID}/checks`, { - checks: payload.checks, - }); - yield put(updateSelectedChecks(payload)); - - yield put( - notify({ - text: `Checks selection for ${clusterName} saved`, - icon: '💾', - }) - ); - yield put(setClusterChecksSelectionSavingSuccess()); - } catch (error) { - yield put( - notify({ - text: `Unable to save selection for ${clusterName}`, - icon: '❌', - }) - ); - yield put(setClusterChecksSelectionSavingError()); - } - yield put(stopSavingClusterChecksSelection()); -} +import { CLUSTER_DEREGISTERED, removeCluster } from '@state/clusters'; export function* clusterDeregistered({ payload: { name, id } }) { yield put(removeCluster({ id })); @@ -56,10 +13,6 @@ export function* clusterDeregistered({ payload: { name, id } }) { ); } -export function* watchClusterChecksSelection() { - yield takeEvery(CLUSTER_CHECKS_SELECTED, checksSelected); -} - export function* watchClusterDeregistered() { yield takeEvery(CLUSTER_DEREGISTERED, clusterDeregistered); } diff --git a/assets/js/state/sagas/clusters.test.js b/assets/js/state/sagas/clusters.test.js index ad9745279f..26127c6b38 100644 --- a/assets/js/state/sagas/clusters.test.js +++ b/assets/js/state/sagas/clusters.test.js @@ -1,23 +1,9 @@ -import { faker } from '@faker-js/faker'; -import MockAdapter from 'axios-mock-adapter'; import { recordSaga } from '@lib/test-utils'; -import { networkClient } from '@lib/network'; - import { clusterFactory } from '@lib/test-utils/factories'; -import { notify } from '@state/actions/notifications'; -import { clusterDeregistered, checksSelected } from '@state/sagas/clusters'; -import { removeCluster, updateSelectedChecks } from '@state/clusters'; - -import { - setClusterChecksSelectionSavingError, - setClusterChecksSelectionSavingSuccess, - startSavingClusterChecksSelection, - stopSavingClusterChecksSelection, -} from '@state/clusterChecksSelection'; - -const axiosMock = new MockAdapter(networkClient); +import { clusterDeregistered } from '@state/sagas/clusters'; +import { removeCluster } from '@state/clusters'; describe('Clusters sagas', () => { it('should remove the cluster', async () => { @@ -29,65 +15,4 @@ describe('Clusters sagas', () => { expect(dispatched).toContainEqual(removeCluster({ id })); }); - - it('should save check selection for a cluster', async () => { - const cluster = clusterFactory.build(); - - axiosMock.onPost(`/clusters/${cluster.id}/checks`).reply(202, {}); - - const actionPayload = { - clusterID: cluster.id, - checks: [faker.datatype.uuid(), faker.datatype.uuid()], - }; - const dispatched = await recordSaga( - checksSelected, - { - payload: actionPayload, - }, - { - clustersList: { clusters: [cluster] }, - } - ); - - expect(dispatched).toEqual([ - startSavingClusterChecksSelection(), - updateSelectedChecks(actionPayload), - notify({ - text: `Checks selection for ${cluster.name} saved`, - icon: '💾', - }), - setClusterChecksSelectionSavingSuccess(), - stopSavingClusterChecksSelection(), - ]); - }); - - it('should notify an error when saving a cluster check selection fails', async () => { - const cluster = clusterFactory.build(); - - axiosMock.onPost(`/clusters/${cluster.id}/checks`).reply(400, {}); - - const actionPayload = { - clusterID: cluster.id, - checks: [faker.datatype.uuid(), faker.datatype.uuid()], - }; - const dispatched = await recordSaga( - checksSelected, - { - payload: actionPayload, - }, - { - clustersList: { clusters: [cluster] }, - } - ); - - expect(dispatched).toEqual([ - startSavingClusterChecksSelection(), - notify({ - text: `Unable to save selection for ${cluster.name}`, - icon: '❌', - }), - setClusterChecksSelectionSavingError(), - stopSavingClusterChecksSelection(), - ]); - }); }); diff --git a/assets/js/state/sagas/hosts.js b/assets/js/state/sagas/hosts.js index bff6729895..b00b625c44 100644 --- a/assets/js/state/sagas/hosts.js +++ b/assets/js/state/sagas/hosts.js @@ -1,5 +1,5 @@ import { delay, put, race, call, take, takeEvery } from 'redux-saga/effects'; -import { post, del } from '@lib/network'; +import { del } from '@lib/network'; import { CHECK_HOST_IS_DEREGISTERABLE, @@ -10,15 +10,8 @@ import { setHostListDeregisterable, setHostDeregistering, setHostNotDeregistering, - updateSelectedChecks, } from '@state/hosts'; -import { - startSavingChecksSelection, - stopSavingChecksSelection, - HOST_CHECKS_SELECTED, -} from '@state/hostChecksSelection'; - import { notify } from '@state/actions/notifications'; export function* markDeregisterableHosts(hosts) { @@ -72,33 +65,6 @@ export function* deregisterHost({ payload }) { } } -export function* checksSelected({ payload }) { - const { hostID, hostName, checks } = payload; - yield put(startSavingChecksSelection()); - - try { - yield call(post, `/hosts/${hostID}/checks`, { - checks, - }); - - yield put(updateSelectedChecks(payload)); - yield put( - notify({ - text: `Checks selection for ${hostName} saved`, - icon: '💾', - }) - ); - } catch (error) { - yield put( - notify({ - text: `Unable to save selection for ${hostName}`, - icon: '❌', - }) - ); - } - yield put(stopSavingChecksSelection()); -} - export function* watchHostDeregisterable() { yield takeEvery(CHECK_HOST_IS_DEREGISTERABLE, checkHostDeregisterable); } @@ -110,7 +76,3 @@ export function* watchHostDeregistered() { export function* watchDeregisterHost() { yield takeEvery(DEREGISTER_HOST, deregisterHost); } - -export function* watchHostChecksSelection() { - yield takeEvery(HOST_CHECKS_SELECTED, checksSelected); -} diff --git a/assets/js/state/sagas/hosts.test.js b/assets/js/state/sagas/hosts.test.js index 74fe1c3b5e..c3fab6e42a 100644 --- a/assets/js/state/sagas/hosts.test.js +++ b/assets/js/state/sagas/hosts.test.js @@ -1,4 +1,3 @@ -import { faker } from '@faker-js/faker'; import MockAdapter from 'axios-mock-adapter'; import { recordSaga } from '@lib/test-utils'; @@ -8,7 +7,6 @@ import { checkHostDeregisterable, hostDeregistered, deregisterHost, - checksSelected, } from '@state/sagas/hosts'; import { @@ -17,14 +15,8 @@ import { removeHost, setHostDeregistering, setHostNotDeregistering, - updateSelectedChecks, } from '@state/hosts'; -import { - startSavingChecksSelection, - stopSavingChecksSelection, -} from '@state/hostChecksSelection'; - import { networkClient } from '@lib/network'; import { notify } from '@state/actions/notifications'; import { hostFactory } from '@lib/test-utils/factories'; @@ -122,53 +114,4 @@ describe('Hosts sagas', () => { setHostNotDeregistering(host), ]); }); - - it('should save check selection for a host', async () => { - const host = hostFactory.build(); - - axiosMock.onPost(`/hosts/${host.id}/checks`).reply(202, {}); - - const actionPayload = { - hostID: host.id, - hostName: host.hostname, - checks: [faker.datatype.uuid(), faker.datatype.uuid()], - }; - const dispatched = await recordSaga(checksSelected, { - payload: actionPayload, - }); - - expect(dispatched).toEqual([ - startSavingChecksSelection(), - updateSelectedChecks(actionPayload), - notify({ - text: `Checks selection for ${host.hostname} saved`, - icon: '💾', - }), - stopSavingChecksSelection(), - ]); - }); - - it('should notify an error when saving check selection fails', async () => { - const host = hostFactory.build(); - - axiosMock.onPost(`/hosts/${host.id}/checks`).reply(400, {}); - - const actionPayload = { - hostID: host.id, - hostName: host.hostname, - checks: [faker.datatype.uuid(), faker.datatype.uuid()], - }; - const dispatched = await recordSaga(checksSelected, { - payload: actionPayload, - }); - - expect(dispatched).toEqual([ - startSavingChecksSelection(), - notify({ - text: `Unable to save selection for ${host.hostname}`, - icon: '❌', - }), - stopSavingChecksSelection(), - ]); - }); }); From d1d679390f99a85bb74c4284edccb2124262d919 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Tue, 18 Jul 2023 15:01:28 +0200 Subject: [PATCH 4/9] Register the new centralized checks selection slice and saga --- assets/js/lib/test-utils/index.jsx | 2 -- assets/js/state/index.js | 6 ++---- assets/js/state/sagas/index.js | 10 +++------- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/assets/js/lib/test-utils/index.jsx b/assets/js/lib/test-utils/index.jsx index 6c8a344e38..2d4b3c3194 100644 --- a/assets/js/lib/test-utils/index.jsx +++ b/assets/js/lib/test-utils/index.jsx @@ -33,8 +33,6 @@ export const defaultInitialState = { (database) => database.database_instances ), }, - clusterChecksSelection: {}, - hostChecksSelection: {}, checksSelection: { host: {}, cluster: {} }, catalog: { loading: false, data: [], error: null }, }; diff --git a/assets/js/state/index.js b/assets/js/state/index.js index cc4ab7802b..b9e3b7c1e5 100644 --- a/assets/js/state/index.js +++ b/assets/js/state/index.js @@ -4,8 +4,7 @@ import createSagaMiddleware from 'redux-saga'; import sapSystemsHealthSummaryReducer from './healthSummary'; import hostsListReducer from './hosts'; import clustersListReducer from './clusters'; -import clusterChecksSelectionReducer from './clusterChecksSelection'; -import hostChecksSelectionReducer from './hostChecksSelection'; +import checksSelectionReducer from './checksSelection'; import checksResultsFiltersReducer from './checksResultsFilters'; import sapSystemListReducer from './sapSystems'; import databasesListReducer from './databases'; @@ -23,8 +22,7 @@ export const store = configureStore({ sapSystemsHealthSummary: sapSystemsHealthSummaryReducer, hostsList: hostsListReducer, clustersList: clustersListReducer, - clusterChecksSelection: clusterChecksSelectionReducer, - hostChecksSelection: hostChecksSelectionReducer, + checksSelection: checksSelectionReducer, checksResultsFilters: checksResultsFiltersReducer, sapSystemsList: sapSystemListReducer, databasesList: databasesListReducer, diff --git a/assets/js/state/sagas/index.js b/assets/js/state/sagas/index.js index e9c898639d..6e38041fd4 100644 --- a/assets/js/state/sagas/index.js +++ b/assets/js/state/sagas/index.js @@ -70,17 +70,14 @@ import { watchHostDeregistered, watchHostDeregisterable, watchDeregisterHost, - watchHostChecksSelection, } from '@state/sagas/hosts'; -import { - watchClusterDeregistered, - watchClusterChecksSelection, -} from '@state/sagas/clusters'; +import { watchClusterDeregistered } from '@state/sagas/clusters'; import { watchUpdateLastExecution, watchRequestExecution, } from '@state/sagas/lastExecutions'; import { watchPerformLogin } from '@state/sagas/user'; +import { watchChecksSelection } from '@state/sagas/checksSelection'; import { getClusterName } from '@state/selectors/cluster'; @@ -376,8 +373,7 @@ export default function* rootSaga() { watchClusterCibLastWrittenUpdated(), watchClusterDeregistered(), watchNotifications(), - watchClusterChecksSelection(), - watchHostChecksSelection(), + watchChecksSelection(), watchChecksExecutionStarted(), watchChecksExecutionCompleted(), watchChecksResultsUpdated(), From c8cf478bea87d9078c6c8a397ab797568b8d53f9 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Tue, 18 Jul 2023 15:02:14 +0200 Subject: [PATCH 5/9] Remove legacy host and cluster state/selectors --- assets/js/state/clusterChecksSelection.js | 38 ------------------- assets/js/state/hostChecksSelection.js | 26 ------------- assets/js/state/hostChecksSelection.test.js | 38 ------------------- .../js/state/selectors/hostChecksSelection.js | 4 -- .../selectors/hostChecksSelection.test.js | 15 -------- 5 files changed, 121 deletions(-) delete mode 100644 assets/js/state/clusterChecksSelection.js delete mode 100644 assets/js/state/hostChecksSelection.js delete mode 100644 assets/js/state/hostChecksSelection.test.js delete mode 100644 assets/js/state/selectors/hostChecksSelection.js delete mode 100644 assets/js/state/selectors/hostChecksSelection.test.js diff --git a/assets/js/state/clusterChecksSelection.js b/assets/js/state/clusterChecksSelection.js deleted file mode 100644 index 0c1a0b6894..0000000000 --- a/assets/js/state/clusterChecksSelection.js +++ /dev/null @@ -1,38 +0,0 @@ -import { createSlice } from '@reduxjs/toolkit'; - -const initialState = { - saving: false, - savingError: null, - savingSuccess: null, -}; - -export const clusterChecksSelectionSlice = createSlice({ - name: 'clusterChecksSelection', - initialState, - reducers: { - startSavingClusterChecksSelection: (state) => { - state.saving = true; - state.savingError = null; - state.savingSuccess = null; - }, - stopSavingClusterChecksSelection: (state) => { - state.saving = false; - }, - setClusterChecksSelectionSavingError: (state) => { - state.savingError = - 'An unexpected error happened while selecting your desired checks'; - }, - setClusterChecksSelectionSavingSuccess: (state) => { - state.savingSuccess = true; - }, - }, -}); - -export const { - startSavingClusterChecksSelection, - stopSavingClusterChecksSelection, - setClusterChecksSelectionSavingError, - setClusterChecksSelectionSavingSuccess, -} = clusterChecksSelectionSlice.actions; - -export default clusterChecksSelectionSlice.reducer; diff --git a/assets/js/state/hostChecksSelection.js b/assets/js/state/hostChecksSelection.js deleted file mode 100644 index 2e206611e7..0000000000 --- a/assets/js/state/hostChecksSelection.js +++ /dev/null @@ -1,26 +0,0 @@ -import { createAction, createSlice } from '@reduxjs/toolkit'; - -const initialState = { - saving: false, -}; - -export const hostChecksSelectionSlice = createSlice({ - name: 'hostChecksSelection', - initialState, - reducers: { - startSavingChecksSelection: (state) => { - state.saving = true; - }, - stopSavingChecksSelection: (state) => { - state.saving = false; - }, - }, -}); - -export const HOST_CHECKS_SELECTED = 'HOST_CHECKS_SELECTED'; -export const checksSelected = createAction(HOST_CHECKS_SELECTED); - -export const { startSavingChecksSelection, stopSavingChecksSelection } = - hostChecksSelectionSlice.actions; - -export default hostChecksSelectionSlice.reducer; diff --git a/assets/js/state/hostChecksSelection.test.js b/assets/js/state/hostChecksSelection.test.js deleted file mode 100644 index 176f2f1a19..0000000000 --- a/assets/js/state/hostChecksSelection.test.js +++ /dev/null @@ -1,38 +0,0 @@ -import hostChecksSelectionReducer, { - startSavingChecksSelection, - stopSavingChecksSelection, -} from '@state/hostChecksSelection'; - -describe('Checks Selection reducer', () => { - it('should mark a check selection as saving', () => { - const initialState = { - saving: false, - }; - - const action = startSavingChecksSelection(); - - const expectedState = { - saving: true, - }; - - expect(hostChecksSelectionReducer(initialState, action)).toEqual( - expectedState - ); - }); - - it('should mark a check selection as completed', () => { - const initialState = { - saving: true, - }; - - const action = stopSavingChecksSelection(); - - const expectedState = { - saving: false, - }; - - expect(hostChecksSelectionReducer(initialState, action)).toEqual( - expectedState - ); - }); -}); diff --git a/assets/js/state/selectors/hostChecksSelection.js b/assets/js/state/selectors/hostChecksSelection.js deleted file mode 100644 index d70b114a13..0000000000 --- a/assets/js/state/selectors/hostChecksSelection.js +++ /dev/null @@ -1,4 +0,0 @@ -export const getCheckSelection = - () => - ({ hostChecksSelection }) => - hostChecksSelection; diff --git a/assets/js/state/selectors/hostChecksSelection.test.js b/assets/js/state/selectors/hostChecksSelection.test.js deleted file mode 100644 index 2c20f6b96d..0000000000 --- a/assets/js/state/selectors/hostChecksSelection.test.js +++ /dev/null @@ -1,15 +0,0 @@ -import { getCheckSelection } from './hostChecksSelection'; - -describe('Checks Selection selector', () => { - it('should get the state of the check selection', () => { - expect( - getCheckSelection()({ - hostChecksSelection: { - saving: false, - }, - }) - ).toEqual({ - saving: false, - }); - }); -}); From 180bc6dd438c6190e0bdfd0e133ec98f0609dcfb Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Tue, 18 Jul 2023 15:04:37 +0200 Subject: [PATCH 6/9] Use the new check selection in HostSettingsPage --- .../HostDetails/HostSettingsPage.jsx | 18 ++++++++++-------- assets/js/state/checksSelection.js | 4 ++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/assets/js/components/HostDetails/HostSettingsPage.jsx b/assets/js/components/HostDetails/HostSettingsPage.jsx index 85a2174750..7a3ca84f1b 100644 --- a/assets/js/components/HostDetails/HostSettingsPage.jsx +++ b/assets/js/components/HostDetails/HostSettingsPage.jsx @@ -4,11 +4,13 @@ import { useParams } from 'react-router-dom'; import LoadingBox from '@components/LoadingBox'; -import { checksSelected } from '@state/hostChecksSelection'; +import { TARGET_HOST } from '@lib/model'; + +import { hostChecksSelected, isSaving } from '@state/checksSelection'; import { updateCatalog } from '@state/actions/catalog'; import { getCatalog } from '@state/selectors/catalog'; import { getHost } from '@state/selectors'; -import { getCheckSelection } from '@state/selectors/hostChecksSelection'; +import { getHostCheckSelection } from '@state/selectors/checksSelection'; import HostChecksSelection from './HostChecksSelection'; function HostSettingsPage() { @@ -23,7 +25,7 @@ function HostSettingsPage() { loading: catalogLoading, } = useSelector(getCatalog()); - const { saving } = useSelector(getCheckSelection()); + const { status } = useSelector(getHostCheckSelection(hostID)); if (!host) { return ; @@ -40,16 +42,16 @@ function HostSettingsPage() { dispatch( updateCatalog({ provider: host.provider, - target_type: 'host', + target_type: TARGET_HOST, }) ); - const saveSelection = (selection, targetID, targetName) => + const saveSelection = (newSelection, targetID, targetName) => dispatch( - checksSelected({ + hostChecksSelected({ hostID: targetID, hostName: targetName, - checks: selection, + checks: newSelection, }) ); @@ -64,7 +66,7 @@ function HostSettingsPage() { catalogError={catalogError} catalogLoading={catalogLoading} onUpdateCatalog={refreshCatalog} - isSavingSelection={saving} + isSavingSelection={isSaving(status)} onSaveSelection={saveSelection} /> ); diff --git a/assets/js/state/checksSelection.js b/assets/js/state/checksSelection.js index 95fc020a5d..c4cd85fab9 100644 --- a/assets/js/state/checksSelection.js +++ b/assets/js/state/checksSelection.js @@ -5,6 +5,10 @@ const SAVING = 'SAVING'; const SUCCESSFULLY_SAVED = 'SUCCESSFULLY_SAVED'; const SAVING_FAILED = 'SAVING_FAILED'; +export const isSaving = (status) => SAVING === status; +export const isSuccessfullySaved = (status) => SUCCESSFULLY_SAVED === status; +export const isSavingFailed = (status) => SAVING_FAILED === status; + const supportsTarget = (target) => [TARGET_CLUSTER, TARGET_HOST].includes(target); From bfd22a6abec00f5be9f5c9d044be5645a19a69d1 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Tue, 18 Jul 2023 15:05:11 +0200 Subject: [PATCH 7/9] Use the new check selection in legacy cluster ChecksSelection --- .../ClusterDetails/ChecksSelection.jsx | 33 ++++++++++++++----- .../ClusterDetails/ChecksSelection.test.jsx | 4 +-- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/assets/js/components/ClusterDetails/ChecksSelection.jsx b/assets/js/components/ClusterDetails/ChecksSelection.jsx index ee59911e8d..042363d130 100644 --- a/assets/js/components/ClusterDetails/ChecksSelection.jsx +++ b/assets/js/components/ClusterDetails/ChecksSelection.jsx @@ -8,8 +8,14 @@ import { EOS_LOADING_ANIMATED } from 'eos-icons-react'; import { remove, uniq, toggle, groupBy } from '@lib/lists'; import { getCatalog } from '@state/selectors/catalog'; import { updateCatalog } from '@state/actions/catalog'; -import { checksSelected } from '@state/clusters'; import { executionRequested } from '@state/actions/lastExecutions'; +import { getClusterCheckSelection } from '@state/selectors/checksSelection'; +import { + clusterChecksSelected, + isSaving, + isSuccessfullySaved, + isSavingFailed, +} from '@state/checksSelection'; import CatalogContainer from '@components/ChecksCatalog/CatalogContainer'; import { @@ -40,9 +46,12 @@ const getGroupSelectedState = (checks, selectedChecks) => { function ChecksSelection({ clusterId, cluster }) { const dispatch = useDispatch(); - const { saving, savingError, savingSuccess } = useSelector( - (state) => state.clusterChecksSelection - ); + const { status } = useSelector(getClusterCheckSelection(clusterId)); + const { saving, savingError, savingSuccess } = { + saving: isSaving(status), + savingSuccess: isSuccessfullySaved(status), + savingError: isSavingFailed(status), + }; const { data: catalogData, @@ -84,7 +93,11 @@ function ChecksSelection({ clusterId, cluster }) { }, [catalogData, selectedChecks]); useEffect(() => { - setLocalSavingError(savingError); + if (savingError === true) { + setLocalSavingError( + 'An unexpected error happened while selecting your desired checks' + ); + } setLocalSavingSuccess(savingSuccess); }, [savingError, savingSuccess]); @@ -103,9 +116,10 @@ function ChecksSelection({ clusterId, cluster }) { const saveSelection = useCallback(() => dispatch( - checksSelected({ - checks: selectedChecks, + clusterChecksSelected({ clusterID: clusterId, + clusterName: cluster.name, + checks: selectedChecks, }) ) ); @@ -158,6 +172,7 @@ function ChecksSelection({ clusterId, cluster }) { - {localSavingError && ( + {savingError && ( setLocalSavingError(null)}> -

{savingError}

+

{localSavingError}

 )} {localSavingSuccess && selectedChecks.length > 0 && ( diff --git a/assets/js/components/ClusterDetails/ChecksSelection.test.jsx b/assets/js/components/ClusterDetails/ChecksSelection.test.jsx index 0cfb4b3220..012e1eebfb 100644 --- a/assets/js/components/ClusterDetails/ChecksSelection.test.jsx +++ b/assets/js/components/ClusterDetails/ChecksSelection.test.jsx @@ -20,7 +20,6 @@ describe('ClusterDetails ChecksSelection component', () => { const initialState = { catalog: { loading: false, data: catalog, error: null }, - clusterChecksSelection: {}, }; const [statefulChecksSelection] = withState( , @@ -64,7 +63,6 @@ describe('ClusterDetails ChecksSelection component', () => { const initialState = { catalog: { loading: false, data: catalog, error: null }, - clusterChecksSelection: {}, }; const [statefulChecksSelection] = withState( , @@ -106,7 +104,6 @@ describe('ClusterDetails ChecksSelection component', () => { const initialState = { catalog: { loading: false, data: catalog, error: null }, - clusterChecksSelection: {}, }; const [statefulChecksSelection, store] = withState( , @@ -136,6 +133,7 @@ describe('ClusterDetails ChecksSelection component', () => { payload: { checks: selectedChecks, clusterID: cluster.id, + clusterName: cluster.name, }, }, ]; From 9514b0753b09312e0a89ec217887ccb844fb318c Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Wed, 19 Jul 2023 11:05:07 +0200 Subject: [PATCH 8/9] Refactor constants assignment in legacy ChecksSelection --- assets/js/components/ClusterDetails/ChecksSelection.jsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/assets/js/components/ClusterDetails/ChecksSelection.jsx b/assets/js/components/ClusterDetails/ChecksSelection.jsx index 042363d130..a3c3a6df8a 100644 --- a/assets/js/components/ClusterDetails/ChecksSelection.jsx +++ b/assets/js/components/ClusterDetails/ChecksSelection.jsx @@ -47,11 +47,9 @@ function ChecksSelection({ clusterId, cluster }) { const dispatch = useDispatch(); const { status } = useSelector(getClusterCheckSelection(clusterId)); - const { saving, savingError, savingSuccess } = { - saving: isSaving(status), - savingSuccess: isSuccessfullySaved(status), - savingError: isSavingFailed(status), - }; + const saving = isSaving(status); + const savingSuccess = isSuccessfullySaved(status); + const savingError = isSavingFailed(status); const { data: catalogData, From a9ec7aca2e5c960b814a9499b427c313337e51d8 Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Wed, 19 Jul 2023 12:02:29 +0200 Subject: [PATCH 9/9] Move check's selection is* to selectors --- .../ClusterDetails/ChecksSelection.jsx | 16 +-- .../HostDetails/HostSettingsPage.jsx | 8 +- assets/js/state/checksSelection.js | 10 +- assets/js/state/selectors/checksSelection.js | 28 ++++- .../state/selectors/checksSelection.test.js | 109 ++++++++++-------- 5 files changed, 100 insertions(+), 71 deletions(-) diff --git a/assets/js/components/ClusterDetails/ChecksSelection.jsx b/assets/js/components/ClusterDetails/ChecksSelection.jsx index a3c3a6df8a..a81c6da423 100644 --- a/assets/js/components/ClusterDetails/ChecksSelection.jsx +++ b/assets/js/components/ClusterDetails/ChecksSelection.jsx @@ -5,17 +5,18 @@ import { useSelector, useDispatch } from 'react-redux'; import { EOS_LOADING_ANIMATED } from 'eos-icons-react'; +import { TARGET_CLUSTER } from '@lib/model'; + import { remove, uniq, toggle, groupBy } from '@lib/lists'; import { getCatalog } from '@state/selectors/catalog'; import { updateCatalog } from '@state/actions/catalog'; import { executionRequested } from '@state/actions/lastExecutions'; -import { getClusterCheckSelection } from '@state/selectors/checksSelection'; import { - clusterChecksSelected, isSaving, isSuccessfullySaved, isSavingFailed, -} from '@state/checksSelection'; +} from '@state/selectors/checksSelection'; +import { clusterChecksSelected } from '@state/checksSelection'; import CatalogContainer from '@components/ChecksCatalog/CatalogContainer'; import { @@ -46,10 +47,11 @@ const getGroupSelectedState = (checks, selectedChecks) => { function ChecksSelection({ clusterId, cluster }) { const dispatch = useDispatch(); - const { status } = useSelector(getClusterCheckSelection(clusterId)); - const saving = isSaving(status); - const savingSuccess = isSuccessfullySaved(status); - const savingError = isSavingFailed(status); + const saving = useSelector(isSaving(TARGET_CLUSTER, clusterId)); + const savingSuccess = useSelector( + isSuccessfullySaved(TARGET_CLUSTER, clusterId) + ); + const savingError = useSelector(isSavingFailed(TARGET_CLUSTER, clusterId)); const { data: catalogData, diff --git a/assets/js/components/HostDetails/HostSettingsPage.jsx b/assets/js/components/HostDetails/HostSettingsPage.jsx index 7a3ca84f1b..6c04229fb8 100644 --- a/assets/js/components/HostDetails/HostSettingsPage.jsx +++ b/assets/js/components/HostDetails/HostSettingsPage.jsx @@ -6,11 +6,11 @@ import LoadingBox from '@components/LoadingBox'; import { TARGET_HOST } from '@lib/model'; -import { hostChecksSelected, isSaving } from '@state/checksSelection'; +import { hostChecksSelected } from '@state/checksSelection'; import { updateCatalog } from '@state/actions/catalog'; import { getCatalog } from '@state/selectors/catalog'; import { getHost } from '@state/selectors'; -import { getHostCheckSelection } from '@state/selectors/checksSelection'; +import { isSaving } from '@state/selectors/checksSelection'; import HostChecksSelection from './HostChecksSelection'; function HostSettingsPage() { @@ -25,7 +25,7 @@ function HostSettingsPage() { loading: catalogLoading, } = useSelector(getCatalog()); - const { status } = useSelector(getHostCheckSelection(hostID)); + const saving = useSelector(isSaving(TARGET_HOST, hostID)); if (!host) { return ; @@ -66,7 +66,7 @@ function HostSettingsPage() { catalogError={catalogError} catalogLoading={catalogLoading} onUpdateCatalog={refreshCatalog} - isSavingSelection={isSaving(status)} + isSavingSelection={saving} onSaveSelection={saveSelection} /> ); diff --git a/assets/js/state/checksSelection.js b/assets/js/state/checksSelection.js index c4cd85fab9..d077dbab17 100644 --- a/assets/js/state/checksSelection.js +++ b/assets/js/state/checksSelection.js @@ -1,13 +1,9 @@ import { createAction, createSlice } from '@reduxjs/toolkit'; import { TARGET_CLUSTER, TARGET_HOST } from '@lib/model'; -const SAVING = 'SAVING'; -const SUCCESSFULLY_SAVED = 'SUCCESSFULLY_SAVED'; -const SAVING_FAILED = 'SAVING_FAILED'; - -export const isSaving = (status) => SAVING === status; -export const isSuccessfullySaved = (status) => SUCCESSFULLY_SAVED === status; -export const isSavingFailed = (status) => SAVING_FAILED === status; +export const SAVING = 'SAVING'; +export const SUCCESSFULLY_SAVED = 'SUCCESSFULLY_SAVED'; +export const SAVING_FAILED = 'SAVING_FAILED'; const supportsTarget = (target) => [TARGET_CLUSTER, TARGET_HOST].includes(target); diff --git a/assets/js/state/selectors/checksSelection.js b/assets/js/state/selectors/checksSelection.js index bf008c851c..541bc02a55 100644 --- a/assets/js/state/selectors/checksSelection.js +++ b/assets/js/state/selectors/checksSelection.js @@ -1,11 +1,35 @@ import { TARGET_HOST, TARGET_CLUSTER } from '@lib/model'; +import { SAVING, SAVING_FAILED, SUCCESSFULLY_SAVED } from '../checksSelection'; -export const getHostCheckSelection = +const getHostCheckSelection = (hostID) => ({ checksSelection }) => checksSelection?.[TARGET_HOST][hostID] || {}; -export const getClusterCheckSelection = +const getClusterCheckSelection = (clusterID) => ({ checksSelection }) => checksSelection?.[TARGET_CLUSTER][clusterID] || {}; + +const targetToSelector = { + [TARGET_HOST]: getHostCheckSelection, + [TARGET_CLUSTER]: getClusterCheckSelection, +}; + +const getTargetSelector = (targetType) => + targetToSelector[targetType] || (() => {}); + +const getTargetStatus = (state, targetType, targetID) => { + const targetSelector = getTargetSelector(targetType); + const { status } = targetSelector(targetID)(state); + return status; +}; + +export const isSaving = (targetType, targetID) => (state) => + getTargetStatus(state, targetType, targetID) === SAVING; + +export const isSuccessfullySaved = (targetType, targetID) => (state) => + getTargetStatus(state, targetType, targetID) === SUCCESSFULLY_SAVED; + +export const isSavingFailed = (targetType, targetID) => (state) => + getTargetStatus(state, targetType, targetID) === SAVING_FAILED; diff --git a/assets/js/state/selectors/checksSelection.test.js b/assets/js/state/selectors/checksSelection.test.js index 8b4ab3b7af..0906747d51 100644 --- a/assets/js/state/selectors/checksSelection.test.js +++ b/assets/js/state/selectors/checksSelection.test.js @@ -1,63 +1,70 @@ import { faker } from '@faker-js/faker'; import { - getHostCheckSelection, - getClusterCheckSelection, + isSaving, + isSuccessfullySaved, + isSavingFailed, } from './checksSelection'; describe('Checks Selection selector', () => { - it(`should get a host's check selection state`, () => { - const hostID = faker.datatype.uuid(); - expect( - getHostCheckSelection(hostID)({ - checksSelection: { - host: { - [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, - [hostID]: { status: 'SAVING' }, - }, - }, - }) - ).toEqual({ status: 'SAVING' }); - }); + it(`should detect a target's status`, () => { + const savingHostSelection = faker.datatype.uuid(); + const successfullySavedHostSelection = faker.datatype.uuid(); + const savingFailedHostSelection = faker.datatype.uuid(); - it(`should not get a host's check selection state if not present`, () => { - const hostWithoutSelection = faker.datatype.uuid(); - expect( - getHostCheckSelection(hostWithoutSelection)({ - checksSelection: { - host: { - [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, - [faker.datatype.uuid()]: { status: 'SAVING' }, - }, - }, - }) - ).toEqual({}); - }); + const savingClusterSelection = faker.datatype.uuid(); + const successfullySavedClusterSelection = faker.datatype.uuid(); + const savingFailedClusterSelection = faker.datatype.uuid(); - it(`should get a cluster's check selection state`, () => { - const clusterID = faker.datatype.uuid(); - expect( - getClusterCheckSelection(clusterID)({ - checksSelection: { - cluster: { - [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, - [clusterID]: { status: 'SAVING' }, - }, + const state = { + checksSelection: { + host: { + [savingHostSelection]: { status: 'SAVING' }, + [successfullySavedHostSelection]: { status: 'SUCCESSFULLY_SAVED' }, + [savingFailedHostSelection]: { status: 'SAVING_FAILED' }, }, - }) - ).toEqual({ status: 'SAVING' }); - }); + cluster: { + [savingClusterSelection]: { status: 'SAVING' }, + [successfullySavedClusterSelection]: { status: 'SUCCESSFULLY_SAVED' }, + [savingFailedClusterSelection]: { status: 'SAVING_FAILED' }, + }, + }, + }; + + expect(isSaving('host', savingHostSelection)(state)).toBe(true); + expect(isSaving('cluster', savingClusterSelection)(state)).toBe(true); + expect(isSaving('host', successfullySavedHostSelection)(state)).toBe(false); + expect(isSaving('cluster', successfullySavedClusterSelection)(state)).toBe( + false + ); + expect(isSaving('host', faker.datatype.uuid())(state)).toBe(false); + expect(isSaving('cluster', faker.datatype.uuid())(state)).toBe(false); - it(`should not get a cluster's check selection state if not present`, () => { - const clustertWithoutSelection = faker.datatype.uuid(); expect( - getClusterCheckSelection(clustertWithoutSelection)({ - checksSelection: { - cluster: { - [faker.datatype.uuid()]: { status: 'SUCCESSFULLY_SAVED' }, - [faker.datatype.uuid()]: { status: 'SAVING' }, - }, - }, - }) - ).toEqual({}); + isSuccessfullySaved('host', successfullySavedHostSelection)(state) + ).toBe(true); + expect( + isSuccessfullySaved('cluster', successfullySavedClusterSelection)(state) + ).toBe(true); + expect(isSuccessfullySaved('host', savingHostSelection)(state)).toBe(false); + expect(isSuccessfullySaved('cluster', savingClusterSelection)(state)).toBe( + false + ); + expect(isSuccessfullySaved('host', faker.datatype.uuid())(state)).toBe( + false + ); + expect(isSuccessfullySaved('cluster', faker.datatype.uuid())(state)).toBe( + false + ); + + expect(isSavingFailed('host', savingFailedHostSelection)(state)).toBe(true); + expect(isSavingFailed('cluster', savingFailedClusterSelection)(state)).toBe( + true + ); + expect(isSavingFailed('host', savingHostSelection)(state)).toBe(false); + expect(isSavingFailed('cluster', savingClusterSelection)(state)).toBe( + false + ); + expect(isSavingFailed('host', faker.datatype.uuid())(state)).toBe(false); + expect(isSavingFailed('cluster', faker.datatype.uuid())(state)).toBe(false); }); });