Merge pull request #67 from xtr-dev/dev

Fix ObjectId casting error when jobs relationship is populated

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@xtr-dev/payload-mailing",
-  "version": "0.4.16",
+  "version": "0.4.19",
   "description": "Template-based email system with scheduling and job processing for PayloadCMS",
   "type": "module",
   "main": "dist/index.js",

src/collections/Emails.ts

@@ -12,7 +12,7 @@ const Emails: CollectionConfig = {
     description: 'Email delivery and status tracking',
   },
   defaultPopulate: {
-    template: true,
+    templateSlug: true,
     to: true,
     cc: true,
     bcc: true,
@@ -40,6 +40,14 @@ const Emails: CollectionConfig = {
         description: 'Email template used (optional if custom content provided)',
       },
     },
+    {
+      name: 'templateSlug',
+      type: 'text',
+      admin: {
+        description: 'Slug of the email template (auto-populated from template relationship)',
+        readOnly: true,
+      },
+    },
     {
       name: 'to',
       type: 'text',
@@ -208,6 +216,27 @@ const Emails: CollectionConfig = {
     },
   ],
   hooks: {
+    beforeChange: [
+      async ({ data, req }) => {
+        // Auto-populate templateSlug from template relationship
+        if (data.template) {
+          try {
+            const template = await req.payload.findByID({
+              collection: 'email-templates',
+              id: typeof data.template === 'string' ? data.template : data.template.id,
+            })
+            data.templateSlug = template.slug
+          } catch (error) {
+            // If template lookup fails, clear the slug
+            data.templateSlug = undefined
+          }
+        } else {
+          // Clear templateSlug if template is removed
+          data.templateSlug = undefined
+        }
+        return data
+      }
+    ],
     // Simple approach: Only use afterChange hook for job management
     // This avoids complex interaction between hooks and ensures document ID is always available
     afterChange: [

src/sendEmail.ts

@@ -1,5 +1,5 @@
 import { Payload } from 'payload'
-import { getMailing, renderTemplate, parseAndValidateEmails, sanitizeFromName } from './utils/helpers.js'
+import { getMailing, renderTemplateWithId, parseAndValidateEmails, sanitizeFromName } from './utils/helpers.js'
 import { BaseEmailDocument } from './types/index.js'
 import { processJobById } from './utils/emailProcessor.js'
 import { createContextLogger } from './utils/logger.js'
@@ -50,7 +50,8 @@ export const sendEmail = async <TEmail extends BaseEmailDocument = BaseEmailDocu
   let emailData: Partial<TEmail> = { ...options.data } as Partial<TEmail>
 
   if (options.template) {
-    const { html, text, subject } = await renderTemplate(
+    // Look up and render the template in a single operation to avoid duplicate lookups
+    const { html, text, subject, templateId } = await renderTemplateWithId(
       payload,
       options.template.slug,
       options.template.variables || {}
@@ -58,6 +59,7 @@ export const sendEmail = async <TEmail extends BaseEmailDocument = BaseEmailDocu
 
     emailData = {
       ...emailData,
+      template: templateId,
       subject,
       html,
       text,

src/services/MailingService.ts

@@ -127,6 +127,17 @@ export class MailingService implements IMailingService {
       throw new Error(`Email template not found: ${templateSlug}`)
     }
 
+    return this.renderTemplateDocument(template, variables)
+  }
+
+  /**
+   * Render a template document (for when you already have the template loaded)
+   * This avoids duplicate template lookups
+   * @internal
+   */
+  async renderTemplateDocument(template: BaseEmailTemplateDocument, variables: TemplateVariables): Promise<{ html: string; text: string; subject: string }> {
+    this.ensureInitialized()
+
     const emailContent = await this.renderEmailTemplate(template, variables)
     const subject = await this.renderTemplateString(template.subject || '', variables)
 

src/types/index.ts

@@ -14,6 +14,7 @@ export type JSONValue = string | number | boolean | { [k: string]: unknown } | u
 export interface BaseEmailDocument {
   id: string | number
   template?: any
+  templateSlug?: string | null
   to: string[]
   cc?: string[] | null
   bcc?: string[] | null

src/utils/helpers.ts

@@ -130,6 +130,53 @@ export const renderTemplate = async (payload: Payload, templateSlug: string, var
   return mailing.service.renderTemplate(templateSlug, variables)
 }
 
+/**
+ * Render a template and return both rendered content and template ID
+ * This is used by sendEmail to avoid duplicate template lookups
+ * @internal
+ */
+export const renderTemplateWithId = async (
+  payload: Payload,
+  templateSlug: string,
+  variables: TemplateVariables
+): Promise<{ html: string; text: string; subject: string; templateId: PayloadID }> => {
+  const mailing = getMailing(payload)
+  const templatesCollection = mailing.config.collections?.templates || 'email-templates'
+
+  // Runtime validation: Ensure the collection exists in Payload
+  if (!payload.collections[templatesCollection]) {
+    throw new Error(
+      `Templates collection '${templatesCollection}' not found. ` +
+      `Available collections: ${Object.keys(payload.collections).join(', ')}`
+    )
+  }
+
+  // Look up the template document once
+  const { docs: templateDocs } = await payload.find({
+    collection: templatesCollection as any,
+    where: {
+      slug: {
+        equals: templateSlug,
+      },
+    },
+    limit: 1,
+  })
+
+  if (!templateDocs || templateDocs.length === 0) {
+    throw new Error(`Template not found: ${templateSlug}`)
+  }
+
+  const templateDoc = templateDocs[0]
+
+  // Render using the document directly to avoid duplicate lookup
+  const rendered = await mailing.service.renderTemplateDocument(templateDoc, variables)
+
+  return {
+    ...rendered,
+    templateId: templateDoc.id,
+  }
+}
+
 export const processEmails = async (payload: Payload): Promise<void> => {
   const mailing = getMailing(payload)
   return mailing.service.processEmails()

src/utils/jobScheduler.ts

@@ -129,7 +129,11 @@ export async function updateEmailJobRelationship(
       id: normalizedEmailId,
     })
 
-    const currentJobs = (currentEmail.jobs || []).map((job: any) => String(job))
+    // Extract IDs from job objects or use the value directly if it's already an ID
+    // Jobs can be populated (objects with id field) or just IDs (strings/numbers)
+    const currentJobs = (currentEmail.jobs || []).map((job: any) =>
+      typeof job === 'object' && job !== null && job.id ? String(job.id) : String(job)
+    )
     const allJobs = [...new Set([...currentJobs, ...normalizedJobIds])] // Deduplicate with normalized strings
 
     await payload.update({