-
Notifications
You must be signed in to change notification settings - Fork 2k
Implementing new Birthday class to handle Graph API response variations #573
Conversation
docs/Birthday.fbmd
Outdated
| ~~~~ | ||
| $fb = new Facebook\Facebook(\* *\); | ||
| // Returns a `Facebook\FacebookResponse` object | ||
| $response = $fb->get('/something'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it's about a user, I'd get /me :)
src/Facebook/GraphNodes/Birthday.php
Outdated
| { | ||
| $parts = explode('/', $date); | ||
|
|
||
| if (count($parts) === 3 || count($parts) === 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ifs are useless, you can just $this->hasYear = count($parts) === 3 || count($parts) === 1, same below with hasDate.
|
@nickma42 Thanks for your contribution! |
|
Thanks for your help in the ticket too! You guys are very helpful and responsive :) |
docs/sdk_reference.fbmd
Outdated
| 'A collection that represents a User node.', | ||
| ], | ||
| [ | ||
| '[`Facebook\\GraphNodes\\Birthday`](/docs/php/Birthday#instance-methods)', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it should be listed here, ping @SammyK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I think I'd modify the docs on GraphNode and edit the lines that talk about the return type and link to the Birthday documentation from there. :)
|
Looks good to me, 👍 |
src/Facebook/GraphNodes/Birthday.php
Outdated
| /** | ||
| * @var bool | ||
| */ | ||
| protected $hasDate = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be private. :)
…ore appropriate location
|
Thanks guys - much appreciated :) |
Fix for #570 since the Graph API can return the birthday field as either mm/dd/yyyy, mm/dd or yyyy.