Meteor-Community-Packages / meteor-roles

Authorization package for Meteor, compatible with built-in accounts packages
http://meteor-community-packages.github.io/meteor-roles/
MIT License
921 stars 168 forks source link

Feature request: API methods to rename and delete groups #153

Closed davidecantini closed 8 years ago

davidecantini commented 8 years ago

Hello,

In some cases, it is necessary to change Group names. As far as I could see, with the current version, the API does not have any method to either rename or delete a group. Are you planning to add such feature in an upcoming version?

Thank you in advance for your attention. Keep up the good work. Regards

alanning commented 8 years ago

Hi @davidecantini, Roles v2.0 will support much easier group management and should land before the end of Jan. 2016.

To do this in the current version of roles, you need to do it manually via mongo updates.

For example, starting with:

Roles.addUsersToRoles(joesUserId, ['manage-team','schedule-game'], 'manchester-united.com')
Roles.addUsersToRoles(joesUserId, ['player','goalie'], 'real-madrid.com')

// NOTE: MongoDB uses periods to represent hierarchy so periods in group names
//   are converted to underscores.
//
// user.roles = {
//   'manchester-united_com': ['manage-team','schedule-game'],
//   'real-madrid_com': ['player','goalie']
// }

... we can rename "manchester-united.com" to "manutd.com" like so:

> // in mongo shell
> db.users.update({}, {$rename: {"roles.manchester-united_com": "manutd_com"}}, {multi: true})
davidecantini commented 8 years ago

Hi @alanning, Sounds good! Looking forward to version 2.0, which I assume will include two API methods to rename and delete groups. Thanks also for the example you provided.

mitar commented 8 years ago

Hm, we should maybe discuss that. While deleting of roles is implemented in v2.0, I explicitly prevented renaming of roles because it is pretty tricky to manage that when you have hierarchy of roles, and because renaming is also linked with the code changes, because you also have to change the code where you used the previous name. So for me this is more something which should be done in the database migration along with your code changes, and not at runtime through a normal API. (We could provide some helper function for database migration when renaming a role.)

What I would suggest is that role's logical name and role's display name should be anyway a different thing. Because role's display name is probably something which is depended on the language.

So, instead of supporting renaming, I would suggest that you look into how to use some translation library for displaying role names to the user.

davidecantini commented 8 years ago

@mitar: • I assume that when you say "roles", you actually mean "groups", which is what this issue refers to. • I may be missing something but I don't get your point. If the package lets you create a group, it should also allow you to rename and delete it, through the API, and not by requiring the application to modify directly its internal data structure (for obvious reasons). The purpose of the package ends there, it doesn't have to make any assumption on what the application does.

mitar commented 8 years ago

I assume that when you say "roles", you actually mean "groups", which is what this issue refers to.

Oh, sorry. I got confused. No, I talked about roles.

Groups/partitions can be renamed without problems in v2.0. There is no API for that though yet, but it is really just a question of one update query on users collections, so it should be easy added.

davidecantini commented 8 years ago

No problem. As for the missing API method(s): it's true that it takes just an update on the users collections, but I believe that the application should only use the API and not modify directly the internal data structure of the package. Do you agree on that?

mitar commented 8 years ago

Yes, we should provide such a method.

davidecantini commented 8 years ago

@mitar: Thanks, I updated the title to feature request.
@alanning: I'd suggest to re-open this issue and close it upon releasing version 2.0. Thanks.

mitar commented 8 years ago

Implemented in v2.0.

davidecantini commented 8 years ago

Please keep the ticket open until the fix/feature is released.

Thank you guys, I appreciate it.

davidecantini commented 8 years ago

Thank you for re-opening it. I guess it's a good practice to do that, so other people know that the issue has been reported already, also preventing others from having to submit the same request. Thank you guys for the great work, keep it up!

mitar commented 8 years ago

After more thought I am closing this. I see open issues more as what more has to be done on the project and it helps me to know what was resolved. Releasing a version is another item and is tracked in #165.

V1.0 is in feature freeze and will not be really getting any more updates besides some bugfixes and compatibility fixes.

Moreover, I think GitHub search finds closed tickets well as well.