Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

Skylled
Copy link

@Skylled Skylled commented Jan 16, 2018

Fixes flutter/flutter#13613

Added features:

  • Query can now have more than one orderBy field.
  • startAt, startAfter, endAt, and endBefore support
  • limit support

cc @collinjackson @kroikie

@Skylled
Copy link
Author

Skylled commented Jan 16, 2018

I've now successfully tested startAt/After endAt/Before, limit and multiple orderBys on Android.

@docet85
Copy link

docet85 commented Jan 17, 2018

I was working more or less on the same improvements in parallel - I also drafted the Document versions of startAt/After and endAt/Before. Any chance to include them too?

@Skylled
Copy link
Author

Skylled commented Jan 17, 2018

@docet85 You could either file a separate PR, or shoot a PR to my branch to have it included in this PR.

Advantage of filing separate being that the Flutter team might like your implementation better. 😄

I thought about implementing the Document versions, but couldn't think of a synchronous way to do so, as it wants DocumentSnapshot not DocumentReference, and DocumentSnapshot isn't currently possible to translate from Flutter to native.

I could, however, see creating a special Query class that has something like a nextPage method on the Dart-side that takes the QuerySnapshot's last document on the native side and pass that to startAfter.

But I'm still getting the hang of this, so I could be way off the mark.

@docet85
Copy link

docet85 commented Jan 17, 2018

@Skylled that's true. I have just realized the translation of DocumentSnapshots to native is non-trivial. Maybe I'll investigate this myself and be back with some ideas, most probably not posting here in this PR

Concerning the nextPage it is indeed the other interesting opportunity to get paging: of course, having the *Document methods, one could implement paging it directly in Dart, but given the DocumentSnapshot translation issue probably it's better to go that way.

Anyhow terrific work - great to have the array version of orderBy! Get this PR merged.

Let's keep in touch!

Best

@collinjackson collinjackson self-assigned this Jan 17, 2018
@collinjackson
Copy link
Contributor

Looks great, thanks!

@collinjackson collinjackson merged commit 97344ae into flutter:master Jan 20, 2018
@Skylled Skylled deleted the firestore-query-methods branch January 21, 2018 04:17
@naumanahmed19
Copy link

is there any example for startAt, startAfter ? I tried following code but getting error at startAfter

 final docRef = Firestore.instance.collection('cities');

 var first = docRef.limit(25);

    return docRef.getDocuments().then((documentSnapshots) {
      // Get the last visible document
      var lastVisible =
          documentSnapshots.documents[documentSnapshots.documents.length - 1];

      // Construct a new query starting at this document,
      // get the next 25 cities.
      var next = docRef.orderBy("population").startAfter(lastVisible).limit(25);
    });

Error:

    The argument type 'DocumentSnapshot' can't be assigned to the parameter type 'List'.

@Skylled
Copy link
Author

Skylled commented Sep 26, 2018

When this code was written, it was not yet possible to implement these methods using DocumentSnapshot, but instead had to use the secondary versions that used a list of values.
API reference here.

Now that #429 is out and flutter/flutter#13043 is long behind us, it may be worth creating new versions of the methods using DocumentSnapshot.

julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
* Extend orderBy to be used multiple times.

* Implement startAt/After and endAt/Before

* Implement `limit` method

* Formatting fix

* Fix missing parameter list and related tests

* Formatting fixes

* Pass start/end methods an array, not a list

* Update CHANGELOG.md

* Update pubspec.yaml
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants