Skip to content

[Mime] Remote email feature #50421

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

mmouih
Copy link

@mmouih mmouih commented May 25, 2023

Q A
Branch? 6.4
Bug fix? NO
New feature? YES
Deprecations? NO
Tickets -
License MIT
Doc PR TODO

Hello,

To send emails via sendingblue, I used Mime Email class with sendingBlue bridge,
I noticed an Exception while trying to send the email to a remote template hosted on sending blue;
Sendingblue/brevo expects to receive a templateId header to select the template to be used for this email, that is supposed to be enough;
I encountered a LogicException because an Email should always have body or an attachment,
The source of this exception comes from Email::ensureBodyValid() private method.

A simple solution for this issue, is to define an arbitrary body (I usually send the templateId as body text), so no big deal.
I do not consider this to be a bug, not a serious one at least.

That gave me the idea to make a pull request, to define a special type for Remote email, with I called RemoteEmail in the Mime component for emails hosted on the cloud, which allows the email to have an empty body, instead it will define two things, a templateId, the header name to be sent the email api,

I had to change the visibility of the class Email::ensureBodyValid() to protected instead of private, to be able to customize it's behaviour.

I included tests for RemoteEmail class in the Mime component, and Tests to integrate it with SendingBlueBridge, but I did not integration bridge tests to avoid unit tests issues in the Mailer component since the RemoteEmail class would not be recognised,

Sending Blue Test file in another branch:
https://github.com/mmouih/symfony/blob/be08c7ca6bdf44ac148f09761cdd0b9e8c755190/src/Symfony/Component/Mailer/Bridge/Sendinblue/Tests/Transport/SendinblueApiTransportRemoteEmailTest.php

Example of a failure test:

public function testInvalidEmail(): void
{
$email = new RemoteEmail();
$email->subject('Remote Email Subject !')
->to(new Address('mounir.mouih@gmail.com', 'Mounir Mouih'))
->from(new Address('fabpot@symfony.com', 'Fabien'))
->addCc('foo@bar.fr')
->addBcc('foo@bar.fr')
->addReplyTo('foo@bar.fr');
$this->expectException(LogicException::class);
$email->ensureValidity();
}

A success example below:

public function testvalidEmail(): void
{
$email = new RemoteEmail();
$email->subject('Remote Email Subject !')
->to(new Address('mounir.mouih@gmail.com', 'Mounir Mouih'))
->from(new Address('fabpot@symfony.com', 'Fabien'))
->addCc('foo@bar.fr')
->addBcc('foo@bar.fr')
->addReplyTo('foo@bar.fr');
$email->setRemoteTemplate('templateId', '1');
$email->ensureValidity();
$this->assertTrue(true);
}

This RemoteEmail class is a subtitute to Email on most usages, however the body is always Empty.

@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@mmouih mmouih force-pushed the feature_remote_email branch 2 times, most recently from 6878ef7 to 3749a12 Compare May 25, 2023 00:27
@mmouih mmouih changed the title New Feature: remote email [Mailer]New Feature: remote email May 25, 2023
@mmouih mmouih force-pushed the feature_remote_email branch 2 times, most recently from a33ca0e to c224194 Compare May 25, 2023 00:41
@mmouih mmouih changed the title [Mailer]New Feature: remote email [Mime]New Feature: remote email May 25, 2023
@mmouih mmouih changed the title [Mime]New Feature: remote email [Mime] Remote email feature May 25, 2023
@fabpot
Copy link
Member

fabpot commented Aug 4, 2023

We don't support email templates from mailer providers (as we wouldn't be able to abstract that for all providers).
Let's not add this feature as there is apparently a workaround that works great for you.
Thank you for the pull request and sorry it cannot make it.

@fabpot fabpot closed this Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants