Skip to content

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

@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


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

No one assigned

    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