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

Move API pagination into a common partial #2181

Merged
merged 2 commits into from
Aug 29, 2017

Conversation

jhawthorn
Copy link
Contributor

@jhawthorn jhawthorn commented Aug 23, 2017

Previously, pagination was handled individually by each index partial, which led to some inconsistencies.

This PR adds a spree/api/shared/pagination.json.jbuilder partial which is passed a paginated collection and renders the pagination attributes.

This doesn't change any existing attributes, but some endpoints will now have per_page and total_count, which were missing previously.

@jhawthorn jhawthorn added the changelog:solidus_api Changes to the solidus_api gem label Aug 23, 2017
@solidusio solidusio deleted a comment from houndci-bot Aug 23, 2017
Copy link
Member

@tvdeyen tvdeyen 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.

Thinking of adding a change log entry just in case someone has overwritten these views exactly for that use case (total_count vs. count) and would not need this anymore.

Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

Nice work. We will already have a changelog entry for all the views being converted from RABL to JBuilder, so I think that should cover it.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍

@cbrunsdon cbrunsdon merged commit 900feda into solidusio:master Aug 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_api Changes to the solidus_api gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants