Add ability to impersonate subuser #571
Conversation
thinkingserious
added
the
status: code review request
|
|
|
Not sure why dockerfile existence test breaks? Has it been removed in the repo? |
|
No worries on the failing test, we will fix that on our end. Thanks for taking the time to submit this PR! It's currently on our backlog for review. |
thinkingserious
added
the
status: code review request
thinkingserious
added
difficulty: medium
lib/SendGrid.php
Outdated
| */ | ||
| public function __construct($apiKey, $options = array()) | ||
| public function __construct($apiKey, $options = array(), $impersonateSubuser = null) |
There was a problem hiding this comment.
Please, remove surplus blankspace before $options.
lib/SendGrid.php
Outdated
| @@ -37,15 +37,20 @@ class SendGrid | |||
| * | |||
| * @param string $apiKey your SendGrid API Key. | |||
| * @param array $options an array of options, currently only "host" and "curl" are implemented. | |||
| * @param string $impersonateSubuser Support impersonation of subusers. | |||
There was a problem hiding this comment.
Please, update docblock according the accepted values (string|null).
lib/SendGrid.php
Outdated
| { | ||
| $headers = array( | ||
| 'Authorization: Bearer '.$apiKey, | ||
| 'User-Agent: sendgrid/' . $this->version . ';php', | ||
| 'Accept: application/json' | ||
| ); | ||
|
|
||
| if (!empty($impersonateSubuser)) { |
There was a problem hiding this comment.
Please, use null !== $impersonateSubuser instead.
test/unit/GenericTest.php
Outdated
| $subuser = 'abcxyz@this.is.a.test.subuser'; | ||
| $headers[] = 'On-Behalf-Of: ' . $subuser; | ||
| $sg5 = new SendGrid(self::$apiKey, [], $subuser); | ||
| $this->assertEquals($sg5->client->getHeaders(), $headers); |
There was a problem hiding this comment.
Regardless assert*() methods are already misused in the existing codebase, please, pass the expectation in argument 1, then the actual value in argument 2. [ref].
Use assertSame() if possible.
|
@phansys For some reason another test started to fail because the test runner installs version 3.9.x of EDIT: Ah there is already a bug report for this in #586 . Then this patch should be complete when that issue is resolved. |
|
That is a valid bug, the function signature should be: https://github.com/sendgrid/php-http-client/blob/929018c62b7fcd99b3b356216ae75fff4d47b5a1/lib/Client.php#L48 I'll be fixing that shortly in a v3.9.4 patch. Thanks for the heads up! With Best Regards, Elmer |
lib/SendGrid.php
Outdated
| */ | ||
| public function __construct($apiKey, $options = array()) | ||
| public function __construct($apiKey, $options = array(), $impersonateSubuser = null) |
There was a problem hiding this comment.
How about including this as part of the $options array?
thinkingserious
added
status: waiting for feedback
Sendgrid API has a feature allowing parent accounts to impersonate subusers by including an HTTP header "On-Behalf-Of" in each API request. This commit enables setting the value for impersonation when creating the Sendgrid API Client instance. Fixes #551
|
Just rebased this PR on master branch now. Would be nice to get it in soon =) |
thinkingserious
added
status: code review request
|
Awesome, thanks @stianpr! |
|
@thinkingserious Sweet. Thank you! =) |
Sendgrid API has a feature allowing parent accounts to impersonate
subusers by including an HTTP header "On-Behalf-Of" in each API request.
This commit enables setting the value for impersonation when creating
the Sendgrid API Client instance, through the
$optionsargument.Fixes #551