-
-
Notifications
You must be signed in to change notification settings - Fork 169
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
Pass in userIds to narrow down result #305
Comments
To be quite honest ... in this case I would also think about extendability. How far would this solution scale? Would it help others or just something that is rather quite unique to my case. I had a similar case, where I was wondering about if my use-case could be something that would help others as well. In the end I decided that it's quite unique:
What I ended up with is a small wrapper-plugin around this package which has a fixed major-version dependency. In this way you can be assured that you will get notified if something in the database structure changes, and you'll not be able to update until you allow it in the dependencies. Unless I get someone else to vote for this feature, I wouldn't take it in, but rather ask you to take the approach of a wrapper-package, where you can write the query in the most performant way for your use-case. Leaving it open for feedback and upvotes. |
Thanks for the feedback / suggestion. I guess the main case for adding it to this package would be, do you see users / Meteor being something that would attract thousands / millions of users. In that scenario, using "_getUsersInRoleCursor" would not scale well and make this package not really usable for high volume scenarios. For example, if there was a million users, and you returned the millions of users with "_getUsersInRoleCursor", what if all those million users used the app at the same time? That would be 1 trillion records you've just pulled down when it could be easily avoided. |
Well, this option starts with an underscore, which makes it kind-of internal by convention, I guess - but referring to the function The documentation of the function clearly states:
If you have too much data, you can use the query-option parameter to limit the amount, but you then clearly want all the assignments users have on a specific role ... otherwise you wouldn't use this function, would you ..? So depending on your data-set you either want or don't want the list. Or am I taking this from the wrong perspective? |
The initial function is I guess where I'm coming from is, the package only allows you to pull data for one user, or pull data for all users instead. Ideally there should be the option to only pull data for the users that you need it for. Having to verify 1 million+ users are in a role when you only need to verify 50+ doesn't seem logical, even if you limit the data in the query options. |
The package is currently only meant for listing all roles of a user or all users of a role, correct. As said, you always have the option to extend this package by a wrapper package. I also have a lot of custom queries in my project, which I set up as a wrapper to this package. This way, I can write queries as most effective for my project while still retain the version requirement. Your example looks like you want to do what SQL calls a Closing this issue. If you think this is definitively a need, please provide a use-case where it wouldn't be used as a |
Yep, all good. I wasn't sure if it was in scope for the package, but for us it's much easier forking it and adding the 1 line of code. Just for context in our use case, we're using it inside a Meteor Method for searching for users, not a subscription. E.g, we need to find a user by their firstName or lastName (plus whatever other data). Using an Aggregate would be my go-to method, however that would mean hardcoding the logic into the app and would be outside this plugin, which is bad from a plugin update perspective. Since the top level function Roles.getUsersInRole already does what I want, I just want it to search on less users. The only practical way using this plugin without tampering with it as suggested above, would be to pull all 50 million user documents using Roles.getUsersInRole and with that result, loop through and check the ones that match the criteria. An extremely unoptimised way to do it. So for us, while the package is fantastic and helps a lot, for our case we can't use it at all without making this change, which works perfectly and it's the cleanest way I can think of to do it (since it's 1 line of code). |
Well, instead of forking the package, you can also write an extension to it. Just create a small package, which only lives in your code. Just need to create a file This link might help you to kick it off: https://guide.meteor.com/writing-atmosphere-packages.html#local-packages If I find some time (I think this will first be after the 4.x) I'll write a tutorial of how to extend this package. |
I've got an issue where there might be a million+ users for a particular role / scope and obviously it's not practical to pull the complete list of users into an array to then filter the result.
Specially, I want to search for a user who's firstName is "Amy", and from the results remove all the users who aren't part of that role.
The current way "meteor-roles" works as far as I can tell is that you can't do any of your own queries or narrow the result down. Currently it's pulling all the users that are a part of that role of which I'd then need to pluck the ids, and then pass that into the query to find the persons who's name matches.
This could pull back millions of users when using "getUsersInRole" based on a particular role.
The easiest way to make this work is to do the query first on the "users" collection to get the ids matching the "firstName", and then pass that array into "getUsersInRole" to then see if the supplied userIds belong to that role.
My thinking is to add this option into "_getUsersInRoleCursor":
For my scenario this works well and only pulls the docs that are actually needed / used.
The text was updated successfully, but these errors were encountered: