Fix backfill for subcollections #52
Conversation
|
Hi, |
| @@ -40,8 +40,16 @@ module.exports = functions.handler.firestore.document | |||
| return false; | |||
| } | |||
|
|
|||
| const querySnapshot = | |||
| await admin.firestore().collection(config.firestoreCollectionPath).get(); | |||
| const isSubcollection = config.firestoreCollectionPath.includes("/"); | |||
There was a problem hiding this comment.
Is there a case where a collection path can include / and it's not a sub-collection?
For eg, let's say a set of admin users are nested under users/admin, and regular users are nested under users/regular?
There was a problem hiding this comment.
This cannot be the case. users/admin and users/regular are not valid subcollections. In that example there would be a document with the id admin under the users collection and a document with the id regular under the users collection. The correct syntax for subcollections would be users/{userId}/admin and users/{userId}/regular
| let query; | ||
| if (isSubcollection) { | ||
| const collectionGroup = config.firestoreCollectionPath.split("/").pop(); | ||
| query = admin.firestore().collectionGroup(collectionGroup); |
There was a problem hiding this comment.
Does this support collections with dynamic path names? Eg: /users/{user_id}/companies
|
Hey, any Chance to get this merged soon? This feature is really needed and the PR looks good for me. This is blocking my migration from Algolia to typesense. |
|
I am also being blocked by this. The sooner this can get merged the better. |
|
@guenth39 Could you add test cases for this PR? |
|
In case you don't want to wait this MR to be merged. You can directly modify the cloud function via the google console. And replace the backfill function with : https://github.com/Bobbele-Ideas/firestore-typesense-search/blob/Fix-backfill-for-subcollections/functions/src/backfillToTypesenseFromFirestore.js |
|
Any update on this issue ? In multi tenants application this features is mandatory. |
|
+1 on this one, would be great to be able to backfill sub-collections. |
|
Hey, any update on this issue? I'd be glad to help if needed. |
|
Thanks @Jad31. We're essentially missing tests for this PR, and now branch conflicts. Feel free to branch off of this branch and submit a new PR |
|
#99 should provide support here. Prob can close this. |
Change Summary
With these changes, the backfill will now work also for subcollections, not only for "root" collection. If the collection path is something like users/{userId}/items the sync has already worked, but the backfill did not as the documents were queried with this collection path, but that did not work. Instead, we use the collectionGroup feature now with only the last part of the path.
Closes #17
PR Checklist