Skip to content
This repository was archived by the owner on Oct 10, 2022. It is now read-only.

Add micro-api-client pagination #44

Closed
wants to merge 2 commits into from
Closed

Add micro-api-client pagination #44

wants to merge 2 commits into from

Conversation

bcomnes
Copy link
Contributor

@bcomnes bcomnes commented Apr 11, 2019

- Summary

This still requires the pagination parameters to be added to open-api.

- Test plan

We should have a test server and make sure we get the right shape back.

- Description for the changelog

Adds pagination data to paginated responses.

data.pagination = { next: 2, last: 2, current: 1, total: null }

@bcomnes bcomnes added the wip label Apr 11, 2019
@bcomnes
Copy link
Contributor Author

bcomnes commented Apr 11, 2019

WIP till I there are some tests on this. Also, this is going to be a really bad breaking change.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 66.462% when pulling 13c116f on pagination into 64c68b5 on master.

@coveralls
Copy link

coveralls commented Apr 11, 2019

Coverage Status

Coverage increased (+0.07%) to 66.531% when pulling 9a0f057 on pagination into 6070f83 on master.

@bcomnes
Copy link
Contributor Author

bcomnes commented Apr 11, 2019

Maybe we can do this without a breaking change, adding pagination as an array property? Kind of weird but would ease the pain.

This still requires the pagination parameters to be added to open-api.  This is also a breaking change, unfortunately.
@bcomnes
Copy link
Contributor Author

bcomnes commented Apr 11, 2019

Ok, I modified this to not be a breaking change. What do you think @DavidWells ?

Also, the total value is blank because bitbaloon does not return X-Total-Count. Maybe we should have the pagination calculate this?

I'm a little less satisfied with the shape, (adding a property to an array instead of returning an object), but it lets us add this without a breaking change, and it shares the same algorithm as micro-api-client.

@swyxio
Copy link
Contributor

swyxio commented Apr 13, 2019

seems unobjectionable. but i have v little contextual knowledge to be of use here.

@bcomnes
Copy link
Contributor Author

bcomnes commented Apr 14, 2019

I spoke with @DavidWells last week over DM. Sounds like we should have a chat this week about this. He suggested we aim for the ideal API shape. I agree, though I worry we'll never get around to it due to the pain / refactoring involved. If we take an iterative approach, (e.g. the array property), we could at least gain the benefit of this without jumping that hurdle now. But it would also increase the probability of never refactoring to get to the ideal shape.

@swyxio
Copy link
Contributor

swyxio commented Jun 6, 2019

we never got around to this :(

@bcomnes
Copy link
Contributor Author

bcomnes commented Jun 6, 2019

FWIW, this is a working solution.

@erezrokah erezrokah added type: chore work needed to keep the product and development running smoothly type: bug code to address defects in shipped code and removed type: chore work needed to keep the product and development running smoothly labels Sep 7, 2020
@JGAntunes
Copy link
Contributor

Addressed via #283, further discussion in #285. Thanks for the work you've put into this 🙌

@JGAntunes JGAntunes closed this Jan 22, 2021
@JGAntunes JGAntunes deleted the pagination branch January 22, 2021 17:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug code to address defects in shipped code wip
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants