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

feat: compute cursor for each result #277

Closed
wants to merge 8 commits into from

Conversation

ayrtonvwf
Copy link

@ayrtonvwf ayrtonvwf commented Dec 8, 2020

Changes Made

Potential Risks

Not fully understanding all the uses of the paginatedField, I just isolated and exctracted the cursor creation logic from existing code I found on this library. If something I didn't predict is passed in the paginatedField value, the cursor validity would be compromised. The good thing is that the new centralized approach handles the cursor logic isolatedly so this event would be easier to spot and fix.

Test Plan

  • Existing code using this library shouldn't see any changes.
  • By passing the new includeCursor parameter, the response should include a _cursor property for each document, containing the document cursor.
  • The first document cursor should be equal to the previous result property.
  • The last document cursor should be equal to the next result property.
  • Being the result empty, the previous and next results should be null.

Checklist

  • I've increased test coverage
  • Since this is a public repository, I've checked I'm not publishing private data in the code, commit comments, or this PR.

@bradvogel
Copy link
Contributor

@ayrtonvwf are you still working on this?

@ayrtonvwf
Copy link
Author

ayrtonvwf commented Apr 13, 2021

@bradvogel

It's implemented but as I was not able to run the automated tests in my computer (some error with the test tooling I couldn't figure out) I'm not able to finish this PR, but I guess it's only a matter of updating the tests and making them pass.

@christiansany
Copy link

Helloooo :)
When will this be merged? Any plans/timeline?

@bradvogel
Copy link
Contributor

Someone just needs to finish it and write tests.

@bradvogel
Copy link
Contributor

Closing due to age. Let me know if you'd like to reopen

@bradvogel bradvogel closed this Dec 1, 2021
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.

[Feature Proposal] Compute cursor for each document
3 participants