Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent more than one active mute/ban infraction per user #273

Open
SebastiaanZ opened this issue Oct 2, 2019 · 0 comments

Comments

@SebastiaanZ
Copy link
Member

commented Oct 2, 2019

Currently, the database accepts multiple active infractions of the same type for a single user. We're currently in the process of making sure the client will try to prevent this from happening, but it's a good idea to implement validation at the server side as well.

My idea is to:

  • Write a data migration that "cleans" the database of this kind of entries.
  • Add a UniqueTogetherValidator to prevent the API from accepting a secondary active infraction of the same type.

Ideally, this validator is applied to all infraction types, meaning that we should no longer accept infractions where active makes no sense to have active=True. This will require some changes at the client-side as well, though, so we need to discuss that first.

@SebastiaanZ SebastiaanZ self-assigned this Oct 2, 2019
SebastiaanZ added a commit that referenced this issue Oct 7, 2019
#273

This commit adds validation rules to the Infraction serializer that
validate if a given infraction should be accepted based on its status
of being considered `active`. If the validation fails, the API will
reject the request and return a 400 status.

Specifically, this validator checks that:

- infractions that can never be active do not have `active=True` set;

- a user can never receive a second active infraction of the same type.

Tests have been added to `test_infractions.py` to ensure that the
validators work as expected.

This commit implements the first part of #273
SebastiaanZ added a commit that referenced this issue Oct 7, 2019
#273

This commit adds a data migration to migrate active infractions that
should not be active to inactive. There are two types of infractions
that this migration will migrate to inactive:

- Infractions of types that should never be active (e.g. notes)

- Secondary active infractions if a given user already has an active
  infraction of the same type.

Since this makes the migration file fairly complex, I have written
tests to make sure the migration works as expected. In order to do
this, I've subclassed `django.test.TestCase` to create a
`MigrationsTestCase` that takes care of reverting the database back
to a state prior to the migrations we want to test and injects test
data before applying the migrations we want to test.

For more information, see `pydis_site.apps.api.tests.migrations.base`

This implements the last part of and closes #273
SebastiaanZ added a commit that referenced this issue Oct 7, 2019
#273

This commits adds a UniqueConstraint for active infractions on a
combination of the `user` and `type` field. This means that a user
can only have one active infraction of a given type in the database
at any time.

I've also added tests to make sure that this behaves as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.