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

Paginate all the things #2896

Closed
sebgie opened this issue Jun 6, 2014 · 0 comments
Closed

Paginate all the things #2896

sebgie opened this issue Jun 6, 2014 · 0 comments
Labels
affects:api Affects the Ghost API
Milestone

Comments

@sebgie
Copy link
Contributor

sebgie commented Jun 6, 2014

In our current implementation Ghost returns inconsistent results for GET requests that return more than one object (e.g. /ghost/api/v0.1/tags/). users, tags, settings and soon apps return all elements from the database. posts returns a paginated result with meta information. This behavior is inconsistent and should be changed to return a paginated result for all API calls that return multiple objects.

The post model has findAll() and findPage() methods atm. findAll() returns all existing post objects and is only used for deleting all content. We would like to keep the use case but remove findAll() by allowing all to be passed as limit parameter.

Default pagination options:

  • page: 1
  • limit: 15 (allows all)

I think it is worth investigating if it is possible to implement a generic findPage() method in base.js and override it for models with special requirements.

@sebgie sebgie added the api label Jun 6, 2014
@ErisDS ErisDS added this to the 0.5 Multi-user milestone Jun 17, 2014
@ErisDS ErisDS mentioned this issue Jul 1, 2014
26 tasks
@ErisDS ErisDS modified the milestones: 0.5.x Feature Release, 0.5 Multi-user Jul 8, 2014
ErisDS added a commit to ErisDS/Ghost that referenced this issue Jun 15, 2015
refs TryGhost#2896

- moves repeated code out of models
- creates a new file for unit-testable code (this should be moved in future)
- adds a default for `page` as that seems sensible
- adds 100% test coverage for the new file
ErisDS added a commit to ErisDS/Ghost that referenced this issue Jun 16, 2015
refs TryGhost#2896

- remove duplicate query-building code
- use the same approach for creating the count query from the main query
- restructure the code to match more closely across the 3 findPage functions (prep for further refactoring)
@ErisDS ErisDS closed this as completed in 7761873 Jun 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:api Affects the Ghost API
Projects
None yet
Development

No branches or pull requests

2 participants