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

[Apollo Pagination] Improve ReversePagination support, implement loadAll support, Bidirectional Pagination #115

Merged
merged 178 commits into from
Jan 24, 2024

Conversation

Iron-Ham
Copy link
Contributor

@Iron-Ham Iron-Ham commented Oct 31, 2023

What are you trying to accomplish?

  1. I want consumers of the GraphQLPager to be able to load all pages at once, should they desire to do so.
  2. In a quest to deliver bidirectional pagination, we must first solidify support for reverse pagination.

closes apollographql/apollo-ios#3265

What approach did you choose and why?

LoadAll Support:
First, I modified the fetch function to follow async/await syntax. That's so that we can await its return prior to beginning to loop over loadMore.

Then, I implemented the function using a while loop.

Finally, I made additive changes to the MockGraphQLServer to support subsequent loads of this nature.

Reverse Pagination:
Leaning hard into differentiating reverse and forward pagination from one another.
If we are to support both types of pagination at once, then they must be treated as being separate from one another: A page can be a previous page or it can be a next page. It cannot be both.

BIdirectional Pagination
Once reverse pagination was solidifed, this was a breeze to implement. It basically amounted to defining the pagination struct.

Anything you want to highlight for special attention from reviewers?

Note that the output for the pagination controller is now a struct. This allows us to make changes to the output without breaking previous versions. I've removed logic around Task and Continuation management.

If something goes wrong, what are the mitigation strategies?

  • Rollback - This change can only be disabled by reverting this pull request or commit.

Copilot Summary

This pull request primarily introduces improvements to the Apollo GraphQL testing suite and pagination functionality. The most significant changes include the addition of a new BidirectionalPaginationTests class for testing bidirectional pagination, modifications to the MockGraphQLServer class to enhance testing capabilities, and updates to the AnyGraphQLQueryPagerTests and ForwardPaginationTests classes to accommodate new pagination functionalities.

Enhancements to Apollo GraphQL testing:

  • Tests/ApolloInternalTestHelpers/MockGraphQLServer.swift: Added a private subscript to the MockGraphQLServer class to handle request expectations for a given operation type. Also, a new expect method was added to handle request expectations for a specific operation. [1] [2]

Improvements to Apollo GraphQL Pagination:

  • Tests/ApolloPaginationTests/AnyGraphQLQueryPagerTests.swift: Modified the eraseToAnyPager method to include an additional parameter. Also, added a new test_loadAll method to test the loading of all pages. Several other minor changes were made to accommodate updates to the pagination functionality. [1] [2] [3] [4]
  • Tests/ApolloPaginationTests/ForwardPaginationTests.swift: Updated several methods to reflect changes in the pagination functionality. [1] [2] [3] [4] [5]
  • Tests/ApolloPaginationTests/ConcurrencyTest.swift: Updated several methods to reflect changes in the pagination functionality. [1] [2] [3] [4] [5] [6]

Addition of new test class:

  • Tests/ApolloPaginationTests/BidirectionalPaginationTests.swift: Added a new BidirectionalPaginationTests class to test bidirectional pagination functionality. This class includes several methods to test fetching multiple pages, loading all pages, and more.

Copy link
Contributor

@AnthonyMDev AnthonyMDev left a comment

Choose a reason for hiding this comment

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

Couple of minor things, but this is looking good!

Copy link
Contributor

@AnthonyMDev AnthonyMDev left a comment

Choose a reason for hiding this comment

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

Couple of minor things, but this is looking good!

@Iron-Ham Iron-Ham force-pushed the hs/reverse-pagination branch from 29a1dda to 86c4885 Compare January 8, 2024 18:41
@AnthonyMDev AnthonyMDev merged commit 3af5cdf into apollographql:main Jan 24, 2024
14 checks passed
BobaFetters pushed a commit that referenced this pull request Jan 24, 2024
BobaFetters pushed a commit to apollographql/apollo-ios-pagination that referenced this pull request Jan 24, 2024
BobaFetters pushed a commit that referenced this pull request Jan 24, 2024
9554ce08 [Apollo Pagination] Improve ReversePagination support, implement  support, Bidirectional Pagination (#115)

git-subtree-dir: apollo-ios-pagination
git-subtree-split: 9554ce088caf4097deb1ae6e934ef9d15254a7c7
BobaFetters pushed a commit that referenced this pull request Jan 24, 2024
…ReversePagination support, implement support, Bidirectional Pagination

git-subtree-dir: apollo-ios-pagination
git-subtree-mainline: 1e20a27
git-subtree-split: 9554ce088caf4097deb1ae6e934ef9d15254a7c7
@apollographql apollographql deleted a comment from Pice12345 Jan 28, 2024
@apollographql apollographql deleted a comment from Karliz24 Feb 1, 2024
@apollographql apollographql deleted a comment from Karliz24 Feb 1, 2024
@apollographql apollographql deleted a comment Mar 6, 2024
@apollographql apollographql deleted a comment from Pice12345 Mar 6, 2024
@apollographql apollographql deleted a comment from 4304417 Mar 30, 2024
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.

[ApolloPagination] Bi-directional paging support
4 participants