Skip to content

follow-up: address remaining review issues from PR #124 (mail templates) #126

@coderabbitai

Description

@coderabbitai

Follow-up from PR #124

This issue tracks three remaining concerns flagged during the review of PR #124 that were not fully addressed before merge.

Requested by: @yash-pouranik | Assigned to: @copilot


1. Raw DB/Mongoose errors exposed in mail template controllers

File: apps/dashboard-api/src/controllers/project.controller.js

Multiple catch blocks in the new mail template endpoints return raw err.message to the client (e.g. in listMailTemplates, getMailTemplate, createMailTemplate, updateMailTemplate, deleteMailTemplate). This can leak internal database error details.

Fix: Replace raw error exposure with a generic message, conditionally surfacing details only in development mode — consistent with the pattern used in deleteFile:

res.status(500).json({
  error: "Internal server error",
  details: process.env.NODE_ENV === "development" ? err.message : undefined,
});

2. Missing test coverage for MailTemplate DB lookup paths

File: apps/public-api/src/__tests__/mail.controller.test.js

The MailTemplate.findOne mock always returns null, so the new project-scoped and global/system MailTemplate resolution paths are never exercised in tests. Only the legacy project.mailTemplates fallback is tested.

Fix: Add at least two test cases:

  • One where MailTemplate.findOne returns a project-scoped template (verify templateUsed.scope === 'project').
  • One where the first findOne call returns null (no project template) but the second returns a global/system template (verify templateUsed.scope === 'global').

3. SDK to type mismatch vs. backend validation

Files: sdks/urbackend-sdk/src/types/index.ts · packages/common/src/utils/input.validation.js

The SDK declares to: string | string[] but the backend Zod schema only accepts a single email string (z.string().email()). Passing an array will fail validation.

Fix (choose one):

  • Update SDK type to to: string to match the current backend, or
  • Update the backend schema to z.union([z.string().email(), z.array(z.string().email())]) and handle the array case in the mail controller.

Backlinks: PR #124, Review thread

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions