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(firestore): Support Firestore Collection Group Queries #2066

Merged
merged 13 commits into from
May 21, 2019

Conversation

jamesdaniels
Copy link
Member

@jamesdaniels jamesdaniels commented May 8, 2019

Checklist

  • Issue number for this PR: #nnn (required)
  • Docs included?: (yes/no; required for all API/functional changes)
  • Test units included?: (yes/no; required)
  • In a clean directory, yarn install, yarn test run successfully? (yes/no; required)

Description

Code sample

constructor(private afs: AngularFirestore) { }

ngOnInit() {
  ...
  // Get all the user's comments, no matter how deeply nested
  this.comments$ = afs.collectionGroup('Comments', ref => ref.where('user', '==', userId))
                      .valueChanges({ idField });
}

@jamesdaniels jamesdaniels self-assigned this May 8, 2019
@jamesdaniels jamesdaniels added this to the 5.2.0 milestone May 8, 2019
@jamesdaniels jamesdaniels changed the base branch from flaky_tests to master May 8, 2019 06:45
@jamesdaniels
Copy link
Member Author

FYI this has been released for testing on @next as of 5.2.0-beta.3

@jamesdaniels jamesdaniels marked this pull request as ready for review May 20, 2019 01:15
@jamesdaniels

This comment has been minimized.

@jamesdaniels jamesdaniels changed the title WIP Collection Group Queries feat(afs): Collection Group Queries May 20, 2019
@jamesdaniels jamesdaniels changed the title feat(afs): Collection Group Queries feat(firestore): Support Firestore Collection Group Queries May 20, 2019
@jamesdaniels
Copy link
Member Author

Adding @davideast for review, alternatively @jhuleatt this could be a good one for a pair review. LMK

}
```

`collectionGroup` returns an `AngularFirestoreCollectionGroup` which is similar to `AngularFirestoreCollection` but as it has no set reference there are no data operation methods such as `add`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I found this a bit tough to interpret on the first pass.

Maybe something like "... similar to AngularFirestoreCollection. The main difference is that AngularFirestoreCollectionGroup has no data operation methods such as add because it doesn't have a unique(?concrete?) reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

I like concrete.

* @param queryGroupFn
*/
collectionGroup<T>(collectionId: string, queryGroupFn?: QueryGroupFn): AngularFirestoreCollectionGroup<T> {
if (major < 6) { throw "collection group queries require Firebase JS SDK >= 6.0"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

worth throwing a link to the release notes into this error? https://firebase.google.com/support/release-notes/js#version_600_-_may_7_2019

Copy link
Member Author

Choose a reason for hiding this comment

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

Eh, probably not. I imagine we'll be talking about so few users over the next couple months & more advanced ones at that.

@jamesdaniels
Copy link
Member Author

Thanks for reviewing @jhuleatt

@jamesdaniels jamesdaniels merged commit c34c0f3 into master May 21, 2019
@jamesdaniels jamesdaniels deleted the collection_group branch May 21, 2019 20:51
@valterh4ck3r
Copy link

So awesome <3

I'm needing this, thank you for your feature request @jamesdaniels and @jhuleatt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants