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

SendGrid\Mail\Mail::setGlobalSubject() behaves unexpectedly #669

Closed
smsalisbury opened this issue Aug 16, 2018 · 2 comments · Fixed by #670
Closed

SendGrid\Mail\Mail::setGlobalSubject() behaves unexpectedly #669

smsalisbury opened this issue Aug 16, 2018 · 2 comments · Fixed by #670
Labels
difficulty: easy fix is easy in difficulty status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library

Comments

@smsalisbury
Copy link
Contributor

Issue Summary

The method SendGrid\Mail\Mail::setGlobalSubject($subject) will always set the class member Mail::$subject to whatever the method parameter $subject is. From the documentation on the class member and on getGlobalSubject() and the way the method is written, I'm led to believe that Mail::$subject should always be an instance of SendGrid\Mail\Subject.

Steps to Reproduce

$mail = new \SendGrid\Mail\Mail;
$mail->setGlobalSubject('This is my subject');
var_dump($mail->getGlobalSubject); // should be an instance of SendGrid\Mail\Subject, but is a string

Technical details:

  • sendgrid-php Version: from what I can tell, this affects versions 7.1.0, 7.1.1, and 7.2.0
  • PHP Version: 5.6, 7.0, 7.1, 7.2 (as of May 17, 2018)
@smsalisbury
Copy link
Contributor Author

The culprit is the setGlobalSubject() method itself:

public function setGlobalSubject($subject)
{
    if (!($subject instanceof Subject)) {
        $this->subject = new Subject($subject);
    }
    $this->subject = $subject;
}

Because of the last line (and because $subject is not modified in the function), the first three lines are meaningless.

I will submit a pull request with a proposed change.

@thinkingserious thinkingserious added type: bug bug in the library status: work in progress Twilio or the community is in the process of implementing difficulty: easy fix is easy in difficulty labels Aug 21, 2018
@thinkingserious
Copy link
Contributor

Thanks @smsalisbury! This is on our backlog for a merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy fix is easy in difficulty status: work in progress Twilio or the community is in the process of implementing type: bug bug in the library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants