fix: adds unique constraint on role_invite

[shreyanshdwivedi]

Fixes #5986

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:

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.

Changes proposed in this pull request:

• Raises error when duplicate role invite is created

[codecov]

Codecov Report

Merging #6000 into development will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           development    #6000   +/-   ##
============================================
  Coverage        66.28%   66.28%           
============================================
  Files              285      285           
  Lines            13995    13995           
============================================
  Hits              9277     9277           
  Misses            4718     4718

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b66321...94cf67a. Read the comment docs.

[iamareebjamal]

Use unique constraint

[shreyanshdwivedi]

@iamareebjamal you mean this -
__table_args__ = (UniqueConstraint('email', 'role_id', 'event_id',name='email_role_event_uc'), ) ?
I tested it locally but the postman didn't raise error. However will check again.
Maybe @uds5501 @kushthedude can test locally too?

[iamareebjamal]

You are checking uniqueness on 3 attributes, email, role and event_id whereas the Unique Constraint has 4 attributes, that's why it's not failing

[shreyanshdwivedi]

@iamareebjamal actually I followed the implementation from here -

open-event-server/app/models/custom_form.py

Line 71 in 3b66321

__table_args__ = (UniqueConstraint('event_id', 'field_identifier', 'form', name='custom_form_identifier'), )

Maybe this needs to be fixed too.

[iamareebjamal]

I'm sorry, I was wrong, the unique constraint is correct. You just haven't created the migration. How is the DB going to know about the constraint?

[uds5501]

This looks good!

[shreyanshdwivedi]

@iamareebjamal thanks a lot for help. I've fixed it.
Please review :)

[iamareebjamal]

Uhh, what?

[iamareebjamal]

Removing duplicates, right?

So are there any foreign keys in role_invites?

[shreyanshdwivedi]

I'm deleting all the past duplicate entries here and tested on my local too. When I didn't add this delete condition the upgradation raised error due to presence of duplicate entries. Is the delete statement wrong? @iamareebjamal

[shreyanshdwivedi]

Yes @iamareebjamal. event_id and role_id are foreignKeys

open-event-server/app/models/role_invite.py

Line 23 in 3b66321

event_id = db.Column(db.Integer, db.ForeignKey('events.id', ondelete='CASCADE'))

[iamareebjamal]

No, which table has role_invites as foreign key?

[iamareebjamal]

I'm deleting all the past duplicate entries here and tested on my local too

Problem is about data loss in production DB, if one of the deleted role invite is linked to other table

[shreyanshdwivedi]

@iamareebjamal I understand your concern. I've cross-checked and didn't find any model which uses role_invite as a foreign key.

[iamareebjamal]

OK, so reverse the condition R1.id > R2.id, we want to delete all but first

[shreyanshdwivedi]

@iamareebjamal changed the condition.