Add "To:" line and "Send to me" to the shared email composer - #1801
Conversation
Closes #1793. - EmailComposerViewModel: RecipientCount/RecipientSummary/RecipientUserId and WarnRecipientCount (default 25). Large sends render as a warning and the host form's submit asks for confirmation naming the count. - Send to me: POST /Email/SendMarkdownToSelf (any authenticated human, rate-limited 5/min per human) queues one composer_self_test System email to the caller's own address and audits EmailComposerSelfTestSent. - All eight composer call sites and the Widget Gallery pass their audience. - Strings in all six cultures; Email/AuditLog docs updated; tests cover the happy, signed-out and no-address paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1CXvfQwWKLEYUcvUHtUf5
PR Surface ReportCompared Summary: 46 changed file(s) | EF migrations: 0 added file(s), max 0/1 per context Reforge Surface Score
Section Deltas
Section Size & Complexity Deltas
Rule Deltas
Corpus Size & Complexity
At head: largest class Published Write Surface14 of 48 sections publish write capability, 23 interfaces (0). Interface SurfaceNo new interfaces or interface methods. Diff Size
New Files
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| internal sealed class ComposerSelfSendService( | ||
| IUserEmailService userEmailService, | ||
| IUserServiceRead userService, | ||
| IEmailService emailService, | ||
| IAuditLogService audit, | ||
| IStringLocalizer<EmailResource> localizer, | ||
| ILogger<ComposerSelfSendService> logger) | ||
| { | ||
| public const string TemplateName = "composer_self_test"; | ||
|
|
||
| /// <summary> | ||
| /// Queues one outbox row to <paramref name="userId"/> and audits it. Returns the address it was | ||
| /// queued to, or null when the human has no notification address (nothing is queued). | ||
| /// </summary> | ||
| public async Task<string?> SendToSelfAsync( | ||
| Guid userId, string? subject, string? markdownBody, CancellationToken ct = default) | ||
| { | ||
| var emails = await userEmailService.GetNotificationTargetEmailsAsync([userId], ct); | ||
| if (!emails.TryGetValue(userId, out var email) || string.IsNullOrWhiteSpace(email)) | ||
| return null; | ||
|
|
||
| var user = await userService.GetUserInfoAsync(userId, ct); | ||
| var fullSubject = string.IsNullOrWhiteSpace(subject) | ||
| ? localizer["Email_ComposerSelfTest_DefaultSubject"].Value | ||
| : string.Format(CultureInfo.CurrentCulture, localizer["Email_ComposerSelfTest_Subject"].Value, subject.Trim()); | ||
|
|
||
| await emailService.SendAsync(new EmailMessage( | ||
| email, user?.BurnerName, fullSubject, SanitizedMarkdownRenderer.Render(markdownBody), | ||
| TemplateName, MessageCategory.System, UserId: userId), ct); |
There was a problem hiding this comment.
BLOCK — builds a second template directly in the Email crosscut
memory/architecture/email-templates-live-in-sender.md: "Adding or editing an email template: build it in the sending section... never in the Email crosscut... FacilitatedMessage is the one template that stays in Email — a generic person-to-person relay with no section vocabulary... Never add a method to IEmailMessageFactory, and never add an Email_* key to EmailResource.*.resx."
This PR adds Email_ComposerSelfTest_Subject / Email_ComposerSelfTest_DefaultSubject to EmailResource.*.resx (all six cultures) and has ComposerSelfSendService, living in Humans.Email itself, build and send a second self-contained EmailMessage/template (composer_self_test) straight from the crosscut — exactly the pattern the atom says is closed off, with FacilitatedMessage as the sole, deliberately-signed-off exception (#1651).
If the composer's "Send to me" is meant to be a second sanctioned exception (it does have some claim to being generic, composer-widget infrastructure rather than section vocabulary, similar to the existing PreviewMarkdown seam), that needs the same explicit design sign-off FacilitatedMessage got, plus an update to this atom recording the new exception — not a silent second instance. As written, the rule forbids this.
There was a problem hiding this comment.
Left open for Peter: a design call, not something to decide in a review round. The finding is literally true: email-templates-live-in-sender forbids new Email_* keys in EmailResource and names FacilitatedMessage as the only template that stays in Email. But the "sending section" here is Email itself. The composer widget and EmailPreviewController live there, and the copy is generic ([Test] {subject}) with no section vocabulary, so there is no other owner to move it to. @peterdrier, choose one: (a) sign off on composer_self_test as a second exception and have the atom record it, or (b) rework it, e.g. send without a subject prefix so no new keys are needed.
Generated by Claude Code
There was a problem hiding this comment.
yes, a path for send to me is fine as a centralized exception to the rule. otherwise each section would need that, duplicating code.
|
Reviewed commit cf3301f. 2 inline finding(s) posted. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf3301f29c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
UseRateLimiter ran before UseAuthentication, so partition keys never saw the signed-in NameIdentifier: the composer's per-human EmailComposerSelfSend policy collapsed into one site-wide "anonymous" bucket (and the global limiter keyed every signed-in human by IP). Move it after authentication. Review-round: 1 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DbnqiSqXV7Abo153ieThBq
2 participants
What
_EmailComposergets a read-only "To:" line and a "Send to me" button, so every compose form shows who it reaches and can send a real branded test to the sender.Why
Recipient counts and send-to-self existed only on some pages and looked different on each. The rule is "same everywhere".
Existing surface checked
EmailComposerViewModel/_EmailComposer/_EmailPreviewModalJS. No new component.EmailPreviewControllerbesidePreviewMarkdown.ComposerSelfSendService(scoped).EmailPreviewServiceis a singleton, so it can't takeIEmailService/audit. Not a public surface.IUserEmailService.GetNotificationTargetEmailsAsync(same as Surveys' self-send),SanitizedMarkdownRenderer,IEmailService.SendAsyncandIAuditLogService.AuditAction.EmailComposerSelfTestSent, plus theEmailComposerSelfSendrate-limit policy (registered in Email'sSection.cs).UI changes / screenshots
Check it on the preview deploy. The Widget Gallery entry shows a 42-recipient warning example.
<vc:human>(profile message, feedback reply, issue comment to reporter), a summary (camp, rota, team), and/or a count. At 25 or more recipients it turns intoalert-warningand the host form asks for confirmation on submit.Checklist
mainonpeterdrier/Humansorigin/mainEF migrations: none (AuditAction is an enum value only)NuGet packages updated?: noNew project rule?: noNav coverage: no new pagesReviewer notes
RecipientCountis nullable, not required. The survey invitation builder and the campaign form edit templates. Their audience is chosen later, so they show only a summary. Everything else passes a count.formnovalidate. That covers the team-rotas "refresh recipients" button, which submits the same form.MessageCategory.System, so there is no unsubscribe footer. As the issue accepts, sections that wrap the body in extra content (rota table, survey link) won't show that content.Closes
Closes #1793
🤖 Generated with Claude Code
https://claude.ai/code/session_01M1CXvfQwWKLEYUcvUHtUf5
Generated by Claude Code