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.

Sign up for GitHub

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

Make frontend listen for events when instances' presence changes #1779

Merged
merged 3 commits into from
Sep 4, 2023
Merged

Make frontend listen for events when instances' presence changes #1779

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aa2c987
Make frontend listen for events when instances' presence changes
jamie-suse Aug 31, 2023
941283a
Db instances updated in sap systems slice when absent state changes
CDimonaco Sep 4, 2023
e47b0ef
Addressing review feedbacks
CDimonaco Sep 4, 2023
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/lib/test-utils/factories/sapSystems.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const sapSystemApplicationInstanceFactory = Factory.define(() => ({
sid: faker.random.alphaNumeric(3, { casing: 'upper' }),
start_priority: faker.datatype.number({ min: 1, max: 9 }).toString(),
sap_system_id: faker.datatype.uuid(),
absent_at: null,
}));

export const sapSystemFactory = Factory.define(({ params }) => {
Expand Down
2 changes: 2 additions & 0 deletions assets/js/state/channels.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const processChannelEvents = (reduxStore, socket) => {
'sap_system_health_changed',
'application_instance_registered',
'application_instance_moved',
'application_instance_absent_at_changed',
'application_instance_deregistered',
'application_instance_health_changed',
'sap_system_deregistered',
Expand All @@ -52,6 +53,7 @@ const processChannelEvents = (reduxStore, socket) => {
'database_restored',
'database_health_changed',
'database_instance_registered',
'database_instance_absent_at_changed',
'database_instance_deregistered',
'database_instance_health_changed',
'database_instance_system_replication_changed',
Expand Down
12 changes: 12 additions & 0 deletions assets/js/state/databases.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ export const databasesListSlice = createSlice({
}
);
},
updateDatabaseInstanceAbsentAt: (state, { payload: instance }) => {
const { absent_at } = instance;

state.databaseInstances = updateInstance(
state.databaseInstances,
instance,
{ absent_at }
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put {absent_at: instance.absent_at} to have the same style like the other reducer functions

Copy link
Member

Choose a reason for hiding this comment

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

done

);
},
setDatabaseInstanceDeregistering: (state, { payload: instance }) => {
state.databaseInstances = updateInstance(
state.databaseInstances,
Expand All @@ -115,6 +124,8 @@ export const DATABASE_DEREGISTERED = 'DATABASE_DEREGISTERED';
export const DATABASE_RESTORED = 'DATABASE_RESTORED';
export const DATABASE_HEALTH_CHANGED = 'DATABASE_HEALTH_CHANGED';
export const DATABASE_INSTANCE_REGISTERED = 'DATABASE_INSTANCE_REGISTERED';
export const DATABASE_INSTANCE_ABSENT_AT_CHANGED =
'DATABASE_INSTANCE_ABSENT_AT_CHANGED';
export const DATABASE_INSTANCE_DEREGISTERED = 'DATABASE_INSTANCE_DEREGISTERED';
export const DATABASE_INSTANCE_HEALTH_CHANGED =
'DATABASE_INSTANCE_HEALTH_CHANGED';
Expand All @@ -137,6 +148,7 @@ export const {
updateDatabaseHealth,
updateDatabaseInstanceHealth,
updateDatabaseInstanceSystemReplication,
updateDatabaseInstanceAbsentAt,
addTagToDatabase,
removeTagFromDatabase,
setDatabaseInstanceDeregistering,
Expand Down
28 changes: 28 additions & 0 deletions assets/js/state/databases.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import databaseReducer, {
upsertDatabaseInstances,
updateDatabaseInstanceHealth,
updateDatabaseInstanceSystemReplication,
updateDatabaseInstanceAbsentAt,
setDatabaseInstanceDeregistering,
unsetDatabaseInstanceDeregistering,
} from '@state/databases';
Expand Down Expand Up @@ -129,6 +130,33 @@ describe('Databases reducer', () => {
expect(databaseReducer(initialState, action)).toEqual(expectedState);
});

it('should update the absent_at field of an database instance', () => {
const instance = databaseInstanceFactory.build();
const absentAt = Date.now();

const initialState = {
databaseInstances: [instance],
};

const instanceToUpdate = {
...instance,
absent_at: absentAt,
};

const action = updateDatabaseInstanceAbsentAt(instanceToUpdate);

const expectedState = {
databaseInstances: [
{
...instance,
absent_at: absentAt,
},
],
};

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

it('should set database instance in deregistering state', () => {
const instance = databaseInstanceFactory.build();

Expand Down
19 changes: 19 additions & 0 deletions assets/js/state/sagas/databases.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
DATABASE_RESTORED,
DATABASE_HEALTH_CHANGED,
DATABASE_INSTANCE_REGISTERED,
DATABASE_INSTANCE_ABSENT_AT_CHANGED,
DATABASE_INSTANCE_DEREGISTERED,
DATABASE_INSTANCE_HEALTH_CHANGED,
DATABASE_INSTANCE_SYSTEM_REPLICATION_CHANGED,
Expand All @@ -16,6 +17,7 @@ import {
updateDatabaseHealth,
updateDatabaseInstanceHealth,
updateDatabaseInstanceSystemReplication,
updateDatabaseInstanceAbsentAt,
removeDatabase,
removeDatabaseInstance,
setDatabaseInstanceDeregistering,
Expand Down Expand Up @@ -111,6 +113,19 @@ function* databaseInstanceSystemReplicationChanged({ payload }) {
yield put(updateSAPSystemDatabaseInstanceSystemReplication(payload));
}

export function* databaseInstanceAbsentAtChanged({ payload }) {
yield put(updateDatabaseInstanceAbsentAt(payload));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as https://github.com/trento-project/web/pull/1779/files#r1311888825

CDimonaco marked this conversation as resolved.
Show resolved Hide resolved
const { sid, absent_at } = payload;
yield put(
notify({
text: `The database instance ${sid} is now ${
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as https://github.com/trento-project/web/pull/1779/files#r1311890291

Copy link
Contributor

Choose a reason for hiding this comment

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

I would put:
The database instance ${instance_number} from ${sid} is absent
and
The database instance ${instance_number} from ${sid} is present again

absent_at ? 'absent' : 'present'
}.`,
icon: 'ℹ️',
})
);
}

export function* deregisterDatabaseInstance({
payload,
payload: { sid, sap_system_id, host_id, instance_number },
Expand Down Expand Up @@ -141,6 +156,10 @@ export function* watchDatabase() {
yield takeEvery(DATABASE_RESTORED, databaseRestored);
yield takeEvery(DATABASE_HEALTH_CHANGED, databaseHealthChanged);
yield takeEvery(DATABASE_INSTANCE_REGISTERED, databaseInstanceRegistered);
yield takeEvery(
DATABASE_INSTANCE_ABSENT_AT_CHANGED,
databaseInstanceAbsentAtChanged
);
yield takeEvery(DATABASE_INSTANCE_DEREGISTERED, databaseInstanceDeregistered);
yield takeEvery(
DATABASE_INSTANCE_HEALTH_CHANGED,
Expand Down
49 changes: 49 additions & 0 deletions assets/js/state/sagas/databases.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import MockAdapter from 'axios-mock-adapter';

import { recordSaga } from '@lib/test-utils';
import {
databaseInstanceAbsentAtChanged,
databaseDeregistered,
databaseInstanceDeregistered,
databaseRestored,
Expand All @@ -12,6 +13,7 @@ import {
removeDatabase,
removeDatabaseInstance,
appendDatabase,
updateDatabaseInstanceAbsentAt,
setDatabaseInstanceDeregistering,
unsetDatabaseInstanceDeregistering,
} from '@state/databases';
Expand Down Expand Up @@ -41,6 +43,53 @@ describe('SAP Systems sagas', () => {
console.error.mockRestore();
});

it('should update the absent_at field when the database instance is marked absent', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this now in the top of the file?
I would put it at the end as it is at the end in the prod code as well

Copy link
Member

Choose a reason for hiding this comment

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

done

const { sap_system_id, instance_number, host_id, sid } =
databaseInstanceFactory.build();
const absent_at = Date.now();
Copy link
Contributor

Choose a reason for hiding this comment

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

I bit strange this absent_at not going in the factory itself, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

done


const dispatched = await recordSaga(databaseInstanceAbsentAtChanged, {
payload: { sap_system_id, instance_number, host_id, sid, absent_at },
});

expect(dispatched).toEqual([
updateDatabaseInstanceAbsentAt({
sap_system_id,
instance_number,
host_id,
sid,
absent_at,
}),
notify({
text: `The database instance ${sid} is now absent.`,
icon: 'ℹ️',
}),
]);
});

it('should update the absent_at field when the database instance is marked present', async () => {
const { sap_system_id, instance_number, host_id, sid, absent_at } =
databaseInstanceFactory.build();

const dispatched = await recordSaga(databaseInstanceAbsentAtChanged, {
payload: { sap_system_id, instance_number, host_id, sid, absent_at },
});

expect(dispatched).toEqual([
updateDatabaseInstanceAbsentAt({
sap_system_id,
instance_number,
host_id,
sid,
absent_at,
}),
notify({
text: `The database instance ${sid} is now present.`,
icon: 'ℹ️',
}),
]);
});

it('should remove the database instance', async () => {
const { sap_system_id, host_id, instance_number, sid } =
databaseInstanceFactory.build();
Expand Down
19 changes: 19 additions & 0 deletions assets/js/state/sagas/sapSystems.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
APPLICATION_INSTANCE_REGISTERED,
APPLICATION_INSTANCE_MOVED,
APPLICATION_INSTANCE_HEALTH_CHANGED,
APPLICATION_INSTANCE_ABSENT_AT_CHANGED,
APPLICATION_INSTANCE_DEREGISTERED,
SAP_SYSTEM_DEREGISTERED,
SAP_SYSTEM_RESTORED,
Expand All @@ -19,6 +20,7 @@ import {
removeApplicationInstance,
updateApplicationInstanceHost,
updateApplicationInstanceHealth,
updateApplicationInstanceAbsentAt,
removeSAPSystem,
updateSAPSystem,
setApplicationInstanceDeregistering,
Expand Down Expand Up @@ -79,6 +81,19 @@ function* applicationInstanceHealthChanged({ payload }) {
yield put(updateApplicationInstanceHealth(payload));
}

export function* applicationInstanceAbsentAtChanged({ payload }) {
yield put(updateApplicationInstanceAbsentAt(payload));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may be a bit technical, but payload will include the field sid, that we don't strictly need to pass to updateApplicationInstanceAbsentAt(). Thoughts on this?

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 really matter, it is fine this way

const { sid, absent_at } = payload;
yield put(
notify({
text: `The application instance ${sid} is now ${
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thoughts on this message?

absent_at ? 'absent' : 'present'
}.`,
icon: 'ℹ️',
})
);
}

export function* sapSystemDeregistered({ payload: { id, sid } }) {
yield put(removeSAPSystem({ id }));
yield put(
Expand Down Expand Up @@ -142,6 +157,10 @@ export function* watchSapSystem() {
applicationInstanceRegistered
);
yield takeEvery(APPLICATION_INSTANCE_MOVED, applicationInstanceMoved);
yield takeEvery(
APPLICATION_INSTANCE_ABSENT_AT_CHANGED,
applicationInstanceAbsentAtChanged
);
yield takeEvery(
APPLICATION_INSTANCE_DEREGISTERED,
applicationInstanceDeregistered
Expand Down
49 changes: 49 additions & 0 deletions assets/js/state/sagas/sapSystems.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import MockAdapter from 'axios-mock-adapter';
import { recordSaga } from '@lib/test-utils';
import {
applicationInstanceMoved,
applicationInstanceAbsentAtChanged,
applicationInstanceDeregistered,
sapSystemDeregistered,
sapSystemRestored,
Expand All @@ -15,6 +16,7 @@ import {
upsertDatabaseInstancesToSapSystem,
updateApplicationInstanceHost,
upsertApplicationInstances,
updateApplicationInstanceAbsentAt,
removeApplicationInstance,
updateSAPSystem,
setApplicationInstanceDeregistering,
Expand Down Expand Up @@ -88,6 +90,53 @@ describe('SAP Systems sagas', () => {
);
});

it('should update the absent_at field when the application instance is marked absent', async () => {
const { sap_system_id, instance_number, host_id, sid } =
sapSystemApplicationInstanceFactory.build();
const absent_at = Date.now();

const dispatched = await recordSaga(applicationInstanceAbsentAtChanged, {
payload: { sap_system_id, instance_number, host_id, sid, absent_at },
});

expect(dispatched).toEqual([
updateApplicationInstanceAbsentAt({
sap_system_id,
instance_number,
host_id,
sid,
absent_at,
}),
notify({
text: `The application instance ${sid} is now absent.`,
icon: 'ℹ️',
}),
]);
});

it('should update the absent_at field when the application instance is marked present', async () => {
const { sap_system_id, instance_number, host_id, sid, absent_at } =
sapSystemApplicationInstanceFactory.build();

const dispatched = await recordSaga(applicationInstanceAbsentAtChanged, {
payload: { sap_system_id, instance_number, host_id, sid, absent_at },
});

expect(dispatched).toEqual([
updateApplicationInstanceAbsentAt({
sap_system_id,
instance_number,
host_id,
sid,
absent_at,
}),
notify({
text: `The application instance ${sid} is now present.`,
icon: 'ℹ️',
}),
]);
});

it('should remove the application instance', async () => {
const { sap_system_id, host_id, instance_number } =
sapSystemApplicationInstanceFactory.build();
Expand Down
12 changes: 12 additions & 0 deletions assets/js/state/sapSystems.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,15 @@ export const sapSystemsListSlice = createSlice({
}
);
},
updateApplicationInstanceAbsentAt: (state, { payload: instance }) => {
const { absent_at } = instance;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we have this like the database reducer?

Copy link
Member

Choose a reason for hiding this comment

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

done


state.applicationInstances = updateInstance(
state.applicationInstances,
instance,
{ absent_at }
);
},
setApplicationInstanceDeregistering: (state, { payload: instance }) => {
state.applicationInstances = updateInstance(
state.applicationInstances,
Expand Down Expand Up @@ -191,6 +200,8 @@ export const SAP_SYSTEM_HEALTH_CHANGED = 'SAP_SYSTEM_HEALTH_CHANGED';
export const APPLICATION_INSTANCE_REGISTERED =
'APPLICATION_INSTANCE_REGISTERED';
export const APPLICATION_INSTANCE_MOVED = 'APPLICATION_INSTANCE_MOVED';
export const APPLICATION_INSTANCE_ABSENT_AT_CHANGED =
'APPLICATION_INSTANCE_ABSENT_AT_CHANGED';
export const APPLICATION_INSTANCE_DEREGISTERED =
'APPLICATION_INSTANCE_DEREGISTERED';
export const APPLICATION_INSTANCE_HEALTH_CHANGED =
Expand Down Expand Up @@ -223,6 +234,7 @@ export const {
removeTagFromSAPSystem,
removeSAPSystem,
updateSAPSystem,
updateApplicationInstanceAbsentAt,
setApplicationInstanceDeregistering,
unsetApplicationInstanceDeregistering,
setDatabaseInstanceDeregisteringToSAPSystem,
Expand Down
Loading