Open-event-server: Multiple role invitations to the same email are allowed and are stored multiple times

Created on 3 Jun 2019  路  12Comments  路  Source: fossasia/open-event-server

Describe the bug
Currently, multiple role invitations can be sent to the same email id and even invitations with the same email and same role can be sent more than once and it is being stored multiple times in the list of roles.

Expected behavior
There can be an option to resend a pending invitation and that should not be stored as a duplicate entry.

needs-info

Most helpful comment

@uds5501 no, we are not sending re-invitations for now. Please refer to Expected Behavior of issue description, where @ShridharGoel mentioned that we can have such a feature.
I think it's better to handle it in separate PR because it'll involve frontend implementation too and label it as a feature. For now we can fix the duplicate entries ?
@uds5501 @ShridharGoel @kushthedude your thoughts?

All 12 comments

On it.

  • I've pushed changes which will raise error if a role-invite having a user with the same email and the same role is created/updated
  • IMO we should allow multiple role invitations which can be sent to the same email id (not with the same email and same role). What's ur opinion?
  • And for an option to resend a pending invitation and that should not be stored as a duplicate entry we need a different route. Bcoz in POST request, we cannot hold the creation of object without raising an error.
    @ShridharGoel @uds5501 @mrsaicharan1 @kushthedude please share your opinion.

@shreyanshdwivedi What is the problem with multiple roles associated with same email?

I don't see any problem with multiple roles with same email as I already mentioned in my previous comment.
The main problem which I can fetch from issue is the creation of duplicate entries in db. And I've created a PR which solves this issue.
@kushthedude do you find any other issue which can be incorporated in this PR?

  • I've pushed changes which will raise error if a role-invite having a user with the same email and the same role is created/updated
  • IMO we should allow multiple role invitations which can be sent to the same email id (not with the same email and same role). What's ur opinion?

I don't see any problem with that, of course a particular email can have multiple role invites

  • And for an option to resend a pending invitation and that should not be stored as a duplicate entry we need a different route. Bcoz in POST request, we cannot hold the creation of object without raising an error.

@shreyanshdwivedi wait, we are re-sending the invitations? I didn't come across this implementation. In case we are doing this, you are right, POST won't make sense, not at least with similar data.

@uds5501 no, we are not sending re-invitations for now. Please refer to Expected Behavior of issue description, where @ShridharGoel mentioned that we can have such a feature.
I think it's better to handle it in separate PR because it'll involve frontend implementation too and label it as a feature. For now we can fix the duplicate entries ?
@uds5501 @ShridharGoel @kushthedude your thoughts?

@uds5501 no, we are not sending re-invitations for now. Please refer to Expected Behavior of issue description, where @ShridharGoel mentioned that we can have such a feature.
I think it's better to handle it in separate PR because it'll involve frontend implementation too and label it as a feature. For now we can fix the duplicate entries ?
@uds5501 @ShridharGoel @kushthedude your thoughts?

This makes sense

  • I've pushed changes which will raise error if a role-invite having a user with the same email and the same role is created/updated
  • IMO we should allow multiple role invitations which can be sent to the same email id (not with the same email and same role). What's ur opinion?

I don't see any problem with that, of course a particular email can have multiple role invites

  • And for an option to resend a pending invitation and that should not be stored as a duplicate entry we need a different route. Bcoz in POST request, we cannot hold the creation of object without raising an error.

@shreyanshdwivedi wait, we are re-sending the invitations? I didn't come across this implementation. In case we are doing this, you are right, POST won't make sense, not at least with similar data.

Feature to resend a pending invitation would be required after we restrict sending invitations with the same email and same role because the old invitation link becomes invalid after sometime.
(I think after 24 hours: https://github.com/fossasia/open-event-server/issues/1234).

  • IMO we should allow multiple role invitations which can be sent to the same email id (not with the same email and same role). What's ur opinion?

Is the system configured to allow multiple roles, like if a role has a permission set to true and another has it as false, would it consider it as true if the organizer has both the roles?

Also, it there any need of multiple roles? A co-organizer having the role of a moderator also would not be required. But maybe the case is different if we consider track_organizer and registrar, I'm not sure what permissions they're having.

Feature to resend a pending invitation would be required after we restrict sending invitations with the same email and same role because the old invitation link becomes invalid after sometime.

I think that feature to resend invitation will be good to have but can you please mention the line where this job is scheduled that makes old invitation invalid? @ShridharGoel

But maybe the case is different if we consider track_organizer and registrar, I'm not sure what permissions they're having.

I also thought of similar example that's why I mentioned not to change multiple role invitation

I think that feature to resend invitation will be good to have but can you please mention the line where this job is scheduled that makes old invitation invalid?

https://github.com/fossasia/open-event-server/blob/1c1c46f899ac8d916ff7c5204a8001ad52515a71/open_event/views/admin/models_views/events.py#L296-L302

For now we can fix the duplicate entries

Add unique constraint

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mariobehling picture mariobehling  路  4Comments

rafalkowalski picture rafalkowalski  路  3Comments

poush picture poush  路  3Comments

SaptakS picture SaptakS  路  3Comments

mariobehling picture mariobehling  路  4Comments