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

Support for Query.getDocuments #376

Merged
merged 9 commits into from
Feb 2, 2018

Conversation

briblanch
Copy link
Contributor

@briblanch briblanch commented Jan 29, 2018

Description

  • The plugin currently does not support Query.getDocuments() as shown in the api here.

What was changed

  • Added new Query.getDocuments() method.
  • Abstracted out the parsing of a QuerySnapshot from the listener handlers to be shared with Query.getDocuments

Testing

  • I have modified the example app slightly here to make this new functionality easy to test.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@briblanch briblanch changed the title Support for Query.getDocuments Support for Query.getDocuments Jan 29, 2018
@briblanch
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@Hixie
Copy link
Contributor

Hixie commented Jan 30, 2018

cc @collinjackson

Copy link
Contributor

@kroikie kroikie left a comment

Choose a reason for hiding this comment

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

@briblanch Thanks for the PR could you have a look at the suggestions I made.

@@ -1,3 +1,6 @@
## 0.2.7
Copy link
Contributor

Choose a reason for hiding this comment

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

Please bump this to match current version.

case "Query#getDocuments":
{
Map<String, Object> arguments = call.arguments();
CollectionReference collectionReference = getCollectionReference(arguments);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here the entire collection will be retrieved, I think you wanted to use getQuery here.

Future<QuerySnapshot> getDocuments() async {
final Map<String, dynamic> data = await Firestore.channel.invokeMethod(
'Query#getDocuments',
<String, dynamic>{
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the _parameters to this Map, otherwise all of the collection is returned.

<Matcher>[
isMethodCall(
'Query#getDocuments',
arguments: <String, dynamic>{'path': 'foo'},
Copy link
Contributor

Choose a reason for hiding this comment

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

Update test to check for expected parameters that would go along with the getDocuments call.

…store-get-documents

# Conflicts:
#	packages/cloud_firestore/CHANGELOG.md
#	packages/cloud_firestore/android/src/main/java/io/flutter/plugins/firebase/cloudfirestore/CloudFirestorePlugin.java
@briblanch
Copy link
Contributor Author

@kroikie addressed your feedback. Thanks!

@kroikie kroikie merged commit 0e5f4e3 into flutter:master Feb 2, 2018
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
Add support of retrieving the documents that match a query.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants