NotificationsYou must be signed in to change notification settings
Fork9.6k
Star30.4k

[Mailer][SendGrid] add support for scheduling delivery via`send_at` API parameter#60372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

MrMitch wants to merge1 commit intosymfony:7.4

base:7.4

Choose a base branch

fromMrMitch:sendgrid-mailer-date-send-at

Open

[Mailer][SendGrid] add support for scheduling delivery via`send_at` API parameter#60372

MrMitch wants to merge1 commit intosymfony:7.4fromMrMitch:sendgrid-mailer-date-send-at

Conversation

Copy link

MrMitch commentedMay 7, 2025•
edited
Loading

Q	A
Branch?	7.3
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	N/A
License	MIT

This PR adds support for scheduling delivery when using the SendGridAPI transport (with asendgrid+api DSN), by providing a\DateTimeInterface object in aSymfony\Component\Mime\Header\DateHeader namedSend-At.

$email =new \Symfony\Component\Mime\Email();$email->getHeaders()->addDateHeader('Send-At', (new \DateTime())->modify('+3 hours'));

It will be mapped to thesend_at parameter of the[POST] /mail/sendAPI endpoint.

carsonbot added Status: Needs Review Feature Mailer labels

May 7, 2025

carsonbot added this to the6.4 milestone

May 7, 2025

carsonbot changed the title~~[Mailer][SendGrid] add support for scheduling delivery via send_at API parameter~~[Mailer] [SendGrid] add support for scheduling delivery via send_at API parameter

May 7, 2025

Copy link

Author

MrMitch commentedMay 7, 2025

Test failures look unrelated

OskarStark modified the milestones:6.4,7.3

May 7, 2025

Copy link

Contributor

OskarStark commentedMay 7, 2025

New features need to target the newest branch, which is 7.3

OskarStark reviewed

May 7, 2025

View reviewed changes

src/Symfony/Component/Mailer/Bridge/Sendgrid/CHANGELOG.md OutdatedShow resolvedHide resolved

src/Symfony/Component/Mailer/Bridge/Sendgrid/README.md OutdatedShow resolvedHide resolved

src/Symfony/Component/Mailer/Bridge/Sendgrid/Tests/Transport/SendgridApiTransportTest.phpShow resolvedHide resolved

OskarStark changed the title~~[Mailer] [SendGrid] add support for scheduling delivery via send_at API parameter~~[Mailer][SendGrid] add support for scheduling delivery via send_at API parameter

May 7, 2025

bkosun suggested changes

May 8, 2025

View reviewed changes

src/Symfony/Component/Mailer/Bridge/Sendgrid/Transport/SendgridApiTransport.php OutdatedShow resolvedHide resolved

src/Symfony/Component/Mailer/Bridge/Sendgrid/Transport/SendgridApiTransport.php

		}
		// the documentation shows parameter names in camelCase but the expected JSON keys should be in snake_case
		$payload['send_at'] = $header->getDateTime()->getTimestamp();
		} elseif ($header instanceof TagHeader) {

Copy link

bkosunMay 8, 2025

Choose a reason for hiding this comment

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

How about using guard clauses, instead of nested conditions, to improve readability?

https://github.com/symfony/symfony/blob/7.3/src/Symfony/Component/Mailer/Bridge/Postmark/Transport/PostmarkApiTransport.php#L114-L145

Copy link

Author

MrMitchMay 8, 2025

Choose a reason for hiding this comment

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

Hi,thank you for your suggestion.
I didn't want to change too much of the code to facilitate the review process but I'll gladly update the successive conditions if necessary. Is it something you would like me to do in this PR ?

Copy link

bkosunMay 15, 2025

Choose a reason for hiding this comment

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

It's just a suggestion, but since you've added new conditions, this approach might make sense, as it's used in other integrations. I think it's worth waiting to hear and consider other opinions before making a final decision.

carsonbot added Status: Needs Work and removed Status: Needs Review labels

May 8, 2025

carsonbot changed the title~~[Mailer][SendGrid] add support for scheduling delivery via send_at API parameter~~[Mailer] [SendGrid] add support for scheduling delivery via send_at API parameter

May 8, 2025

[Mailer][SendGrid] add support for scheduling delivery via send_at AP…

a317491

…I parameter

MrMitch force-pushed thesendgrid-mailer-date-send-at branch from50ac018 toa317491Compare

May 8, 2025 10:39

MrMitch requested review fromlyrixx,yceruto,chalasr,dunglas,jderusse andxabbuh ascode owners

May 8, 2025 10:39

carsonbot added Status: Needs Review and removed Status: Needs Work labels

May 8, 2025

MrMitch changed the base branch from6.4 to7.3

May 8, 2025 10:39

Copy link

Author

MrMitch commentedMay 8, 2025

@OskarStark @bkosun Thank you for your feedback, I've applied all the suggested changes. Let me know if you would like me to make additional changes.

Copy link

Member

stof commentedMay 16, 2025

Is this Send-At header supported by their SMTP server ?

Copy link

Author

MrMitch commentedMay 16, 2025

Hi@stof,

Yes, the Send-At header is supported by their SMTP server as well, as perthe documentation :

You can use theSMTP API header to schedule your emails when sending viaSMTP or ourmail.send API endpoint.

OskarStark changed the title~~[Mailer] [SendGrid] add support for scheduling delivery via send_at API parameter~~[Mailer][SendGrid] add support for scheduling delivery viasend_at API parameter

May 16, 2025