Skip to content

docs: add snippet for a subcollection query #23

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

Merged
merged 2 commits into from
May 17, 2023

Conversation

jlara310
Copy link
Contributor

@jlara310 jlara310 commented May 8, 2023

Add a beginner-level snippet that shows how to query a subcollection.
Similar PR for Android: firebase/snippets-android#447
Note that I added two snippet tags: one for the convention used by the file and one for
the convention used on cloud.google.com

Risk Level

  • [ x ] No risk
  • Somewhat Risky
  • High risk

Pre-submit checklist

@thatfiredev thatfiredev requested review from kroikie and nohe427 May 9, 2023 09:21
Copy link
Contributor

@nohe427 nohe427 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@nohe427
Copy link
Contributor

nohe427 commented May 9, 2023

@thatfiredev -> Do you know why CI/CD might be breaking?

@thatfiredev
Copy link
Member

@nohe427 It looks like count() was introduced in FlutterFire Firestore v4.0.0, but CI is using 3.1.11:

lib/snippets/firestore.dart:827:28: Error: The method 'count' isn't defined for the class 'CollectionReference<Map<String, dynamic>>'.
 - 'CollectionReference' is from 'package:cloud_firestore/cloud_firestore.dart' ('../../../../../.pub-cache/hosted/pub.dev/cloud_firestore-3.1.11/lib/cloud_firestore.dart').
 - 'Map' is from 'dart:core'.
Try correcting the name to the name of an existing method, or defining a method named 'count'.

@thatfiredev
Copy link
Member

Oh, looks like 3.1.x is coming from pubspec.yaml:

@nohe427
Copy link
Contributor

nohe427 commented May 11, 2023

@jlara310 - Could you run flutter pub upgrade --major-versions to see if that helps get CI to run successfully?

@jlara310
Copy link
Contributor Author

@jlara310 - Could you run flutter pub upgrade --major-versions to see if that helps get CI to run successfully?

Done, but I encountered additional errors when trying to run the app locally. Filed bugs here:

@thatfiredev
Copy link
Member

thatfiredev commented May 12, 2023

@jlara310 Thanks for filing those. Can you commit your current pubspec files after you ran that command so that CI can pick it up in this PR?

@jlara310
Copy link
Contributor Author

@jlara310 Thanks for filing those. Can you commit your current pubspec files after you ran that command so that CI can pick it up in this PR?

Yep. Done.

@nohe427
Copy link
Contributor

nohe427 commented May 17, 2023

I am going to merge this since it seems the last few merges were also failing. I will then go through and try and fix the logged bugs. Thanks for making these changes and documenting the new errors. This will give us a great launching off point. @thatfiredev - I will ask you for reviews :)

@nohe427 nohe427 merged commit fdb1bd8 into firebase:main May 17, 2023
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.

3 participants