Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add database deregistered event usage to redux #1594

Merged
merged 2 commits into from
Jul 5, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions assets/js/state/channels.js
Original file line number Diff line number Diff line change
@@ -45,6 +45,7 @@ const processChannelEvents = (reduxStore, socket) => {
]);
registerEvents(reduxStore, socket, 'monitoring:databases', [
'database_registered',
'database_deregistered',
'database_health_changed',
'database_instance_registered',
'database_instance_deregistered',
7 changes: 7 additions & 0 deletions assets/js/state/databases.js
Original file line number Diff line number Diff line change
@@ -30,6 +30,11 @@ export const databasesListSlice = createSlice({
appendDatabaseInstance: (state, action) => {
state.databaseInstances = [...state.databaseInstances, action.payload];
},
removeDatabase: (state, { payload: { id } }) => {
state.databases = state.databases.filter(
(database) => !(database.id === id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason to use !(database.id === id) over database.id !== id here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope... XD
Yes, i will change it to use your suggestion, it is more naturla

);
},
removeDatabaseInstance: (
state,
{ payload: { sap_system_id, host_id, instance_number } }
@@ -92,6 +97,7 @@ export const databasesListSlice = createSlice({
});

export const DATABASE_REGISTERED = 'DATABASE_REGISTERED';
export const DATABASE_DEREGISTERED = 'DATABASE_DEREGISTERED';
export const DATABASE_HEALTH_CHANGED = 'DATABASE_HEALTH_CHANGED';
export const DATABASE_INSTANCE_REGISTERED = 'DATABASE_INSTANCE_REGISTERED';
export const DATABASE_INSTANCE_DEREGISTERED = 'DATABASE_INSTANCE_DEREGISTERED';
@@ -105,6 +111,7 @@ export const {
stopDatabasesLoading,
setDatabases,
appendDatabase,
removeDatabase,
removeDatabaseInstance,
appendDatabaseInstance,
updateDatabaseHealth,
27 changes: 24 additions & 3 deletions assets/js/state/databases.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import databaseReducer, { removeDatabaseInstance } from '@state/databases';
import { databaseInstanceFactory } from '@lib/test-utils/factories/databases';
import databaseReducer, {
removeDatabase,
removeDatabaseInstance,
} from '@state/databases';
import {
databaseFactory,
databaseInstanceFactory,
} from '@lib/test-utils/factories/databases';

describe('Databases reducer', () => {
it('should remove a datase instance from state', () => {
it('should remove a database instance from state', () => {
const [instance1, instance2] = databaseInstanceFactory.buildList(2);
const initialState = {
databaseInstances: [instance1, instance2],
@@ -16,4 +22,19 @@ describe('Databases reducer', () => {

expect(databaseReducer(initialState, action)).toEqual(expectedState);
});

it('should remove a database from state', () => {
const [database1, database2] = databaseFactory.buildList(2);
const initialState = {
databases: [database1, database2],
};

const action = removeDatabase(database1);

const expectedState = {
databases: [database2],
};

expect(databaseReducer(initialState, action)).toEqual(expectedState);
});
});
13 changes: 13 additions & 0 deletions assets/js/state/sagas/databases.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { put, select, takeEvery } from 'redux-saga/effects';
import {
DATABASE_REGISTERED,
DATABASE_DEREGISTERED,
DATABASE_HEALTH_CHANGED,
DATABASE_INSTANCE_REGISTERED,
DATABASE_INSTANCE_DEREGISTERED,
@@ -11,6 +12,7 @@ import {
updateDatabaseHealth,
updateDatabaseInstanceHealth,
updateDatabaseInstanceSystemReplication,
removeDatabase,
removeDatabaseInstance,
} from '@state/databases';

@@ -77,6 +79,16 @@ function* databaseInstanceRegistered({ payload }) {
);
}

export function* databaseDeregistered({ payload }) {
yield put(removeDatabase(payload));
yield put(
notify({
text: `The database ${payload.sid} has been deregistered.`,
icon: 'ℹ️',
})
);
}

export function* databaseInstanceDeregistered({ payload }) {
yield put(removeDatabaseInstance(payload));
yield put(removeDatabaseInstanceFromSapSystem(payload));
@@ -106,6 +118,7 @@ function* databaseInstanceSystemReplicationChanged({ payload }) {

export function* watchDatabase() {
yield takeEvery(DATABASE_REGISTERED, databaseRegistered);
yield takeEvery(DATABASE_DEREGISTERED, databaseDeregistered);
yield takeEvery(DATABASE_HEALTH_CHANGED, databaseHealthChanged);
yield takeEvery(DATABASE_INSTANCE_REGISTERED, databaseInstanceRegistered);
yield takeEvery(DATABASE_INSTANCE_DEREGISTERED, databaseInstanceDeregistered);
34 changes: 31 additions & 3 deletions assets/js/state/sagas/databases.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { recordSaga } from '@lib/test-utils';
import { databaseInstanceDeregistered } from '@state/sagas/databases';
import { removeDatabaseInstance } from '@state/databases';
import {
databaseDeregistered,
databaseInstanceDeregistered,
} from '@state/sagas/databases';
import { removeDatabase, removeDatabaseInstance } from '@state/databases';
import { removeDatabaseInstanceFromSapSystem } from '@state/sapSystems';
import { databaseInstanceFactory } from '@lib/test-utils/factories';
import {
databaseFactory,
databaseInstanceFactory,
} from '@lib/test-utils/factories';
import { notify } from '@state/actions/notifications';

describe('SAP Systems sagas', () => {
@@ -32,4 +38,26 @@ describe('SAP Systems sagas', () => {
})
);
});

it('should remove the database', async () => {
const { sap_system_id: id, sid } = databaseFactory.build();

const dispatched = await recordSaga(databaseDeregistered, {
payload: { id, sid },
});

expect(dispatched).toContainEqual(
removeDatabase({
id,
sid,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like sid is necessary for removeDatabase(). A minor thing though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, the payload is composed with the sid in the incoming event, so we need to put it, even though we don't use it in the reducer

})
);

expect(dispatched).toContainEqual(
notify({
text: `The database ${sid} has been deregistered.`,
icon: 'ℹ️',
})
);
});
});