Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

Add user_roles table for organizers and admins #296

Closed
vkWeb opened this issue Dec 15, 2019 · 44 comments · Fixed by #335
Closed

Add user_roles table for organizers and admins #296

vkWeb opened this issue Dec 15, 2019 · 44 comments · Fixed by #335
Labels
Good First Issue This issue is beginner friendly! It shouldn't take much experience to get up and running.

Comments

@vkWeb
Copy link
Member

vkWeb commented Dec 15, 2019

Create a user_roles table with id and type as string and reference the id of user_roles via users table.

@vkWeb vkWeb added Beginner Friendly Help Wanted Extra attention is needed labels Dec 15, 2019
@AryanJ-NYC
Copy link
Contributor

Relates to #16.

@Madalena-15
Copy link

Madalena-15 commented Dec 24, 2019

Based on the discussion in Discord, I have explained what I thought each role (organiser and admin) could do:

Admin
Only the administrator is able to do create chapters, although they have full control of what is going on in the Chapter instance, they probably will have little involvement in specific chapters (such as CRUD events, managing chapter members unless the situation is high-level priority). Other than that, they aim to focus on admin tasks and keeping the Chapter sailing afloat.

Organiser
The chapter organiser focuses on managing and organising events for that chapter they are assigned to. That does include any kind of tasks relating to their chapter only.

Also, this can be useful for chapters to have more than one organiser (or at least a co-organiser), as unforeseen circumstances may occur and a particular organiser may not be able to do their role, at least the co-organiser can fill in the gap (even if it is just temporarily).

@vkWeb
Copy link
Member Author

vkWeb commented Dec 24, 2019

@madaleneaza-design @allella I think the organizers shouldn't be able to add other organizers to the chapter they are appointed to. Suppose fCC is the organization and Quincy is the admin. Now the admin appointed me as an organizer for fCC Delhi (a chapter under fCC). If I add Tim as an organizer of fCC Delhi then we will need to put there a feature to inform Quincy that Tim has been added as an organizer for fCC Delhi by me because in Quincy's mind I should be the only organizer.

We can make this thing more feature-rich by giving admin an option to "allow the organizer to add more organizers" but I don't think this should be in our MVP. So only administrators should be able to add new organizers to keep things a bit straightforward for our MVP. If the admin needs help or is busy then he/she can add more admin(s).

So here's my take on the permissions of administrators and organizers:

Administrators

  • can add, remove admins
  • can add, remove organizers for a chapter
  • can CRUD chapters and its details
  • can CRUD any chapters' event and its details
  • can email, ban any chapter organizer
  • can email, ban users
    • can email or ban only when the user has attended an event under any of the chapters
    • can email only when the user has subscribed to one of the chapter
  • has full control over the Chapter instance and site-wide settings

Organizers

  • can CRUD events and its details only under the chapter they are appointed to
  • can email, ban users under the chapter they are appointed to
    • can email or ban only when the user has attended an event under that chapter
    • can email only when the user has subscribed to the chapter

@allella @madaleneaza-design how does that look? Tell me If I missed something.

@allella
Copy link
Contributor

allella commented Dec 24, 2019

@vkWeb
Looks good.

I'm hip to holding back the "organizers adding other organizers to their chapter" and keeping it out of the MVP.

I feel this will be a desirable future (post-MVP) feature. As you said, if it's an option to turn delegation on / off then the admin could pick between tighter control of organizers, or delegating that permission to organizers.

@Madalena-15
Copy link

@vkWeb These are really good details and points you have written. 🙂

I really like your suggestion of the feature, which is " giving admin an option to "allow the organizer to add more organizers".

The reason being is we must remember that every user (organisation) comes in different sizes, and structures of running their system. So, not all organisations will agree with our suggestions such as one may not like the idea of organisers being allowed to add another organiser (my suggestion). That could be the same for your suggestion of admins adding other admins, some may like that idea, but not to have the full control of the Chapter instance (basically you may get feedback from users that they may want an assistant admin where it holds more control that organisers, but less than the admin - this could end up being another feature to be considered.

However, I do understand that there may be a limit to what we can provide, which is why the vital part of the UX design process is user research and that is to gather insights of both qualitative research (why the user prefers that feature) and quantitative research (how many users prefers that feature).

Therefore, I can agree with @allella on this feature being the future MVP, and go with whatever will be decided for now and hopefully with some feedback from users for any improvement, we can work on it then.

@allella
Copy link
Contributor

allella commented Jan 16, 2020

Being that this is a prerequisite for #16 I wanted to see if I can help keep it going for @AryanJ-NYC

@vkWeb started working on a user_role table in #253 and that was rolled into this issue because it wasn't as fleshed out as the rest of that PR.

@timmyichen had mentioned having a chapter_id key on the user_role
to go along with the user_role.id and user_role.type.

The "type" lead into whether to specify these values as an enum or a string and @Zeko369 weighed in on using strings over enums for simplicity. @timmyichen does that should good to you or was there a specific migrations concern?

I assume the "type" would be defined in the DDL, similar to the sponsor_type, but as a string. Is that right?

create type sponsor_type as enum
('FOOD', 'VENUE', 'OTHER');

#313 was just merged in (by me, sorry, not sorry) and it aimed to provide clarity on the user role terminology in the README.

Hopefully, that's enough to get us chatting again and move along on the issues related to administrator and organizer and user roles. I think those are the three roles for the MVP.

Now, all we have to decide is organizer vs organiser :)

@allella
Copy link
Contributor

allella commented Jan 16, 2020

And, even though this is more about authorization then authentication, I feel @vaibhavsingh97 should weigh in since he was looking at #305

@AryanJ-NYC
Copy link
Contributor

Will a user only have one role?

@AryanJ-NYC
Copy link
Contributor

Either way, please assign this to me.

@allella
Copy link
Contributor

allella commented Jan 17, 2020

@AryanJ-NYC I think it's safe to assume a user might have more than one record in the user_role table.

For instance, if a user is an organizer or two different chapters then they'd likely need a record for each user + role + chapter combination because being an organizer for one chapter doesn't mean you can organize all the organization's chapters

@allella
Copy link
Contributor

allella commented Jan 17, 2020

Also, I was thinking that for the regular user, who is not an organizer or an administrator, it may be helpful if we call their role something like member.

If we didn't differentiate the default / base user role, then we'd potentially end up saying things like "a basic user has the role of user", which is likely to get rather confusing.

If the basic / default role of a user is the member role, then we could describe roles like:

  • a user with the role of administrator can create new chapters
  • a user with the role of organizer can email chapter members
  • a user with the role of member can RSVP for an event

@madaleneaza-design @vkWeb and I described the administrator and organizer roles above on this thread, but we didn't talk about the default/base role, which I'm proposing we don't call the user role, since user is already an entity.

Thoughts on these role names? @AryanJ-NYC @madaleneaza-design @vkWeb @Zeko369 @timmyichen @vaibhavsingh97

@allella allella assigned allella and AryanJ-NYC and unassigned allella Jan 17, 2020
@AryanJ-NYC
Copy link
Contributor

I like the names a lot. Just to confirm and make sure I have it straight:

  • A user will have exactly one role per chapter they belong to (they cannot be both an organizer and an admin of a chapter).
    • That assumes that admins have all the permissions thag organizers have (and more) who, in turn, have all the permissions that user have (and more).

Does this ticket entail setting up the user role permissions as well?

@allella
Copy link
Contributor

allella commented Jan 17, 2020

@AryanJ-NYC one complication with the roles, no matter what we call them, is that the administrator is the administrator for the entire organization and not just a single chapter. As the "super user" we don't gain anything by mapping a user as an administrator for every single chapter.

So, I'm not immediately clear if a user_roles table is ideal, or even necessary for capturing the administrator role. Though, unless we're going to assume user # 1 is always the administrator, then we'd need to store the admin's user ID somewhere in the configuration or database.

For a moment, let's assume user # 1 is always the administrator, then would we prefer to scrap the user_roles table and extend the existing user_chapters table since it already maps users to chapters?

user_chapter.user_id
user_chapter.chapter_id
user_chapter.role_id

I think the reason this is a bit confusing is the administrator role is more of a system role for the entire organization, whereas the organizer and member roles would be more specific to a chapter.

@vkWeb
Copy link
Member Author

vkWeb commented Jan 17, 2020

Also, I was thinking that for the regular user, who is not an organizer or an administrator, it may be helpful if we call their role something like member.

@allella Thanks for starting the conversation flow after Christmas break :)

So you mean if they sign up in the organization, they become its member? #106 will conflict with naming discussed here. So we need to clear this confusion.

A user will have exactly one role per chapter they belong to (they cannot be both an organizer and an admin of a chapter).

That assumes that admins have all the permissions thag organizers have (and more) who, in turn, have all the permissions that user have (and more).

Exactly @AryanJ-NYC. But users (normal visitors) won't have any special permission I believe.

@vkWeb vkWeb removed the Help Wanted Extra attention is needed label Jan 17, 2020
@AryanJ-NYC
Copy link
Contributor

@allella one idea would be an is_admin boolean field on the user model while having the user_chapter_rolestable be responsible for only member roles and member membership to organization (if there exists a user_id and org_id combo inside the table, then the user is a member of that org).

@allella
Copy link
Contributor

allella commented Jan 17, 2020

@AryanJ-NYC yes, I agree on a user.is_admin boolean. I'm not sure we'll have other organization-level roles besides an administrator. An admin flag would keep it simple and still allow an organization to have one or more admins. Plus, this avoids the confusion of mixing system-wide and chapter-level roles.

I think @AryanJ-NYC was suggesting we'd rename the user_chapters table to user_chapter_roles and I think that will work. Would this be called user_chapter_roles or _user_chapters_roles?

@vkWeb I think the title from #106 is hinting at the need for a member role because we are naturally referring to a basic user as a _ chapter member_.

The user is an entity, but they can also have specific roles related to a specific chapter. If someone signs up for a user account and never joins a chapter then they are still a user with no role. When a user joins a chapter they inherit a role of member and a record is added to the _user_chapter(s)_roles table.

Thoughts?

@Madalena-15
Copy link

I am not exactly sure if this is relevant to this issue, but last week as part of the UX user research, I created a "user role hierarchy" map (see the photo).

user role map

Each post-it is a user role and each colour indicates a "confirmation" status:

  • Green - we are definitely having this role
  • Yellow - it is favoured and likely to have this role but not yet officially confirmed
  • Orange - either not likely to have this at this moment but possible in later future OR it has not been discussed yet.

So, the co-organiser is the only one where it is not happening, but sitting on the shelves i in case it may ever be needed. The rest of the orange post-its are the ones I have not yet to discuss it with you.

This is only a very basic map, but wanted to see if this is useful before I go any further and also it may help to give you some ideas too 🤷🏼‍♀️

PS: I hope you can read my handwriting!

@allella
Copy link
Contributor

allella commented Jan 18, 2020

Thanks @madaleneaza-design I think that we're all using the word member without thinking about it tells use that member is a good name for the basic chapter role.

We'll need to update the README terminology to differentiate the user and member terminology. I think it's pretty easy to understand administrator, organiser, and member are the possible roles of a user.

@allella
Copy link
Contributor

allella commented Jan 18, 2020

@madaleneaza-design also what do you think about

  • user who is a chapter member
  • user with no chapter role
  • visitor

to describe the bottom row of sticky notes (from left to right)?

@allella
Copy link
Contributor

allella commented Feb 9, 2020

A starter list of Administrator FAQs have been roughed in on a wiki page. Feel free to edit the wiki or suggest more questions admins might have either here or on Discord.

@allella
Copy link
Contributor

allella commented Feb 12, 2020

There was reference to a "super admin" in the Figma comments. However, @tomiiide agreed "an admin is an admin".

So, to recap and pinging @Zeko369 @timmyichen in case they missed this conversation.

There currently is no user roles table, which is why this issue exists.

The suggestions above yielded the desire to add a "user_chapter_role" table since roles like organizer and member are specific to each chapter.

Then, (via @AryanJ-NYC ) we came up with an is_admin flag on the user table to toggle on a system-level admin "role" since trying to make a system-wide admin wouldn't fit well in a user_chapter_roles table.

When the instance is deployed we'll need a "user # 1" concept which will automatically have user.is_admin set to TRUE.

If / as other users are invited and "user # 1" sets the user.is_admin flag on them to TRUE then they become an administrator, just the same as user # 1.

Based on this conversation do we want to pick a common terminology to refer to "user # 1" / "super admin".

I don't really like either of them, so I'm hoping someone has a better suggestion to define this initial, automatic-admin user.

OR, should we just continue calling anybody with user.is_admin = TRUE and admin and avoid calling the first user anything more than an admin?

@allella
Copy link
Contributor

allella commented Feb 12, 2020

More thoughts / questions regarding "what do we call the person who setups up the instance and becomes the first user".

@tomiiide and I were talking and he suggested owner or creator.

I thought perhaps "instance owner" or "instance creator" could work as a term.

This person would also be the one setting up the hosting for the Docker and probably registering 3rd party services, doing the configuration, and more. So, perhaps we need to give them a unique name.

They will certainly be an administrator, but one with the special job of kick-starting the initial instance.

@timmyichen
Copy link
Contributor

Creator works in this case. I'll throw up a PR for the following:

  • New table: user_chapter_role with possible values "organizer", "member"
  • New table: user_global_role with possible values "creator", "admin"

@allella
Copy link
Contributor

allella commented Feb 12, 2020

Thanks @timmyichen I'm not opposed to the global roles, but do we think that's going to complicate the permissions and logic versus having the is_admin flag?

Obviously, it gives more flexibility, but I wonder if the creator and administrator roles would have any different permissions where we need two roles.

@Zeko369
Copy link
Member

Zeko369 commented Feb 12, 2020

I think that @timmyichen suggestion is better because we can easily add helpers to the user class to have user.isAdmin and user.isCreator.

Also we should add possible value to user_global_role: "member" or "user" for the default role.
Also maybe we could just have it as a roleId on the user and have a enum with roles and just have every higher role inherit the child (organiser has everything that member has)

@timmyichen
Copy link
Contributor

Obviously, it gives more flexibility, but I wonder if the creator and administrator roles would have any different permissions where we need two roles.

Admins probably shouldn't be able to invite other admins; that should solely be within the hands of Creator.

@allella
Copy link
Contributor

allella commented Feb 12, 2020

@Zeko369 do we want to keep the user booleans if we have a user_global_role table with an "owner" role. Does that make it easier to lookup the permissions, or is it redundant and something that might get out of sync?

@timmyichen yes, if we assume only an owner can add more admins, then that would make a place for an "owner" role, or the isAdmin / isOwner flags. Heretofore, we've been operating on the premise that and admin is an admin.

It's not uncommon to see "owner" in hosting platforms, so I guess it's a decent expectation to follow. If that's the case then the first user of the system would be differentiated by more than just the initial creation, so perhaps "owner" is a better role/term than "creator" given these new assumptions.

Also we should add possible value to user_global_role: "member" or "user" for the default role.

The "member" and "organizer" roles obviously make more sense in the context of user_chapter_roles table, so I don't see having those go into user_global_role

Since "user" is a table already I don't see a need to having people be a "role" of user in user_global_role. If they exist in the user table then they are a user, but there's no special permissions of being a user.

could just have it as a roleId on the user and have a enum with roles and just have every higher role inherit the child (organiser has everything that member has)
Parent-child relationships always tend to complicate things in the code, so my gut is to always look for a simple, elegant solution using just having people have one role in user_global_role and only one role, per chapter, in user_chapter_role.

@timmyichen
Copy link
Contributor

No booleans would be necessary here. Lookups are slightly more costly but it shouldn't matter given the admin/owner role will not be checked very often. Also, if we're going to have 2 or more related boolean fields on an row, more often times than not it should go into its own enum table unless quick lookups are essential.

No need to do any role inheritance. A chapter organizer will have two entries in the user_chapter_roles for member and organizer.

@allella
Copy link
Contributor

allella commented Feb 12, 2020

Okay, so to summarize:
(EDIT)

  • New table: user_chapter_role with possible role values organizer, member. A user can have 0 or more roles per chapter.
  • New table: user_instance_role with possible role values owner, administrator
  • Update the README to describe the role of owner to differentiate it for a typical administrator

@allella
Copy link
Contributor

allella commented Feb 12, 2020

Just for consistency with the terminology, would we consider user_instance_role instead of user_global_role ? @timmyichen @Zeko369

@allella
Copy link
Contributor

allella commented Feb 12, 2020

@vkWeb @AryanJ-NYC @madaleneaza-design are you all hip to #296 (comment) ?

@AryanJ-NYC
Copy link
Contributor

@allella Acknowledged. Sorry I've been dragging my feet here!

@allella
Copy link
Contributor

allella commented Feb 12, 2020

@AryanJ-NYC @timmyichen may have a PR coming for this, so perhaps you can work with him, or review any PRs related to this issue.

@timmyichen
Copy link
Contributor

timmyichen commented Feb 12, 2020

@AryanJ-NYC do you already have a branch up for this WIP? you might be able to finish it faster haha. Ideally we'd be able to get the PR up by tomorrow. Let me know if you'd rather me take it off your hands!

@vkWeb
Copy link
Member Author

vkWeb commented Feb 13, 2020

@allella Yes, I am with you all for creator and admin roles.

@AryanJ-NYC AryanJ-NYC removed their assignment Feb 13, 2020
@AryanJ-NYC
Copy link
Contributor

All good, @timmyichen. Apologies again!

@allella
Copy link
Contributor

allella commented Nov 7, 2020

(via Discord) I don't think we have the permissions logic entirely figured out yet, but the ability for a user to post events on 1 or more chapters will depend on their authorization.

Authorization will come from either of

  • user_chapter_roles with possible role values organizer, member. A user can have 0 or more roles per chapter.
  • user_instance_roles with possible role values owner, administrator

Lots of earlier discussion on this issue led to this summary #296 (comment) which was then closed via PR: #335

A user with the role of owner or administrator would be able to CRUD events for all chapters in the instance and we infer that from their role in user_instance_roles and any event.chapter_id values would be from a drop-down field that populated with all values from the chapters table.

An organizer would be a user with specific authorization role entries via the user_chapter_roles table. They would have 1 or more such roles previously granted by the administrator or owner. The user_instance_roles.chapter_id values would be joined to the chapters table to populate a form field (drop-down) of possible values for events.chapter_id when an organizer creates or updates an event. Or, in the case the organizer only has authorization to a single chapter, then perhaps the app logic forces the events.chapter_id to that specific chapter_id

The role tables aren't reflected in the out-of-date schema ER diagram, but the rest of the earlier schema is shown at
https://github.com/freeCodeCamp/chapter/blob/master/docs/data/schema.png

@allella
Copy link
Contributor

allella commented Nov 7, 2020

@katien I edited the above comment to remove the part about an organizer having a role in user_instance_roles. It seems we decided that role would get all its authorizations from user_chapter_roles.

Though, I wouldn't entirely rule out user_instance_roles also accepting an organizer role for reasons not related to the CRUD of events, but it's probably not necessary / advisable and we'll assume that's not the case for now.

@allella
Copy link
Contributor

allella commented Feb 1, 2022

Linking this old issue to the most recent conversation about roles tables in case anybody stumbles up the previous direction.
#735 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Good First Issue This issue is beginner friendly! It shouldn't take much experience to get up and running.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants