#1777 - Process MSFAA Cancellation and Reactivation Notification #2063
Conversation
sh16011993
requested review from
dheepak-aot,
guru-aot,
andrewsignori-aot,
ann-aot,
andrepestana-aot,
hellolynn-tbtb and
JasonCTang
| }); | ||
| const systemUser = await this.systemUsersService.systemUser(); | ||
| // Only if the msfaa record was successfully cancelled, then save the msfaa cancellation notification. | ||
| // This also takes care of the scenario where the msfaa record was already cancelled before in which case the record will not be update and hence no notification will be saved. |
There was a problem hiding this comment.
Please change it to "update and hence no notification will be sent"
There was a problem hiding this comment.
I believe saved is right here because here we are saving the notifications only. Sending will happen through the processNotifications job later.
sources/packages/backend/libs/integrations/src/services/msfaa-number/msfaa-number.service.ts
Outdated
Show resolved
Hide resolved
sources/packages/backend/libs/sims-db/src/entities/notification.model.ts
Outdated
Show resolved
Hide resolved
sources/packages/backend/libs/integrations/src/services/msfaa-number/msfaa-number.service.ts
Outdated
Show resolved
Hide resolved
sources/packages/backend/libs/integrations/src/services/msfaa-number/msfaa-number.service.ts
Outdated
Show resolved
Hide resolved
| @@ -6,7 +6,11 @@ import { | |||
| OfferingIntensity, | |||
| } from "@sims/sims-db"; | |||
| import { getISODateOnlyString } from "@sims/utilities"; | |||
There was a problem hiding this comment.
Are we not suppose to send notification when we cancel an MSFAA during re-activation or reissue?
There was a problem hiding this comment.
I would say no @dheepak-aot. In my understanding, we are deactivating some MSFAA(s) in order to activate other. The student will receive an email notification about it.
The point about sending a notification during the cancellation is that it may require an action from the student and it will not be communicated in any other way.
sources/packages/backend/libs/integrations/src/services/msfaa-number/msfaa-number.service.ts
Outdated
Show resolved
Hide resolved
sources/packages/backend/libs/integrations/src/services/msfaa-number/msfaa-number.service.ts
Show resolved
Hide resolved
| const notificationToSend = { | ||
| userId: notification.userId, | ||
| messageType: NotificationMessageType.MSFAACancellationComplete, | ||
| messagePayload: { |
There was a problem hiding this comment.
Please have the message payload typed like this.
const messagePayload: NotificationEmailMessage = {
email_address: notification.toAddress,
template_id: templateId,
personalisation: {
givenNames: notification.givenNames ?? "",
lastName: notification.lastName,
},
};
const notificationToSend = {
userId: notification.userId,
messageType: NotificationMessageType.MSFAACancellationComplete,
messagePayload: messagePayload,
};
|
Good job on first notification effort. Please have a look at the comments. |
| } | ||
| // Only if the msfaa record was successfully cancelled, then save the msfaa cancellation notification. | ||
| // This also takes care of the scenario where the msfaa record was already cancelled before in which case the record will not be updated and hence no notification will be saved. | ||
| if (updateResult.affected) { |
There was a problem hiding this comment.
This condition is not required anymore. If code is past line 224 the updateResult.affected cannot be 0.
There was a problem hiding this comment.
Removed it. Thanks.
| }, | ||
| ); | ||
| // In case no MSFAA record was cancelled. | ||
| if (!updateResult.affected) { |
There was a problem hiding this comment.
we are throwing the error on the consumers function, so, do we need it here?
There was a problem hiding this comment.
I have removed it from the consumer to keep it with msfaa-number.service.ts generating the updateResult.
|
Kudos, SonarCloud Quality Gate passed!
|
Backend Unit Tests Coverage Report
|
E2E Workflow Workers Coverage Report
|
E2E Queue Consumers Coverage Report
|
E2E SIMS API Coverage Report
|
sh16011993
deleted the
1777_process_msfaa_cancellation_and_reactivation_notification
branch
As a part of this PR:
Screenshot of the sent notification email:
To be done in a separate PR:
NOTE: There is an update in the Acceptance Criteria (point 2) - MSFAA reactivation template is not required as the federal government will send them notification. Only MSFAA cancellation notification needs to be sent.