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

#7 - Added option to allow searching with offset and set limit of items re… #8

Merged
merged 3 commits into from
Jan 12, 2021

Conversation

KrishnaSolo
Copy link
Owner

…turned. Also refactored to create searchService

Why

As discussed in issue #7 this pr will allow the user the api to search with an offset. Since spotify by default only returns back only 20 items at a time, this pr will fix the issue of getting results past the 20 returned. In addition this pr has added limit option - up to 50 items can be returned at a time! Also refactored out the searchTrack function as it was doing too many things. We now follow how authentication works.

What

Refactored searchTrack to be like authenticate - creating a new searchService.
Added limit and offset option for search feature.

…turned. Also refactored to create searchService
@KrishnaSolo KrishnaSolo linked an issue Jan 11, 2021 that may be closed by this pull request
@KrishnaSolo KrishnaSolo requested a review from SuhaybY January 11, 2021 01:23
-required to allow reviewer to test branch locally-
Copy link
Collaborator

@SuhaybY SuhaybY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Only suggestion I have is to perhaps have the default arguments (offset, limit) be as an options object so that a user can specify which of the arguments he's setting. A user might want to just set the limit for instance.

@SuhaybY SuhaybY merged commit b94ca3b into main Jan 12, 2021
@SuhaybY SuhaybY deleted the #7-fix-pagniate-bug-when-searching branch January 12, 2021 00:24
@KrishnaSolo KrishnaSolo linked an issue May 18, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix index.d.ts Wrapper does not paginate
2 participants