Skip to content

Emails: Add button to send emails synchronously. - #88

Open
isabellalam12 wants to merge 1 commit into
mainfrom
08-24-emails-add-sync-button
Open

Emails: Add button to send emails synchronously.#88
isabellalam12 wants to merge 1 commit into
mainfrom
08-24-emails-add-sync-button

Conversation

@isabellalam12

Copy link
Copy Markdown
Member

No description provided.

This was referenced Aug 26, 2026

isabellalam12 commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch 2 times, most recently from 5aea21b to a6da2c8 Compare August 26, 2026 17:55
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-remove-defaults branch from 67fb00b to 89ca9db Compare August 26, 2026 17:55
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch from a6da2c8 to 76bf469 Compare August 26, 2026 19:11
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-remove-defaults branch 3 times, most recently from 9d0d65f to 8f76576 Compare August 26, 2026 19:27
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch from 76bf469 to cd00456 Compare August 26, 2026 19:27
Comment thread js/src/features/emails/_components/SyncEmailSender.tsx
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-remove-defaults branch from 8f76576 to d5e8517 Compare August 26, 2026 19:57
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch from cd00456 to 09810e8 Compare August 26, 2026 19:57
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-remove-defaults branch from d5e8517 to 6a582b0 Compare August 26, 2026 20:01
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch 2 times, most recently from 606a1d9 to ba9446c Compare August 26, 2026 20:12
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-remove-defaults branch from 6a582b0 to b10d002 Compare August 26, 2026 20:16
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch 2 times, most recently from 7496a96 to 8b5194d Compare August 26, 2026 20:23
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-remove-defaults branch 3 times, most recently from cc49247 to 19caa29 Compare August 26, 2026 20:29
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch from 8b5194d to bd5c4e3 Compare August 26, 2026 20:29
@isabellalam12 isabellalam12 changed the title Add button to send emails synchronously. Emails: Add button to send emails synchronously. Aug 26, 2026
@isabellalam12
isabellalam12 changed the base branch from 08-24-emails-remove-defaults to graphite-base/88 September 3, 2026 19:39
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch from bd5c4e3 to 44ce8d9 Compare September 3, 2026 19:39
@isabellalam12
isabellalam12 changed the base branch from graphite-base/88 to main September 3, 2026 19:39
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch 4 times, most recently from d60b7ea to 664f9a7 Compare September 3, 2026 20:04
@isabellalam12
isabellalam12 marked this pull request as ready for review September 3, 2026 20:07
@isabellalam12
isabellalam12 requested a review from a team September 3, 2026 20:07
@isabellalam12
isabellalam12 requested review from spiffyy99 and removed request for Arshadul-Monir September 3, 2026 20:07
@graphite-app

graphite-app Bot commented Sep 3, 2026

Copy link
Copy Markdown

Graphite Automations

"Request reviewers once CI passes" took an action on this PR • (09/03/26)

2 reviewers were added to this PR based on Henry Chen's automation.

Comment thread js/src/features/emails/_components/EmailSender.tsx Outdated
Comment thread src/main/java/org/patinanetwork/patchats/email/EmailService.java Outdated
Comment thread src/main/java/org/patinanetwork/patchats/email/EmailService.java Outdated
@isabellalam12
isabellalam12 force-pushed the 08-24-emails-add-sync-button branch from 664f9a7 to bce8637 Compare September 3, 2026 20:40
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

<Text size="sm">
Please confirm that you want to send {request?.messages.length} email
{request?.messages.length === 1 ? "" : "s"}.
{request?.messages.length === 1 ? "" : "s"} asynchronously.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we should explain this a tad bit more. Sure it's not a super important implementation detail, but "asynchronously" may not make sense to the people reading this.

Maybe something like "queued up for send, emails will be retried if an error occurs". Just something that at least explains it a bit

children: (
<Text size="sm">
Please confirm that you want to send {request?.messages.length} email
{request?.messages.length === 1 ? "" : "s"} synchronously.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar to above, maybe just say "will be sent immediately".

.templateId(template.getId())
.templateValues(variables)
.status(result.sent() ? EmailStatus.SENT : EmailStatus.ERROR)
.errorMessage(result.error())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit - what if there's no error? Will this be handled properly?

Mainly just thinking about if a send succeeds, user won't see an error showing up

MANUAL,
MATCHING
MATCHING,
SYNCHRONOUS,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another nit - i feel like async vs sync should be a separate enum from manual vs matching.

Not because the functionality will break, but it better reflects the state of the system. Async vs sync and manual vs manual are different paradigms, having them together would imply that an email cannot be both async and matching at the same time.

@spiffyy99

Copy link
Copy Markdown

Have a screenshot of what this looks like from the front end?

Copy link
Copy Markdown
Member Author

Have a screenshot of what this looks like from the front end?

Screenshot 2026-09-04 at 1.12.55 PM.png

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants