sendgrid / sendgrid-php

The Official Twilio SendGrid PHP API Library
https://sendgrid.com
MIT License
1.49k stars 624 forks source link

Personalization missing functions #1080

Open PELock opened 2 years ago

PELock commented 2 years ago

1st bug

Adding header to the message before adding personalization, creates an empty personalization object within the email object, and when you add following personalization with recipients emails, your interface throws error because the first personalization initialized by adding header doesn't contain TO field! Who wrote this code??? Seriously, this is bad design decision...

2nd I dunno missing feature

$personalization->addHeader("key", "value");

doesn't work like we do it for the single email, we need to do:

$personalization->addHeader(\SendGrid\Mail\Header("key", "value"));

just add wrapper to handle it like it's done for the $email->addHeader(key, value)

childish-sambino commented 2 years ago

For the second issue, it has been added to our internal backlog to be prioritized. Pull requests and +1s on the issue summary will help it move up the backlog.

For the first issue, could you provide a code sample for what the issue is that you're seeing? That will help to understand what's going on.

PELock commented 2 years ago

If you want to send a newsletter, for example, you want to have the same headers for all the emails:


$email = new \SendGrid\Mail\Mail();
$email->addHeader('List-Unsubscribe', "<$link>");; // <-- this created an empty Personalization object
...

foreach($subscribers as $subscriber)
{
            $personalization = new Personalization();
            $personalization->addTo(new To($subscriber->email, ""));
,,,
}

// throws an error because first $email->addHeader() creates an empty
// Personalization object without defined "to" field
$response = $sendgrid->send($email);

Take a look at addHeader from sendgrid/sendgrid/lib/mail/Mail.php


    public function addHeader(
        $key,
        $value = null,
        $personalizationIndex = null,
        $personalization = null
    ) {
        $header = null;
        if ($key instanceof Header) {
            $h = $key;
            $header = new Header($h->getKey(), $h->getValue());
        } else {
            $header = new Header($key, $value);
        }

        $personalization = $this->getPersonalization($personalizationIndex, $personalization);
        $personalization->addHeader($header);
    }

You see? It creates an empty Personalization object WITHOUT 'to' field declared, so when you want to send the email blast it fails, because the first Personalization for the sender is this empty header personalization.

Solution?

I have to add the same headers to every Personalization object and cannot add the universal header for the $email = new \SendGrid\Mail\Mail() instance.

Unproductive and bad design. Hard to spot and debug.

Even your own documentation is wrong at this:

https://docs.sendgrid.com/for-developers/sending-email/personalizations

`Some properties can be defined both at the root level of the request body (message level) and at the personalizations level.

For example, the from, subject, headers, custom_arg, and send_at properties can all be defined at the message level or at the personalizations level. `

childish-sambino commented 2 years ago

Ah, I see. Definitely an issue. Will add that to the backlog as well. PRs are welcome.