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

Why adminRefs? #4

Closed
jeffreydavis83 opened this issue Jan 7, 2021 · 8 comments
Closed

Why adminRefs? #4

jeffreydavis83 opened this issue Jan 7, 2021 · 8 comments

Comments

@jeffreydavis83
Copy link

Hi Chaoming Li, thanks for this amazing starter template. I am new to web dev so this has been much of help.

I just wanted to ask that why do we need "adminRefs" in our database? Like we already have the "admin" array to check if user has the permissions.

@chaoming
Copy link
Collaborator

chaoming commented Jan 7, 2021

The adminRefs and userRefs are for reading the user documents under the users collection.

@jeffreydavis83
Copy link
Author

Makes sense. Thanks!

@jeffreydavis83
Copy link
Author

Sorry I am gonna ask another noob question,

What if we don’t use the adminRefs and userRefs and instead fetch from the database using id only? I am just curious how adminRefs and userRefs are benefiting us?

@chaoming
Copy link
Collaborator

No worries.

There are two approaches to not use adminRefs and userRefs.

  1. After reading the user IDs from access or admins array, loop through the users collection one by one based on the list of IDs. I think this is definitely not a very efficient way to retrieve data because each ID will need an API call to the users collection to retrieve the user document.

  2. After reading the user IDs from access or admins array, execute a query to retrieve all the user documents from the users collection that match the IDs in the array. However, I can't find examples to query a collection based on a list of IDs in the document https://firebase.google.com/docs/firestore/query-data/queries, so I designed the system to use array of reference to resolve it. This is possible based on .where(admin.firestore.FieldPath.documentId(), 'in', ['id1', 'id2']) fails: '... FieldPath.documentId() must be a string or a DocumentReference' googleapis/nodejs-firestore#990, but I am not sure about the performance compared to the current method.

Cheers,

@jeffreydavis83
Copy link
Author

Thanks man! I just have this one doubt,

The cloud function getAccountUsers includes:

 let getUsers = [];
            accountRef.data().accessRefs.forEach(user => {
                getUsers.push(getDoc('users/'+user.id));
            });
            return Promise.all(getUsers);

Aren’t you executing getDoc for each user? Wouldn’t each user then will require an API call in this case?

@chaoming
Copy link
Collaborator

True, looks like it's OK to remove adminRefs and accessRefs and simply to use admins and access to simplify the data structure. Thanks for the question.

@chaoming
Copy link
Collaborator

This issue is resolved in commit dd1286c

@jeffreydavis83
Copy link
Author

Thanks!

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

No branches or pull requests

2 participants