Skip to content
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

Create personalization in constructor #568

Closed
izhukovich opened this issue Dec 18, 2017 · 5 comments
Closed

Create personalization in constructor #568

izhukovich opened this issue Dec 18, 2017 · 5 comments
Labels
status: help wanted requesting help from the community type: question question directed at the library

Comments

@izhukovich
Copy link

izhukovich commented Dec 18, 2017

I want to send email with personalization and substitution tags, but an empty personalization is created in Mail constructor:

public function __construct($from, $subject, $to, $content)
    {
        $this->setFrom($from);
	    $this->setSubject($subject);
	    $personalization = new Personalization();
        $personalization->addTo($to);
        $this->addPersonalization($personalization);
        $this->addContent($content);
    }

Then I create one more with substitution tags and add it to mail. Finally I have 2 personalizations and empty emails are sent from time to time.

Now I iterate all existing personalizations and add tags to them, but it is not good solution.
Can you make parameters in constructor optional?

public function __construct($from = null, $subject = null, $to = null, $content = null)
    {
        if ($from) {
            $this->setFrom($from);
        }
        if ($subject) {
            $this->setSubject($subject);
        }
	    if ($to) {
            $personalization = new Personalization();
            $personalization->addTo($to);
            $this->addPersonalization($personalization);
        }
        if ($content) {
            $this->addContent($content);
        }
    }

Or add an optional substitutionTags param:

public function __construct($from, $subject, $to, $content, $substitutionTags = null)
    {
        $this->setFrom($from);
	    $this->setSubject($subject);
	    $personalization = new Personalization();
        $personalization->addTo($to);
        
        if ($substitutionTags) {
            foreach ($substitutionTags as $variable => $value) {
                $personalization->addSubstitution($variable, $value);
            }
        }

        $this->addPersonalization($personalization);
        $this->addContent($content);
    }
@thinkingserious thinkingserious added status: help wanted requesting help from the community type: question question directed at the library labels Dec 19, 2017
@thinkingserious
Copy link
Contributor

Hello @izhukovich,

You can add to the initial personalization we create for you in the constructor through this object: $mail->personalization[0].

You may be interested in providing your thoughts on the redesign, which is underway now:
#434 Your feedback would be very much appreciated!

With Best Regards,

Elmer

@TzipporahWitty
Copy link

TzipporahWitty commented Jan 9, 2018

+1 for making the parameters optional.

  • when using a transactional template both the subject and the content are set by setting the template ID, requiring them in the constructor is confusing and illogical.

  • when sending to many recipients it doesn't follow normal processing logic to pull out one of them to use for the constructor and then add the others afterwards. Also makes things messier when there's significant processing involved with adding in custom substitutions since you can't just iterate through the list any more, you have to treat the first one differently.

Perhaps instead of/in addition to this constructor there could be other methods for creating an instance of the object that do not require these parameters.

(hope this isn't too repetitive - I'm going through the discussion you linked to above but it's pretty long.)

As an aside, my company's use case involves sending tens of thousands of emails a day, some as bulk mailings and some as individual mailings. Each of those is sent with a transactional template so our marketing department can manage them. For each recipient we are sending about 20 substitution tags. So ability to easily send to groups and add substitution content without doing weird things in the code is a use case I very much identify with.

Thanks for all your time and help!

@thinkingserious
Copy link
Contributor

Hi @Tzafra!

This is great feedback and I've added a note to the thread over at #434. Thank you!

@senorgeno
Copy link

@thinkingserious completely agree with @Tzafra. I am trying to send multiple personalisations and it doesn't make any sense to have to set the first user via the mail constructor. It is also really awkward to then have to edit that personalisation for substitutions. I will look to revert to 5.6 for the mean time.

@Bijoy-George
Copy link

@iskov==

izhukovich

I want to send email with personalization and substitution tags, but an empty personalization is created in Mail constructor:

public function __construct($from, $subject, $to, $content)
    {
        $this->setFrom($from);
	    $this->setSubject($subject);
	    $personalization = new Personalization();
        $personalization->addTo($to);
        $this->addPersonalization($personalization);
        $this->addContent($content);
    }

Then I create one more with substitution tags and add it to mail. Finally I have 2 personalizations and empty emails are sent from time to time.

Now I iterate all existing personalizations and add tags to them, but it is not good solution. Can you make parameters in constructor optional?

public function __construct($from = null, $subject = null, $to = null, $content = null)
    {
        if ($from) {
            $this->setFrom($from);
        }
        if ($subject) {
            $this->setSubject($subject);
        }
	    if ($to) {
            $personalization = new Personalization();
            $personalization->addTo($to);
            $this->addPersonalization($personalization);
        }
        if ($content) {
            $this->addContent($content);
        }
    }

Or add an optional substitutionTags param:

public function __construct($from, $subject, $to, $content, $substitutionTags = null)
    {
        $this->setFrom($from);
	    $this->setSubject($subject);
	    $personalization = new Personalization();
        $personalization->addTo($to);
        
        if ($substitutionTags) {
            foreach ($substitutionTags as $variable => $value) {
                $personalization->addSubstitution($variable, $value);
            }
        }

        $this->addPersonalization($personalization);
        $this->addContent($content);
    }

Thanks it working

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: question question directed at the library
Projects
None yet
Development

No branches or pull requests

5 participants