added recently-played endpoint #67
Conversation
| } | ||
|
|
||
| if (isset($options['before'])) { | ||
| $options['after'] = ''; |
There was a problem hiding this comment.
change to that instead:
$options['after'] = NULL;
| } | ||
|
|
||
| if (isset($options['after'])) { | ||
| $options['before'] = ''; |
There was a problem hiding this comment.
change to that instead:
$options['before'] = NULL;
| $options = (array) $options; | ||
|
|
||
| if (isset($options['limit'])) { | ||
| $options['limit'] = checkLimit($options['limit'], 1, 50) ? $options['limit'] : 20; |
There was a problem hiding this comment.
I think we can leave these checks to Spotify. We try to avoid having defaults in our code (in case Spotify decides to change it). So just passing $options straight through should be fine.
| * | ||
| * @param array|object $options Optional. Options to get tracks history. | ||
| * - int limit Optional. Maximum number of items to return. Default: 20. Minimum: 1. Maximum: 50 | ||
| * - float after Optional. Unix timestamp in ms. Returns all items after (but not including) this cursor position. If after is specified, before must not be specified |
There was a problem hiding this comment.
These lines are a bit too long (failing the linting), I think we can remove "If after is specified, before must not be specified" and "If before is specified, after must not be specified" since that's stated in Spotify docs and enforced there.
| * - float after Optional. Unix timestamp in ms. Returns all items after (but not including) this cursor position. If after is specified, before must not be specified | ||
| * - float before Optional. Unix timestamp in ms. Returns all items before (but not including) this cursor position. If before is specified, after must not be specified | ||
| * | ||
| * @return array|object The most recent 50 tracks played (more than 30seconds) by a user. Type is controlled by `SpotifyWebAPI::setReturnType()`. |
There was a problem hiding this comment.
Same as above, "(more than 30seconds)" can be removed. And something else too probably, not sure what though.
|
|
||
| $headers = $this->authHeaders(); | ||
|
|
||
| $uri = '/v1/me/player/recently-played' ; |
There was a problem hiding this comment.
Could you remove the space before ;?
|
This mostly looks good, thanks for the PR! A test is needed, though. If you need some inspiration, |
|
Thanks for the comments, will apply the changes and provide a test. Just need to know how to make changes/edits to the PR. It says that I need "push access" to the patch-1 branch... (first time I do that on github :) ) |
|
It looks like you deleted your fork of this repository. I'm not sure if it's possible to continue working in this PR. Perhaps if you fork this repository again and create a branch named If it's impossible to continue working on this PR, just submit a new one with the changes and a test in the same one :) |
|
@hycday Sorry, I forgot. You can run the tests with |
|
forked it again, added the changes in patch-1 and patch-1-1 for the test. let me know |
|
It looks like you need to open a new PR, so I'm gonna close this one. But merge those two branches first so we get the test and the new method at the same time. Thanks! |
|
I opened a new PR (finally kinda understood how that works :)) #68 |
not sure if the test on after vs before is correct (if one is set, the other must not be specified)