#4823 - Parent Declare: Email Notification (Part 1) #4927

sh16011993 · 2025-07-14T22:53:31Z

As a part of this PR, the following were completed:

Added DB migrations and added code logic to send notification for the following scenarios:

Parent able to report their declaration
Parent unable to report their declaration

Note: E2E tests will be part of the next PR.

Screenshots:

NOTE: The content for the below email notifications will have its wording revised by the business later. All the required variables from the business have been passed into the templates.

Parents able to report their declaration

Parent 1

Parent 2

Parent unable to report their declaration

Migration rollback:

...s/src/migrations/1752517099854-InsertParentDeclarationRequiredParentCanReportNotification.ts

.../packages/backend/apps/workers/src/controllers/supporting-user/supporting-user.controller.ts

dheepak-aot · 2025-07-15T20:45:28Z

.../packages/backend/apps/workers/src/controllers/supporting-user/supporting-user.controller.ts

+                    ? "parent"
+                    : "partner",
+                parentFullName: fullName,
+                applicationNumber: application.applicationNumber,


If the application number is required by the business, can we have this AC updated?

I have mentioned it to @CarlyCotton

.../packages/backend/apps/workers/src/controllers/supporting-user/supporting-user.controller.ts

...ackages/backend/libs/services/src/notifications/notification/notification-actions.service.ts

sources/packages/backend/libs/services/src/notifications/notification/notification.model.ts

dheepak-aot · 2025-07-15T21:09:06Z

Good start @sh16011993 Please have a look at the comments.

Copilot

Pull Request Overview

This PR implements email notifications for parent declaration scenarios in a student information management system. The changes add support for notifying students when parent declaration is required for applications from 2025 program year onwards, covering both cases where parents can report and cannot report their declarations.

Key Changes:

Added two new notification message types for parent declaration scenarios
Implemented notification services and data models for both parent reporting scenarios
Integrated notification triggers into the supporting user creation workflow

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
notification.model.ts	Added enum values for new parent declaration notification types
notification.model.ts (services)	Defined TypeScript interfaces for notification data structures
notification-actions.service.ts	Implemented notification creation methods for both parent scenarios
supporting-user.controller.ts	Integrated notification triggers into user creation workflow
Migration files	Added database migrations to insert notification message configurations

Copilot · 2025-07-15T21:44:36Z

sources/packages/backend/libs/services/src/notifications/notification/notification.model.ts

  supportingUserType: NotificationSupportingUserType;
 }

+export interface ParentDeclarationRequiredParentCanReportNotification {


The interfaces ParentDeclarationRequiredParentCanReportNotification and ParentDeclarationRequiredParentCannotReportNotification are identical. Consider creating a single interface like ParentDeclarationRequiredNotification to reduce code duplication.

Suggested change

export interface ParentDeclarationRequiredParentCanReportNotification {

export interface ParentDeclarationRequiredNotification {

Copilot · 2025-07-15T21:44:36Z

...ackages/backend/libs/services/src/notifications/notification/notification-actions.service.ts

+    const auditUser = this.systemUsersService.systemUser;
+    const { templateId } =
+      await this.notificationMessageService.getNotificationMessageDetails(
+        NotificationMessageType.ParentDeclarationRequiredParentCanReportNotification,
+      );
+    const supportingUserInformationNotification = {
+      userId: notification.userId,
+      messageType:
+        NotificationMessageType.ParentDeclarationRequiredParentCanReportNotification,
+      messagePayload: {
+        email_address: notification.toAddress,
+        template_id: templateId,
+        personalisation: {
+          givenNames: notification.givenNames ?? "",
+          lastName: notification.lastName,
+          parentFullName: notification.parentFullName,
+          applicationNumber: notification.applicationNumber,
+          supportingUserType: notification.supportingUserType,
+        },
+      },
+    };
+    await this.notificationService.saveNotifications(
+      [supportingUserInformationNotification],
+      auditUser.id,
+      { entityManager },


The methods saveParentDeclarationRequiredParentCanReportNotification and saveParentDeclarationRequiredParentCannotReportNotification contain nearly identical logic. Consider extracting a common helper method that accepts the notification type as a parameter to reduce code duplication.

Suggested change

const auditUser = this.systemUsersService.systemUser;

const { templateId } =

await this.notificationMessageService.getNotificationMessageDetails(

NotificationMessageType.ParentDeclarationRequiredParentCanReportNotification,

);

const supportingUserInformationNotification = {

userId: notification.userId,

messageType:

NotificationMessageType.ParentDeclarationRequiredParentCanReportNotification,

messagePayload: {

email_address: notification.toAddress,

template_id: templateId,

personalisation: {

givenNames: notification.givenNames ?? "",

lastName: notification.lastName,

parentFullName: notification.parentFullName,

applicationNumber: notification.applicationNumber,

supportingUserType: notification.supportingUserType,

},

},

};

await this.notificationService.saveNotifications(

[supportingUserInformationNotification],

auditUser.id,

{ entityManager },

await this.saveParentDeclarationRequiredNotification(

NotificationMessageType.ParentDeclarationRequiredParentCanReportNotification,

notification,

entityManager,

Copilot · 2025-07-15T21:44:36Z

.../packages/backend/apps/workers/src/controllers/supporting-user/supporting-user.controller.ts

          jobLogger.error(message);
          return job.error(SUPPORTING_USER_FULL_NAME_NOT_RESOLVED, message);
        }
      }


The comment 'If the full name is not provided, use the student' appears to be incomplete or incorrect. The code logic doesn't show any fallback to using student information when full name is not provided.

Suggested change

}

}

// If the full name is not resolved, use the student's full name as a fallback.

if (!fullName) {

fullName = `${application.student.user.firstName} ${application.student.user.lastName}`;

}

Copilot · 2025-07-15T21:44:36Z

.../packages/backend/apps/workers/src/controllers/supporting-user/supporting-user.controller.ts

+          if (isAbleToReport) {
+            await this.notificationActionsService.saveParentDeclarationRequiredParentCanReportNotification(
+              {
+                givenNames: application.student.user.firstName,
+                lastName: application.student.user.lastName,
+                toAddress: application.student.user.email,
+                userId: application.student.user.id,
+                supportingUserType:
+                  job.variables.supportingUserType === "Parent"
+                    ? "parent"
+                    : "partner",
+                parentFullName: fullName,
+                applicationNumber: application.applicationNumber,
+              },
+              entityManager,
+            );
+          } else {
+            await this.notificationActionsService.saveParentDeclarationRequiredParentCannotReportNotification(
+              {
+                givenNames: application.student.user.firstName,
+                lastName: application.student.user.lastName,
+                toAddress: application.student.user.email,
+                userId: application.student.user.id,
+                supportingUserType:
+                  job.variables.supportingUserType === "Parent"
+                    ? "parent"
+                    : "partner",
+                parentFullName: fullName,
+                applicationNumber: application.applicationNumber,
+              },


The notification creation logic contains significant code duplication between the if and else blocks. Consider extracting the common notification data structure into a variable and only varying the notification service method call.

Suggested change

if (isAbleToReport) {

await this.notificationActionsService.saveParentDeclarationRequiredParentCanReportNotification(

{

givenNames: application.student.user.firstName,

lastName: application.student.user.lastName,

toAddress: application.student.user.email,

userId: application.student.user.id,

supportingUserType:

job.variables.supportingUserType === "Parent"

? "parent"

: "partner",

parentFullName: fullName,

applicationNumber: application.applicationNumber,

},

entityManager,

);

} else {

await this.notificationActionsService.saveParentDeclarationRequiredParentCannotReportNotification(

{

givenNames: application.student.user.firstName,

lastName: application.student.user.lastName,

toAddress: application.student.user.email,

userId: application.student.user.id,

supportingUserType:

job.variables.supportingUserType === "Parent"

? "parent"

: "partner",

parentFullName: fullName,

applicationNumber: application.applicationNumber,

},

const notificationData = {

givenNames: application.student.user.firstName,

lastName: application.student.user.lastName,

toAddress: application.student.user.email,

userId: application.student.user.id,

supportingUserType:

job.variables.supportingUserType === "Parent"

? "parent"

: "partner",

parentFullName: fullName,

applicationNumber: application.applicationNumber,

};

if (isAbleToReport) {

await this.notificationActionsService.saveParentDeclarationRequiredParentCanReportNotification(

notificationData,

entityManager,

);

} else {

await this.notificationActionsService.saveParentDeclarationRequiredParentCannotReportNotification(

notificationData,

...ps/db-migrations/src/sql/NotificationMessages/Insert-parent-declaration-required-message.sql

...rations/src/sql/NotificationMessages/Rollback-insert-parent-declaration-required-message.sql

.../packages/backend/apps/workers/src/controllers/supporting-user/supporting-user.controller.ts

dheepak-aot · 2025-07-16T15:58:36Z

Thanks for making the changes @sh16011993 . It is almost there. Added few comments.

bidyashish

Please review @sh16011993

bidyashish · 2025-07-16T16:19:17Z

sources/packages/backend/libs/services/src/notifications/notification/notification.model.ts

+  applicationNumber: string;
+  userId: number;
+  supportingUserType: NotificationSupportingUserType;
+}


What is the need of same interface data with different name, ParentInformationRequiredFromParentNotification and ParentInformationRequiredFromParentNotification?
It can be just 1 interface

For notifications specifically, we follow this pattern of creating a method with its interface irrespective of its data.

bidyashish · 2025-07-16T16:20:22Z

...ackages/backend/libs/services/src/notifications/notification/notification-actions.service.ts

+   * @param entityManager entity manager to execute in transaction.
+   */
+  async saveParentInformationRequiredFromParentNotification(
+    notification: ParentInformationRequiredFromParentNotification,


It can be just 1 function as it is repeating.
async saveParentInformationRequiredFromParentNotification...

#4927 (comment)

bidyashish · 2025-07-16T16:21:42Z

.../packages/backend/apps/workers/src/controllers/supporting-user/supporting-user.controller.ts

          return job.error(SUPPORTING_USER_FULL_NAME_NOT_RESOLVED, message);
        }
      }
+      const isAbleToReport = job.variables.isAbleToReport;


const isAbleToReport = job.variables.isAbleToReport; can be reverted.
and used as previous isAbleToReport: job.variables.isAbleToReport,

I created a constant because I had to use it in multiple places.

sonarqubecloud · 2025-07-16T16:23:47Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

bidyashish

please review

github-actions · 2025-07-16T16:24:22Z

Backend Unit Tests Coverage Report

Totals
Statements:	21.65% ( 4092 / 18903 )
Methods:	9.67% ( 234 / 2420 )
Lines:	25% ( 3539 / 14154 )
Branches:	13.7% ( 319 / 2329 )

github-actions · 2025-07-16T16:24:37Z

E2E Workflow Workers Coverage Report

Totals
Statements:	73.08% ( 725 / 992 )
Methods:	73.5% ( 86 / 117 )
Lines:	75.13% ( 568 / 756 )
Branches:	59.66% ( 71 / 119 )

github-actions · 2025-07-16T16:27:18Z

E2E Queue Consumers Coverage Report

Totals
Statements:	86.43% ( 1548 / 1791 )
Methods:	84.62% ( 176 / 208 )
Lines:	88.75% ( 1278 / 1440 )
Branches:	65.73% ( 94 / 143 )

github-actions · 2025-07-16T16:28:03Z

E2E SIMS API Coverage Report

Totals
Statements:	73.37% ( 7198 / 9811 )
Methods:	70.83% ( 884 / 1248 )
Lines:	76.83% ( 5576 / 7258 )
Branches:	56.55% ( 738 / 1305 )

dheepak-aot

Thanks for making the changes @sh16011993 Good job!

bidyashish

LGTM

sh16011993 added 2 commits July 14, 2025 11:51

#4823 - Parent Declare: Email Notification

a926eb4

Code Change

81d76b6

sh16011993 self-assigned this Jul 14, 2025

sh16011993 added Camunda Workers DB DB migration involved labels Jul 14, 2025

sh16011993 requested a review from Copilot July 14, 2025 23:03

This comment was marked as outdated.

Sign in to view

sh16011993 changed the title ~~4823 parent declare email notification~~ #4823 - Parent Declare: Email Notification Jul 14, 2025

sh16011993 changed the title ~~#4823 - Parent Declare: Email Notification~~ #4823 - Parent Declare: Email Notification (Part 1) Jul 15, 2025

dheepak-aot self-requested a review July 15, 2025 18:22