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

Support for create a Firebase Document Reference and offset query #181

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lucasdidur
Copy link

Because in my project I need to use document references and the fireorm still doesn't have support for references, both for searching and for creating new documents, I implemented a function to facilitate the manipulation to get a native data of firebase.

I also implemented support for query offset, very useful in document pagination.

@codecov
Copy link

codecov bot commented Aug 18, 2020

Codecov Report

Merging #181 into master will decrease coverage by 1.67%.
The diff coverage is 37.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #181      +/-   ##
==========================================
- Coverage   94.36%   92.68%   -1.68%     
==========================================
  Files          21       21              
  Lines         532      547      +15     
  Branches       83       86       +3     
==========================================
+ Hits          502      507       +5     
- Misses         30       40      +10     
Impacted Files Coverage Δ
src/types.ts 100.00% <ø> (ø)
src/AbstractFirestoreRepository.ts 86.66% <25.00%> (-2.87%) ⬇️
src/QueryBuilder.ts 91.93% <33.33%> (-6.32%) ⬇️
src/BaseFirestoreRepository.ts 93.44% <50.00%> (-3.05%) ⬇️
.../Transaction/BaseFirestoreTransactionRepository.ts 82.97% <50.00%> (-1.47%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f66fe7e...11fa721. Read the comment docs.

@skalashnyk
Copy link
Contributor

skalashnyk commented Sep 17, 2020

offset is something that is not recommended to use by google - https://cloud.google.com/firestore/docs/best-practices#read_and_write_operations

Do not use offsets. Instead, use cursors. Using an offset only avoids returning the skipped documents to your application, but these documents are still retrieved internally. The skipped documents affect the latency of the query, and your application is billed for the read operations required to retrieve them.

Oh, this is your 2nd attempt #132 :-)

Copy link

@justusburger justusburger left a comment

Choose a reason for hiding this comment

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

This is exactly what I need now - the ability to specify an offset in order to do pagination.

@wovalle
Copy link
Owner

wovalle commented Sep 26, 2024

Hi @lucasdidur!

In order to deploy this I'd still need two things:

  • Fix the branch conflicts
  • Add a unit test for the offset

But nonetheless, great job!

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.

4 participants