feat: Adds event owner role to the system

[shreyanshdwivedi]

Fixes #6001

Checklist

• I have read the Contribution & Best practices Guide and my PR follows them.
• My branch is up-to-date with the Upstream development branch.
• The unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

• All the functions created/modified in this PR contain relevant docstrings.

Short description of what this resolves:

Currently, owners of the event cannot be differentiated. So organizer event role is to be changed to owner, to differentiate and make it clear

Changes proposed in this pull request:

Adds event owner role to the system

[shreyanshdwivedi]

@iamareebjamal I wanted to ask on thing. How are we storing roles in db? I see 6 roles in db but couldn't find any migration which adds those roles.
I want to update the roles table and insert owner at id=1. Can you please give me a little background?

[abhinavk96]

Why do you want to store it at ID 1? We don’t manually set the ID of the records. Role is searched by its name everywhere not ID

…

On Thu, 13 Jun 2019 at 5:26 PM, Shreyansh Dwivedi ***@***.***> wrote: @iamareebjamal <https://github.com/iamareebjamal> I wanted to ask on thing. How are we storing roles in db? I see 6 roles in db but couldn't find any migration which adds those roles. I want to update the roles table and insert owner at id=1. Can you please give me a little background? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#6042?email_source=notifications&email_token=AEDUDRN6PGWK44MU3524XZTP2IHFRA5CNFSM4HXWQNC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXTDBMI#issuecomment-501625009>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEDUDRNYHCCVSUBGECSSPC3P2IHFRANCNFSM4HXWQNCQ> .

[shreyanshdwivedi]

Actually there are a few tables where id 1 is being used to associate event and role. So I thought I could add a role at id 1 which will automatically make the user the owner. I thought I could do it in a single go.
However now I feel that there might be more than 1 organizer in production and there can be only one owner. So this will not be good.
I will add role and use its id to make 1st organizer the event owner.

[abhinavk96]

Okay, as to the answer to your original question I believe the roles are created in the create DB script.

…

On Thu, 13 Jun 2019 at 5:33 PM, Shreyansh Dwivedi ***@***.***> wrote: Actually there are a few tables where id 1 is being used to associate event and role. So I thought I could add a role at id 1 which will automatically make the user the owner. I thought I could do it in a single go. However now I feel that there might be more than 1 organizer in production and there can be only one owner. So this will not be good. I will add role and use its id to make 1st organizer the event owner. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6042?email_source=notifications&email_token=AEDUDRKU5IJHQ6ZVKA5P5O3P2IH5LA5CNFSM4HXWQNC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXTDSLI#issuecomment-501627181>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEDUDRNBRGS2YFBS3LSMIMTP2IH5LANCNFSM4HXWQNCQ> .

[iamareebjamal]

I believe the roles are created in the create DB script

This is problematic

Make a non conflicting migration for it

[abhinavk96]

@shreyanshdwivedi here,

open-event-server/populate_db.py

Line 60 in f192ad5

def create_roles():

[shreyanshdwivedi]

@CosmicCoder96 yes I got that earlier but how will it reflect in my db because changing it is not affecting my roles table.
I think I'll have to make a migration file to add roles in db as mentioned by @iamareebjamal above. But if the order of the roles will change, their ids will changes and other tables which stores id of roles will be affected.
I'll check what I can do to avoid any conflicts

[abhinavk96]

@shreyanshdwivedi @iamareebjamal I don't think looking up for roles using their ID is ever a good idea, their name or slug is their distinguishing factor, shouldn't they be referred to using that, in which case this task can be merely reduced to adding the owner role in the populate script, and writing a migration which adds the role of owner, IF no other role with the name owner exists.

@shreyanshdwivedi Can you point me to where

Actually there are a few tables where id 1 is being used to associate event and role

I think they should be replaced with

role = Role.query.filter_by(name=ORGANIZER).first()

Like it has been used at some places in the app. (This example is from the events API).

[shreyanshdwivedi]

@CosmicCoder96 you can refer to users_events_roles table in db for instance where event_id, user_id and role_id are the columns.

[abhinavk96]

@shreyanshdwivedi For the legacy data, for each event, in the user events table the record which was inserted first (with an organizer relationship), denotes the creator of the event. You can leave the original data as is, create a migration to add owner role if it does not exist in roles table, and also add it to the end of populate db file. Next, add a migration to find the implicit creator of the event, (using the above logic), find the id of the entry in roles table whose name is 'OWNER' and insert into user_events_roles table the user_id, role_id(for the owner, whatever it may be ) and the event ID. What do you think?

[shreyanshdwivedi]

I totally agree to the steps you suggested. Just one thing, when we find out the owner of the event, I'll suggest to update the role_id of the entry in users_events_roles table rather than inserting a new one as it may lead to confusion later. If we update the role_id only, it'll be better. Does it sound correct?

[abhinavk96]

But an event owner is still an organiser yes ? Or no?

…

On Fri, 14 Jun 2019 at 2:52 AM, Shreyansh Dwivedi ***@***.***> wrote: I totally agree to the steps you suggested. Just one thing, when we find out the owner of the event, I'll suggest to update the role_id of the entry in users_events_roles table rather than inserting a new one as it may lead to confusion later. If we update the role_id only, it'll be better. Does it sound correct? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6042?email_source=notifications&email_token=AEDUDRIQTLGT2SYULWDHXKTP2KJO5A5CNFSM4HXWQNC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXUWH3Q#issuecomment-501834734>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEDUDROH6J4LKNOENLR77P3P2KJO5ANCNFSM4HXWQNCQ> .

[shreyanshdwivedi]

@CosmicCoder96 yes. An event owner will have all the rights of an organizer just like an organizer has all the rights of a coorganizer but the roles will still be different. Later we will provide more rights to the owner

[shreyanshdwivedi]

@uds5501 @iamareebjamal plz review

[iamareebjamal]

Change function name

[shreyanshdwivedi]

@iamareebjamal did the changes. Please review

[shreyanshdwivedi]

@CosmicCoder96 please review

[iamareebjamal]

Fix Travis Build

[iamareebjamal]

Most probably due to split migration heads, and the script did not fail correctly

[shreyanshdwivedi]

Yeah I'll have to update the migration to recent merged one
Error: Multiple heads are present; please specify a single target revision is in travis failure

[shreyanshdwivedi]

@CosmicCoder96 @iamareebjamal please review