Skip to content

Ks 024 sep 23 email notification 1959#2214

Merged
MuhammadKhalilzadeh merged 7 commits intodevelopfrom
ks-024-Sep-23-email-notification-1959
Sep 26, 2025
Merged

Ks 024 sep 23 email notification 1959#2214
MuhammadKhalilzadeh merged 7 commits intodevelopfrom
ks-024-Sep-23-email-notification-1959

Conversation

@solan117
Copy link
Copy Markdown
Contributor

@solan117 solan117 commented Sep 25, 2025

Describe your changes

feat(notifications): add user-added-as-admin notification flow

  • Introduced sendUserAddedAdminNotification service to notify users when added as project admins.
  • Added new MJML email template user-added-admin.mjml for notification styling.
  • Integrated the service into updateProjectById to trigger notifications on project owner changes.
  • Adjusted projectCreationNotification path for consistency.

Write your issue number after "Fixes "

Fixes #1959

Please ensure all items are checked off before requesting a review:

  • I deployed the code locally.
  • I have performed a self-review of my code.
  • I have included the issue # in the PR.
  • I have labelled the PR correctly.
  • The issue I am working on is assigned to me.
  • I have avoided using hardcoded values to ensure scalability and maintain consistency across the application.
  • I have ensured that font sizes, color choices, and other UI elements are referenced from the theme.
  • My pull request is focused and addresses a single, specific feature.
  • If there are UI changes, I have attached a screenshot or video to this PR.

- Introduced `sendUserAddedAdminNotification` service to notify users when added as project admins.
- Added new MJML email template `user-added-admin.mjml` for notification styling.
- Integrated the service into `updateProjectById` to trigger notifications on project owner changes.
- Adjusted `projectCreationNotification` path for consistency.
@solan117 solan117 added this to the 1.4 milestone Sep 25, 2025
@solan117 solan117 self-assigned this Sep 25, 2025
@solan117 solan117 added the backend Backend related tasks/issues label Sep 25, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Adds a new "user added as project admin" notification service and MJML template, updates the project controller to detect owner changes and fire the notification (non-blocking with error logging), adjusts several import paths, and makes a minor import formatting change in the UI. No other behavior changes reported.

Changes

Cohort / File(s) Summary
UI import formatting
Clients/src/presentation/containers/Dashboard/index.tsx
Minor import/line-break reformat; no runtime or behavioral changes.
Project controller notification hooks
Servers/controllers/project.ctrl.ts
Fetches current project before update to detect owner change; computes ownerChanged and fire-and-forget calls sendUserAddedAdminNotification (catches and logs errors). Adjusts import paths for project creation notification.
Notification import path updates
Servers/services/projectNotification/projectCreationNotification.ts
Adjusted relative import paths for notificationService, getUserByIdQuery, frontEndUrl, and logger; no logic changes.
New user-admin notification service
Servers/services/userNotification/userAddedAdminNotification.ts
New interface UserAddedAdminNotification and async sendUserAddedAdminNotification(data) that: validates admin/user and email, builds project URL, composes template data, calls notificationService.sendEmailWithTemplate('user-added-admin.mjml', ...), logs success/failure and rethrows on error.
New email template
Servers/templates/user-added-admin.mjml
Adds MJML template with placeholders user_name, project_name, actor_name, project_url, CTA button and footer.
Template structure rework
Servers/templates/project-created-admin.mjml
Reworks MJML layout: uses mj-section with text-align="left" and padding="0px", mj-text with align="left", and simplifies wrapper structure (removed intermediate column pairing); content unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Controller as project.ctrl.updateProjectById
  participant DB as Project Store
  participant Notify as sendUserAddedAdminNotification
  Note over Controller: Update project flow with owner-change detection

  Client->>Controller: PUT /projects/:id { ownerId, ... }
  Controller->>DB: getProjectById(id)
  DB-->>Controller: currentProject
  Controller->>DB: updateProjectById(id, payload)
  DB-->>Controller: updatedProject

  alt owner changed
    Note over Controller,Notify: Fire-and-forget notification (non-blocking)
    Controller--)Notify: sendUserAddedAdminNotification({ projectId, projectName, adminId, userId })
    Note right of Notify: Fetch admin/user → validate email → build project URL → send templated email → log outcome
  else no change
    Note over Controller: Skip notification
  end

  Controller-->>Client: 200 OK updatedProject
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

I twitch my nose at code that sings,
A new admin bell with flapping wings.
Templates stitched, the message sped,
From burrow to inbox — carrots led.
Hop on — the notification springs! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The pull request only implements the “User Added As Project Admin” notification and adjusts an import path for project creation, but it does not add the other required email workflows for editor, reviewer, auditor roles, role-change notifications, or policy due reminders specified in issue #1959. Implement the remaining notification workflows outlined in issue #1959—editor, reviewer, auditor, role change, and policy due reminders—to fully satisfy the linked issue requirements.
Out of Scope Changes Check ⚠️ Warning The minor formatting adjustments in Clients/src/presentation/containers/Dashboard/index.tsx are unrelated to the notification functionality and fall outside the scope of issue #1959. Move unrelated formatting changes into a separate cleanup pull request to keep this PR focused on notification feature implementation.
Title Check ❓ Inconclusive The title “Ks 024 sep 23 email notification 1959” is cryptic and primarily reflects a branch or ticket reference rather than clearly describing the main change—adding a user-added-as-admin notification flow. As a result it does not concisely convey the primary feature implemented. Rename the title to a clear, concise description of the main change, for example: “feat(notifications): send email when user is added as project admin (#1959)”.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed The PR description follows the repository template by providing a detailed “Describe your changes” section, specifying the issue fixed with “Fixes #1959,” and completing the required checklist items, so it meets the standard structure and content requirements.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ks-024-Sep-23-email-notification-1959

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb7fda3 and 34c0fa4.

📒 Files selected for processing (2)
  • Servers/templates/project-created-admin.mjml (1 hunks)
  • Servers/templates/user-added-admin.mjml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Servers/templates/user-added-admin.mjml

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

# Conflicts:
#	Clients/src/presentation/pages/Framework/index.tsx
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (5)
Servers/services/userNotification/userAddedAdminNotification.ts (2)

31-35: Consider graceful fallback for missing actor name.
If adminUser.name is null/empty, render a generic actor label to avoid a blank in the email body.

Apply this minimal guard:

-      actor_name: adminUser.name,
+      actor_name: adminUser.name || 'an administrator',

66-71: Avoid logging full email addresses (PII) in success logs.
Mask or log userId instead to reduce PII in logs.

Apply this diff to mask the email:

-    await logSuccess({
-      eventType: "Update",
-      description: `Added as Admin notification sent to ${user.email}`,
+    const masked = user.email.replace(/(.).+(@.*)/, '$1***$2');
+    await logSuccess({
+      eventType: "Update",
+      description: `Added as Admin notification sent to ${masked}`,
       functionName: "sendUserAddedAdminNotification",
       fileName: "userAddedAdminNotification.ts",
     });
Servers/templates/user-added-admin.mjml (1)

14-23: Nice template; add preview text and use mj-button for better email client support.
Improves inbox preview and button rendering across clients.

Apply this diff:

   <mj-head>
+    <mj-preview>You are now a project admin for {{project_name}}</mj-preview>
@@
-          <p style="margin: 20px 0;">
-            <a href="{{project_url}}" style="background-color: #3498db; color: white; padding: 12px 24px; text-decoration: none; border-radius: 4px; display: inline-block;">Open project</a>
-          </p>
+          <mj-button href="{{project_url}}" background-color="#3498db" color="#ffffff" font-weight="500" border-radius="4px" padding="20px 0">
+            Open project
+          </mj-button>

Optional: add a plaintext fallback link below the button for strict clients.

Servers/services/projectNotification/projectCreationNotification.ts (1)

59-64: Reduce PII in logs by masking admin email.
Align logging hygiene with other services.

-    await logSuccess({
-      eventType: "Create",
-      description: `Project creation notification sent to ${adminUser.email}`,
+    const masked = adminUser.email.replace(/(.).+(@.*)/, '$1***$2');
+    await logSuccess({
+      eventType: "Create",
+      description: `Project creation notification sent to ${masked}`,
       functionName: "sendProjectCreatedNotification",
       fileName: "projectCreationNotification.ts",
     });
Servers/controllers/project.ctrl.ts (1)

254-257: ownerChanged may be non-boolean; coerce explicitly.
Current expression can be null when currentProject is null. Coerce to boolean for clarity.

-    const ownerChanged = currentProject && currentProject.owner !== updatedProject.owner;
+    const ownerChanged = !!(currentProject && currentProject.owner !== updatedProject.owner);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a6fcdb and 1576526.

📒 Files selected for processing (5)
  • Clients/src/presentation/containers/Dashboard/index.tsx (1 hunks)
  • Servers/controllers/project.ctrl.ts (3 hunks)
  • Servers/services/projectNotification/projectCreationNotification.ts (1 hunks)
  • Servers/services/userNotification/userAddedAdminNotification.ts (1 hunks)
  • Servers/templates/user-added-admin.mjml (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Servers/services/userNotification/userAddedAdminNotification.ts (3)
Servers/utils/logger/logHelper.ts (3)
  • logProcessing (22-30)
  • logSuccess (33-49)
  • logFailure (51-68)
Servers/utils/user.utils.ts (1)
  • getUserByIdQuery (137-144)
Servers/services/notificationService.ts (1)
  • notificationService (110-110)
Servers/controllers/project.ctrl.ts (3)
Servers/utils/project.utils.ts (1)
  • getProjectByIdQuery (114-159)
Servers/services/userNotification/userAddedAdminNotification.ts (1)
  • sendUserAddedAdminNotification (19-84)
Servers/utils/logger/logHelper.ts (1)
  • logFailure (51-68)
🔇 Additional comments (3)
Servers/services/userNotification/userAddedAdminNotification.ts (1)

19-27: Good: non-blocking, structured logging before send.
Pattern matches existing notification flows and keeps the response path clean.

Servers/services/projectNotification/projectCreationNotification.ts (1)

1-3: Import path adjustments look correct.
Paths resolve one/two levels up as expected from services/projectNotification.

Clients/src/presentation/containers/Dashboard/index.tsx (1)

6-7: Import path and VerifyWiseContext export are correct. No changes needed.

Comment thread Servers/controllers/project.ctrl.ts
…n emails

- Updated `user-added-admin.mjml` and `project-created-admin.mjml` templates to set body width to 100% and standardize section padding for consistent styling.
Copy link
Copy Markdown
Collaborator

@MuhammadKhalilzadeh MuhammadKhalilzadeh left a comment

Choose a reason for hiding this comment

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

Thank you @solan117

@MuhammadKhalilzadeh MuhammadKhalilzadeh merged commit 0d04c42 into develop Sep 26, 2025
3 checks passed
@MuhammadKhalilzadeh MuhammadKhalilzadeh deleted the ks-024-Sep-23-email-notification-1959 branch September 26, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Backend related tasks/issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implementing email notifications

2 participants