New functionality #3
Conversation
(required for resubscribing recipients)
There was a problem hiding this comment.
Hi @nkm, thanks for your contribution! This is a great feature. I'm going to leave some comments over the code you wrote. Once you finished, can you squash all your commits into a single commit please?
Unfortunately, this library lacks of tests. I'm also opening an issue to track it down.
Thanks again and sorry for the delay reviewing this!
| @@ -19,7 +19,7 @@ | |||
| class Context | |||
| { | |||
| const AUTH_TOKEN_URI = 'https://services.mailup.com/Authorization/OAuth/Token'; | |||
| const BASE_URI = 'https://services.mailup.com/API/v1.1/Rest'; | |||
| const BASE_URI = 'https://services.mailup.com/API/v1.1/Rest'; | |||
There was a problem hiding this comment.
please, do not use internal indentation for assignments
| 'Content-Type' => 'application/json', | ||
| 'Authorization' => 'Bearer '.$this->token->getAccessToken(), | ||
| ], | ||
| json_encode($params) |
| $this->username = $options['username']; | ||
| $this->password = $options['password']; | ||
| $this->username = $options['username']; | ||
| $this->password = $options['password']; |
There was a problem hiding this comment.
please, do not use internal indentation for assignments
| $message = "Response not OK when requesting an access token. Response body: $responseBody"; | ||
| $responseBody = (string) $response->getBody(); | ||
| $message = "Response not OK when requesting an access token. " | ||
| . "Response body: {$responseBody}"; |
There was a problem hiding this comment.
do not break a line (event strings) if it stays within the soft limit of PSR-2 (120 chars, http://www.php-fig.org/psr/psr-2/#overview).
Also, keep the operator in the line of the first argument. In this case it should be (but it is not for what mentioned above):
$message = "Response not OK when requesting an access token. " .
"Response body: {$responseBody}"
;| self::AUTH_TOKEN_URI, | ||
| [ | ||
| 'Content-Type' => 'application/x-www-form-urlencoded', | ||
| 'Authorization' => 'Basic '.base64_encode($this->clientId.':'.$this->clientSecret), |
There was a problem hiding this comment.
spaces between . operator and its arguments
| 'MobileNumber' => $this->mobileNumber, | ||
| 'MobilePrefix' => $this->mobilePrefix, | ||
| 'Fields' => $this->fields, | ||
| 'Fields' => $this->fields, |
There was a problem hiding this comment.
internal indentation
| string $refreshToken | ||
| ) { | ||
| $this->accessToken = $accessToken; | ||
| $this->validUntil = $validUntilTimestamp; |
There was a problem hiding this comment.
internal indentation
| if ( | ||
| null === $object | ||
| || ! isset($object->accessToken, $object->validUntil, $object->refreshToken) | ||
| ) { |
There was a problem hiding this comment.
if statement and binary operators
| if ( | ||
| null === $object | ||
| || ! isset($object->access_token, $object->expires_in, $object->refresh_token) | ||
| ) { |
There was a problem hiding this comment.
if and internal indentation
| 'accessToken' => $this->accessToken, | ||
| 'validUntil' => $this->validUntil, | ||
| 'accessToken' => $this->accessToken, | ||
| 'validUntil' => $this->validUntil, |
There was a problem hiding this comment.
internal indentation
|
@nkm any news about this? |
Hi again,
I've added new functionality to the library:
I've updated the README accordingly.
Finally I've also made some stylistic code changes, mainly to better comply with PSR-2.
Edit: Forgot to add that all changes are Backwards Compatible :)