Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion js/src/features/emails/EmailSendPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
import { CsvUploader } from "@/features/emails/_components/CsvUploader";
import { EmailPreviewer } from "@/features/emails/_components/EmailPreviewer";
import { EmailSender } from "@/features/emails/_components/EmailSender";
import { SyncEmailSender } from "@/features/emails/_components/SyncEmailSender";
import { TemplateSelector } from "@/features/emails/_components/TemplateSelector";
import { Box, Flex, Stack } from "@mantine/core";
import { useState } from "react";
Expand Down Expand Up @@ -37,14 +38,16 @@ export function EmailSendPage() {
/>
{/* CSV uploader */}
<CsvUploader templateId={selectedTemplateId} setRequest={setRequest} />
{/* Send button */}
{/* Send Async button */}
<EmailSender
request={request}
selectedTemplateId={selectedTemplateId}
isSending={isSending}
setIsSending={setIsSending}
navigate={navigate}
/>
{/* Send Synchronous button */}
<SyncEmailSender request={request} />
</Stack>
<Box w="70%">
{/* Preview */}
Expand Down
2 changes: 1 addition & 1 deletion js/src/features/emails/_components/EmailSender.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export function EmailSender({
loading={isSending}
fullWidth
>
Send Emails
Send Asynchronous Emails
</Button>
);
}
115 changes: 115 additions & 0 deletions js/src/features/emails/_components/SyncEmailSender.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
import type {
SendAsyncRequest,
SendRequest,
EmailTemplate,
} from "@/features/emails/dto/emailDto";

import { listTemplates, sendToEmailApi } from "@/features/emails/api/emailAPI";
import {
showEmailError,
showEmailPending,
showEmailSuccess,
} from "@/features/emails/api/emailError";
import { Button, Flex, Text } from "@mantine/core";
import { modals } from "@mantine/modals";
import { useMutation } from "@tanstack/react-query";
import { useEffect, useState } from "react";

export function SyncEmailSender({
request,
}: {
request: SendAsyncRequest | null;
}) {
const [template, setTemplate] = useState<EmailTemplate | null>(null);

const mutation = useMutation({
mutationFn: async (req: SendRequest) => sendToEmailApi(req),
});

useEffect(() => {
// Find template subject and body from request.templateId
if (!request?.templateId) {
setTemplate(null);
return;
}

const loadTemplate = async () => {
try {
const templates = await listTemplates();
const found = templates.find((t) => t.id === request.templateId);
setTemplate(found ?? null);
} catch {
setTemplate(null);
}
};

void loadTemplate();
}, [request?.templateId]);
Comment on lines +29 to +47

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical Race Condition: If the user rapidly changes templates, multiple loadTemplate() calls will be in flight simultaneously and may resolve out of order, causing the wrong template to be set.

Scenario:

  1. User selects Template A → starts loading
  2. User quickly switches to Template B → starts loading
  3. Template B finishes loading → sets template to B
  4. Template A finishes loading → sets template to A (WRONG!)
  5. User clicks send → emails sent with Template A's subject/body instead of B

Fix: Add cleanup to ignore stale results:

useEffect(() => {
  if (!request?.templateId) {
    setTemplate(null);
    return;
  }

  let cancelled = false;

  const loadTemplate = async () => {
    try {
      const templates = await listTemplates();
      const found = templates.find((t) => t.id === request.templateId);
      if (!cancelled) {
        setTemplate(found ?? null);
      }
    } catch {
      if (!cancelled) {
        setTemplate(null);
      }
    }
  };

  void loadTemplate();

  return () => {
    cancelled = true;
  };
}, [request?.templateId]);
Suggested change
useEffect(() => {
// Find template subject and body from request.templateId
if (!request?.templateId) {
setTemplate(null);
return;
}
const loadTemplate = async () => {
try {
const templates = await listTemplates();
const found = templates.find((t) => t.id === request.templateId);
setTemplate(found ?? null);
} catch {
setTemplate(null);
}
};
void loadTemplate();
}, [request?.templateId]);
useEffect(() => {
// Find template subject and body from request.templateId
if (!request?.templateId) {
setTemplate(null);
return;
}
let cancelled = false;
const loadTemplate = async () => {
try {
const templates = await listTemplates();
const found = templates.find((t) => t.id === request.templateId);
if (!cancelled) {
setTemplate(found ?? null);
}
} catch {
if (!cancelled) {
setTemplate(null);
}
}
};
void loadTemplate();
return () => {
cancelled = true;
};
}, [request?.templateId]);

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


useEffect(() => {
if (mutation.status === "pending") {
showEmailPending("pending", "Email sending in progress...");
}
}, [mutation.status]);

const openModal = () =>
modals.openConfirmModal({
title: "Email Send Confirmation",
children: (
<Text size="sm">
Please confirm that you want to send {request?.messages.length} email
{request?.messages.length === 1 ? "" : "s"}.
</Text>
),
labels: { confirm: "Confirm", cancel: "Cancel" },
onCancel: () =>
showEmailPending(
"Cancel",
`${request?.messages.length} Emails cancelled.`,
),
onConfirm: () => void handleSend(),
});

const handleSend = async () => {
if (!request) {
showEmailError("Preview first", "Preview before sending.");
return;
}

if (!template) {
showEmailError(
"Template missing",
"Could not load the template subject/body.",
);
return;
}
// Convert SendAsyncRequest to SendRequest
const syncRequest: SendRequest = {
templateId: request.templateId,
subject: template.subject,
body: template.body,
replyTo: request.replyTo ?? null,
messages: request.messages,
};

try {
await mutation.mutateAsync(syncRequest);
showEmailSuccess("Success", "Emails sent.");
} catch {
showEmailError("Error", "Unable to send emails.");
}
};

return (
<Flex>
<Button
fullWidth
loading={mutation.isPending}
onClick={openModal}
disabled={!request || !template}
>
Send Synchronous Emails
</Button>
</Flex>
);
}
1 change: 1 addition & 0 deletions js/src/features/emails/dto/emailDto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export interface MessagePreview {
}

export interface SendRequest {
templateId: string;
subject: string;
body: string;
replyTo: string | null;
Expand Down
61 changes: 55 additions & 6 deletions src/main/java/org/patinanetwork/patchats/email/EmailService.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
package org.patinanetwork.patchats.email;

import java.time.LocalDate;
import java.time.ZoneId;
import java.time.format.TextStyle;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.UUID;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.patinanetwork.patchats.common.web.exception.EmailTemplateNotFoundException;
import org.patinanetwork.patchats.email.db.models.Email;
import org.patinanetwork.patchats.email.db.models.EmailRequest;
import org.patinanetwork.patchats.email.db.models.EmailSource;
import org.patinanetwork.patchats.email.db.models.EmailStatus;
import org.patinanetwork.patchats.email.db.models.EmailTemplate;
import org.patinanetwork.patchats.email.db.repos.EmailRepo;
import org.patinanetwork.patchats.email.db.repos.EmailRequestRepo;
import org.patinanetwork.patchats.email.db.repos.EmailTemplateRepo;
import org.patinanetwork.patchats.email.dto.PreviewEmailResponse;
import org.patinanetwork.patchats.email.dto.SendEmailRequest;
import org.patinanetwork.patchats.email.dto.SendEmailResponse;
Expand All @@ -22,8 +36,10 @@
@RequiredArgsConstructor
@Slf4j
public class EmailService {

private final TemplateRenderer renderer;
private final EmailTemplateRepo templateRepo;
private final EmailRequestRepo requestRepo;
private final EmailRepo emailRepo;
private final TemplateRenderer templateRenderer;
private final EmailSender sender;

public SendEmailResponse send(final SendEmailRequest request) {
Expand All @@ -38,8 +54,8 @@ public SendEmailResponse send(final SendEmailRequest request) {
.toList();
try {
final Map<String, String> variables = mergeVariables(message.variables(), message.recipients());
final String subject = renderer.render(request.subject(), variables);
final String body = renderer.render(request.body(), variables);
final String subject = templateRenderer.render(request.subject(), variables);
final String body = templateRenderer.render(request.body(), variables);
sender.send(new OutgoingEmail(recipients, subject, body, replyTo));
log.info("Sent email to {}", recipients);
results.add(new SendEmailResponse.MessageResult(recipients, true, null));
Expand All @@ -50,7 +66,40 @@ public SendEmailResponse send(final SendEmailRequest request) {
failed++;
}
}
final EmailTemplate template = templateRepo
.findById(request.templateId())
.orElseThrow(() -> new EmailTemplateNotFoundException(request.templateId()));

final UUID requestId = UUID.randomUUID();
requestRepo.insert(EmailRequest.builder()
.id(requestId)
.source(EmailSource.SYNCHRONOUS)
.templateId(template.getId())
.totalCount(request.messages().size())
.build());

// Fill in the send-time month once for the whole batch so ${month} resolves consistently. Callers can
// still override it by passing an explicit "month" variable (putIfAbsent below leaves theirs untouched).
final String currentMonth =
LocalDate.now(ZoneId.of("America/New_York")).getMonth().getDisplayName(TextStyle.FULL, Locale.ENGLISH);
final List<Email> emails = new ArrayList<>(request.messages().size());
for (final SendEmailRequest.Message message : request.messages()) {
final Map<String, String> variables =
EmailService.mergeVariables(message.variables(), message.recipients());
variables.putIfAbsent("month", currentMonth);
final List<SendEmailRequest.Recipient> recipients = message.recipients();
emails.add(Email.builder()
.id(UUID.randomUUID())
.requestId(requestId)
.recipient1(recipients.get(0).email())
.recipient2(recipients.size() > 1 ? recipients.get(1).email() : null)
.replyTo(request.replyTo())
.templateId(template.getId())
.templateValues(variables)
.status(EmailStatus.SENT)
.build());
}
emailRepo.insertAll(emails);
return new SendEmailResponse(sent, failed, results);
}

Expand All @@ -66,8 +115,8 @@ public PreviewEmailResponse preview(final SendEmailRequest request) {
.toList();
try {
final Map<String, String> variables = mergeVariables(message.variables(), message.recipients());
final String subject = renderer.render(request.subject(), variables);
final String body = renderer.render(request.body(), variables);
final String subject = templateRenderer.render(request.subject(), variables);
final String body = templateRenderer.render(request.body(), variables);
previews.add(new PreviewEmailResponse.MessagePreview(recipients, subject, body, null));
} catch (final RuntimeException ex) {
previews.add(new PreviewEmailResponse.MessagePreview(recipients, null, null, ex.getMessage()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
/** Which producer enqueued a sending session. */
public enum EmailSource {
MANUAL,
MATCHING
MATCHING,
SYNCHRONOUS,
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@
import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotEmpty;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Size;
import java.util.List;
import java.util.Map;
import java.util.UUID;

/**
* Request to send one or more templated plain-text emails. {@code subject} and {@code body} are templates shared across
* all messages; each message supplies the variables to merge in.
*/
public record SendEmailRequest(
@NotNull UUID templateId,
@NotBlank String subject,
@NotBlank String body,
@Email String replyTo,
Expand Down