aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGravatar Matteo Bertucci <[email protected]>2019-11-15 10:11:45 +0100
committerGravatar GitHub <[email protected]>2019-11-15 10:11:45 +0100
commit0e8f623d15f043f1c8712366bdaf83153a619bca (patch)
tree611c4f111855788c00a5e3ef6c909d9021b29c5c
parentDelete useless migration file (diff)
parentAdd pipenv run start command (#308) (diff)
Merge branch 'master' into bot#549-show-attachments-staff
-rw-r--r--Pipfile1
-rw-r--r--pydis_site/__init__.py5
-rw-r--r--pydis_site/apps/api/migrations/0047_active_infractions_migration.py105
-rw-r--r--pydis_site/apps/api/migrations/0048_add_infractions_unique_constraints_active.py17
-rw-r--r--pydis_site/apps/api/models/bot/infraction.py7
-rw-r--r--pydis_site/apps/api/models/bot/user.py2
-rw-r--r--pydis_site/apps/api/serializers.py13
-rw-r--r--pydis_site/apps/api/tests/migrations/__init__.py1
-rw-r--r--pydis_site/apps/api/tests/migrations/base.py102
-rw-r--r--pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py496
-rw-r--r--pydis_site/apps/api/tests/migrations/test_base.py135
-rw-r--r--pydis_site/apps/api/tests/test_infractions.py195
-rw-r--r--pydis_site/apps/home/forms/__init__.py0
-rw-r--r--pydis_site/apps/home/forms/account_deletion.py24
-rw-r--r--pydis_site/apps/home/signals.py84
-rw-r--r--pydis_site/apps/home/tests/test_signal_listener.py70
-rw-r--r--pydis_site/apps/home/tests/test_views.py197
-rw-r--r--pydis_site/apps/home/urls.py11
-rw-r--r--pydis_site/apps/home/views/__init__.py3
-rw-r--r--pydis_site/apps/home/views/account/__init__.py4
-rw-r--r--pydis_site/apps/home/views/account/delete.py37
-rw-r--r--pydis_site/apps/home/views/account/settings.py59
-rw-r--r--pydis_site/apps/staff/migrations/0002_add_is_staff_to_role_mappings.py18
-rw-r--r--pydis_site/apps/staff/models/role_mapping.py5
-rw-r--r--pydis_site/settings.py25
-rw-r--r--pydis_site/static/css/base/base.css20
-rw-r--r--pydis_site/static/js/base/modal.js100
-rw-r--r--pydis_site/templates/base/base.html1
-rw-r--r--pydis_site/templates/base/navbar.html31
-rw-r--r--pydis_site/templates/home/account/delete.html44
-rw-r--r--pydis_site/templates/home/account/settings.html137
-rw-r--r--pydis_site/tests/test_utils_account.py132
-rw-r--r--pydis_site/utils/account.py79
33 files changed, 2122 insertions, 38 deletions
diff --git a/Pipfile b/Pipfile
index f136a328..cb5776e0 100644
--- a/Pipfile
+++ b/Pipfile
@@ -43,6 +43,7 @@ unittest-xml-reporting = "~=2.5.1"
python_version = "3.7"
[scripts]
+start = "python manage.py run --debug"
makemigrations = "python manage.py makemigrations"
django_shell = "python manage.py shell"
test = "coverage run manage.py test"
diff --git a/pydis_site/__init__.py b/pydis_site/__init__.py
index c6146450..df67cf71 100644
--- a/pydis_site/__init__.py
+++ b/pydis_site/__init__.py
@@ -2,3 +2,8 @@ from wiki.plugins.macros.mdx import toc
# Remove the toc header prefix. There's no option for this, so we gotta monkey patch it.
toc.HEADER_ID_PREFIX = ''
+
+# Empty list of validators for Allauth to ponder over. This is referred to in settings.py
+# by a string because Allauth won't let us just give it a list _there_, we have to point
+# at a list _somewhere else_ instead.
+VALIDATORS = []
diff --git a/pydis_site/apps/api/migrations/0047_active_infractions_migration.py b/pydis_site/apps/api/migrations/0047_active_infractions_migration.py
new file mode 100644
index 00000000..9ac791dc
--- /dev/null
+++ b/pydis_site/apps/api/migrations/0047_active_infractions_migration.py
@@ -0,0 +1,105 @@
+# Generated by Django 2.2.6 on 2019-10-07 15:59
+
+from django.db import migrations
+from django.db.models import Count, Prefetch, QuerySet
+
+
+class ExpirationWrapper:
+ """Wraps an expiration date to properly compare permanent and temporary infractions."""
+
+ def __init__(self, infraction):
+ self.expiration_date = infraction.expires_at
+
+ def __lt__(self, other):
+ """An `expiration_date` is considered smaller when it comes earlier than the `other`."""
+ if self.expiration_date is None:
+ # A permanent infraction can never end sooner than another infraction
+ return False
+ elif other.expiration_date is None:
+ # If `self` is temporary, but `other` is permanent, `self` is smaller
+ return True
+ else:
+ return self.expiration_date < other.expiration_date
+
+ def __eq__(self, other):
+ """If both expiration dates are permanent they're equal, otherwise compare dates."""
+ if self.expiration_date is None and other.expiration_date is None:
+ return True
+ elif self.expiration_date is None or other.expiration_date is None:
+ return False
+ else:
+ return self.expiration_date == other.expiration_date
+
+
+def migrate_inactive_types_to_inactive(apps, schema_editor):
+ """Migrates infractions of non-active types to inactive."""
+ infraction_model = apps.get_model('api', 'Infraction')
+ infraction_model.objects.filter(type__in=('note', 'warning', 'kick')).update(active=False)
+
+
+def get_query(user_model, infraction_model, infr_type: str) -> QuerySet:
+ """
+ Creates QuerySet to fetch users with multiple active infractions of the given `type`.
+
+ The QuerySet will prefetch the infractions and attach them as an `.infractions` attribute to the
+ `User` instances.
+ """
+ active_infractions = infraction_model.objects.filter(type=infr_type, active=True)
+
+ # Build an SQL query by chaining methods together
+
+ # Get users with active infraction(s) of the provided `infr_type`
+ query = user_model.objects.filter(
+ infractions_received__type=infr_type, infractions_received__active=True
+ )
+
+ # Prefetch their active received infractions of `infr_type` and attach `.infractions` attribute
+ query = query.prefetch_related(
+ Prefetch('infractions_received', queryset=active_infractions, to_attr='infractions')
+ )
+
+ # Count and only include them if they have at least 2 active infractions of the `type`
+ query = query.annotate(num_infractions=Count('infractions_received'))
+ query = query.filter(num_infractions__gte=2)
+
+ # Make sure we return each individual only once
+ query = query.distinct()
+
+ return query
+
+
+def migrate_multiple_active_infractions_per_user_to_one(apps, schema_editor):
+ """
+ Make sure a user only has one active infraction of a given "active" infraction type.
+
+ If a user has multiple active infraction, we keep the one with longest expiration date active
+ and migrate the others to inactive.
+ """
+ infraction_model = apps.get_model('api', 'Infraction')
+ user_model = apps.get_model('api', 'User')
+
+ for infraction_type in ('ban', 'mute', 'superstar', 'watch'):
+ query = get_query(user_model, infraction_model, infraction_type)
+ for user in query:
+ infractions = sorted(user.infractions, key=ExpirationWrapper, reverse=True)
+ for infraction in infractions[1:]:
+ infraction.active = False
+ infraction.save()
+
+
+def reverse_migration(apps, schema_editor):
+ """There's no need to do anything special to reverse these migrations."""
+ return
+
+
+class Migration(migrations.Migration):
+ """Data migration to get the database consistent with the new infraction validation rules."""
+
+ dependencies = [
+ ('api', '0046_reminder_jump_url'),
+ ]
+
+ operations = [
+ migrations.RunPython(migrate_inactive_types_to_inactive, reverse_migration),
+ migrations.RunPython(migrate_multiple_active_infractions_per_user_to_one, reverse_migration)
+ ]
diff --git a/pydis_site/apps/api/migrations/0048_add_infractions_unique_constraints_active.py b/pydis_site/apps/api/migrations/0048_add_infractions_unique_constraints_active.py
new file mode 100644
index 00000000..4ea1fb90
--- /dev/null
+++ b/pydis_site/apps/api/migrations/0048_add_infractions_unique_constraints_active.py
@@ -0,0 +1,17 @@
+# Generated by Django 2.2.6 on 2019-10-07 18:27
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('api', '0047_active_infractions_migration'),
+ ]
+
+ operations = [
+ migrations.AddConstraint(
+ model_name='infraction',
+ constraint=models.UniqueConstraint(condition=models.Q(active=True), fields=('user', 'type'), name='unique_active_infraction_per_type_per_user'),
+ ),
+ ]
diff --git a/pydis_site/apps/api/models/bot/infraction.py b/pydis_site/apps/api/models/bot/infraction.py
index dfb32a97..108fd3a2 100644
--- a/pydis_site/apps/api/models/bot/infraction.py
+++ b/pydis_site/apps/api/models/bot/infraction.py
@@ -71,3 +71,10 @@ class Infraction(ModelReprMixin, models.Model):
"""Defines the meta options for the infraction model."""
ordering = ['-inserted_at']
+ constraints = (
+ models.UniqueConstraint(
+ fields=["user", "type"],
+ condition=models.Q(active=True),
+ name="unique_active_infraction_per_type_per_user"
+ ),
+ )
diff --git a/pydis_site/apps/api/models/bot/user.py b/pydis_site/apps/api/models/bot/user.py
index 21617dc4..5140d2bf 100644
--- a/pydis_site/apps/api/models/bot/user.py
+++ b/pydis_site/apps/api/models/bot/user.py
@@ -50,7 +50,7 @@ class User(ModelReprMixin, models.Model):
def __str__(self):
"""Returns the name and discriminator for the current user, for display purposes."""
- return f"{self.name}#{self.discriminator}"
+ return f"{self.name}#{self.discriminator:0>4}"
@property
def top_role(self) -> Role:
diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py
index e091ff4f..2af04e0e 100644
--- a/pydis_site/apps/api/serializers.py
+++ b/pydis_site/apps/api/serializers.py
@@ -1,6 +1,6 @@
"""Converters from Django models to data interchange formats and back."""
-
from rest_framework.serializers import ModelSerializer, PrimaryKeyRelatedField, ValidationError
+from rest_framework.validators import UniqueTogetherValidator
from rest_framework_bulk import BulkSerializerMixin
from .models import (
@@ -106,11 +106,22 @@ class InfractionSerializer(ModelSerializer):
fields = (
'id', 'inserted_at', 'expires_at', 'active', 'user', 'actor', 'type', 'reason', 'hidden'
)
+ validators = [
+ UniqueTogetherValidator(
+ queryset=Infraction.objects.filter(active=True),
+ fields=['user', 'type'],
+ message='This user already has an active infraction of this type.',
+ )
+ ]
def validate(self, attrs: dict) -> dict:
"""Validate data constraints for the given data and abort if it is invalid."""
infr_type = attrs.get('type')
+ active = attrs.get('active')
+ if active and infr_type in ('note', 'warning', 'kick'):
+ raise ValidationError({'active': [f'{infr_type} infractions cannot be active.']})
+
expires_at = attrs.get('expires_at')
if expires_at and infr_type in ('kick', 'warning'):
raise ValidationError({'expires_at': [f'{infr_type} infractions cannot expire.']})
diff --git a/pydis_site/apps/api/tests/migrations/__init__.py b/pydis_site/apps/api/tests/migrations/__init__.py
new file mode 100644
index 00000000..38e42ffc
--- /dev/null
+++ b/pydis_site/apps/api/tests/migrations/__init__.py
@@ -0,0 +1 @@
+"""This submodule contains tests for functions used in data migrations."""
diff --git a/pydis_site/apps/api/tests/migrations/base.py b/pydis_site/apps/api/tests/migrations/base.py
new file mode 100644
index 00000000..0c0a5bd0
--- /dev/null
+++ b/pydis_site/apps/api/tests/migrations/base.py
@@ -0,0 +1,102 @@
+"""Includes utilities for testing migrations."""
+from django.db import connection
+from django.db.migrations.executor import MigrationExecutor
+from django.test import TestCase
+
+
+class MigrationsTestCase(TestCase):
+ """
+ A `TestCase` subclass to test migration files.
+
+ To be able to properly test a migration, we will need to inject data into the test database
+ before the migrations we want to test are applied, but after the older migrations have been
+ applied. This makes sure that we are testing "as if" we were actually applying this migration
+ to a database in the state it was in before introducing the new migration.
+
+ To set up a MigrationsTestCase, create a subclass of this class and set the following
+ class-level attributes:
+
+ - app: The name of the app that contains the migrations (e.g., `'api'`)
+ - migration_prior: The name* of the last migration file before the migrations you want to test
+ - migration_target: The name* of the last migration file we want to test
+
+ *) Specify the file names without a path or the `.py` file extension.
+
+ Additionally, overwrite the `setUpMigrationData` in the subclass to inject data into the
+ database before the migrations we want to test are applied. Please read the docstring of the
+ method for more information. An optional hook, `setUpPostMigrationData` is also provided.
+ """
+
+ # These class-level attributes should be set in classes that inherit from this base class.
+ app = None
+ migration_prior = None
+ migration_target = None
+
+ @classmethod
+ def setUpTestData(cls):
+ """
+ Injects data into the test database prior to the migration we're trying to test.
+
+ This class methods reverts the test database back to the state of the last migration file
+ prior to the migrations we want to test. It will then allow the user to inject data into the
+ test database by calling the `setUpMigrationData` hook. After the data has been injected, it
+ will apply the migrations we want to test and call the `setUpPostMigrationData` hook. The
+ user can now test if the migration correctly migrated the injected test data.
+ """
+ if not cls.app:
+ raise ValueError("The `app` attribute was not set.")
+
+ if not cls.migration_prior or not cls.migration_target:
+ raise ValueError("Both ` migration_prior` and `migration_target` need to be set.")
+
+ cls.migrate_from = [(cls.app, cls.migration_prior)]
+ cls.migrate_to = [(cls.app, cls.migration_target)]
+
+ # Reverse to database state prior to the migrations we want to test
+ executor = MigrationExecutor(connection)
+ executor.migrate(cls.migrate_from)
+
+ # Call the data injection hook with the current state of the project
+ old_apps = executor.loader.project_state(cls.migrate_from).apps
+ cls.setUpMigrationData(old_apps)
+
+ # Run the migrations we want to test
+ executor = MigrationExecutor(connection)
+ executor.loader.build_graph()
+ executor.migrate(cls.migrate_to)
+
+ # Save the project state so we're able to work with the correct model states
+ cls.apps = executor.loader.project_state(cls.migrate_to).apps
+
+ # Call `setUpPostMigrationData` to potentially set up post migration data used in testing
+ cls.setUpPostMigrationData(cls.apps)
+
+ @classmethod
+ def setUpMigrationData(cls, apps):
+ """
+ Override this method to inject data into the test database before the migration is applied.
+
+ This method will be called after setting up the database according to the migrations that
+ come before the migration(s) we are trying to test, but before the to-be-tested migration(s)
+ are applied. This allows us to simulate a database state just prior to the migrations we are
+ trying to test.
+
+ To make sure we're creating objects according to the state the models were in at this point
+ in the migration history, use `apps.get_model(app_name: str, model_name: str)` to get the
+ appropriate model, e.g.:
+
+ >>> Infraction = apps.get_model('api', 'Infraction')
+ """
+ pass
+
+ @classmethod
+ def setUpPostMigrationData(cls, apps):
+ """
+ Set up additional test data after the target migration has been applied.
+
+ Use `apps.get_model(app_name: str, model_name: str)` to get the correct instances of the
+ model classes:
+
+ >>> Infraction = apps.get_model('api', 'Infraction')
+ """
+ pass
diff --git a/pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py b/pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py
new file mode 100644
index 00000000..8dc29b34
--- /dev/null
+++ b/pydis_site/apps/api/tests/migrations/test_active_infraction_migration.py
@@ -0,0 +1,496 @@
+"""Tests for the data migration in `filename`."""
+import logging
+from collections import ChainMap, namedtuple
+from datetime import timedelta
+from itertools import count
+from typing import Dict, Iterable, Type, Union
+
+from django.db.models import Q
+from django.forms.models import model_to_dict
+from django.utils import timezone
+
+from pydis_site.apps.api.models import Infraction, User
+from .base import MigrationsTestCase
+
+log = logging.getLogger(__name__)
+log.setLevel(logging.DEBUG)
+
+
+InfractionHistory = namedtuple('InfractionHistory', ("user_id", "infraction_history"))
+
+
+class InfractionFactory:
+ """Factory that creates infractions for a User instance."""
+
+ infraction_id = count(1)
+ user_id = count(1)
+ default_values = {
+ 'active': True,
+ 'expires_at': None,
+ 'hidden': False,
+ }
+
+ @classmethod
+ def create(
+ cls,
+ actor: User,
+ infractions: Iterable[Dict[str, Union[str, int, bool]]],
+ infraction_model: Type[Infraction] = Infraction,
+ user_model: Type[User] = User,
+ ) -> InfractionHistory:
+ """
+ Creates `infractions` for the `user` with the given `actor`.
+
+ The `infractions` dictionary can contain the following fields:
+ - `type` (required)
+ - `active` (default: True)
+ - `expires_at` (default: None; i.e, permanent)
+ - `hidden` (default: False).
+
+ The parameters `infraction_model` and `user_model` can be used to pass in an instance of
+ both model classes from a different migration/project state.
+ """
+ user_id = next(cls.user_id)
+ user = user_model.objects.create(
+ id=user_id,
+ name=f"Infracted user {user_id}",
+ discriminator=user_id,
+ avatar_hash=None,
+ )
+ infraction_history = []
+
+ for infraction in infractions:
+ infraction = dict(infraction)
+ infraction["id"] = next(cls.infraction_id)
+ infraction = ChainMap(infraction, cls.default_values)
+ new_infraction = infraction_model.objects.create(
+ user=user,
+ actor=actor,
+ type=infraction["type"],
+ reason=f"`{infraction['type']}` infraction (ID: {infraction['id']} of {user}",
+ active=infraction['active'],
+ hidden=infraction['hidden'],
+ expires_at=infraction['expires_at'],
+ )
+ infraction_history.append(new_infraction)
+
+ return InfractionHistory(user_id=user_id, infraction_history=infraction_history)
+
+
+class InfractionFactoryTests(MigrationsTestCase):
+ """Tests for the InfractionFactory."""
+
+ app = "api"
+ migration_prior = "0046_reminder_jump_url"
+ migration_target = "0046_reminder_jump_url"
+
+ @classmethod
+ def setUpPostMigrationData(cls, apps):
+ """Create a default actor for all infractions."""
+ cls.infraction_model = apps.get_model('api', 'Infraction')
+ cls.user_model = apps.get_model('api', 'User')
+
+ cls.actor = cls.user_model.objects.create(
+ id=9999,
+ name="Unknown Moderator",
+ discriminator=1040,
+ avatar_hash=None,
+ )
+
+ def test_infraction_factory_total_count(self):
+ """Does the test database hold as many infractions as we tried to create?"""
+ InfractionFactory.create(
+ actor=self.actor,
+ infractions=(
+ {'type': 'kick', 'active': False, 'hidden': False},
+ {'type': 'ban', 'active': True, 'hidden': False},
+ {'type': 'note', 'active': False, 'hidden': True},
+ ),
+ infraction_model=self.infraction_model,
+ user_model=self.user_model,
+ )
+ database_count = Infraction.objects.all().count()
+ self.assertEqual(3, database_count)
+
+ def test_infraction_factory_multiple_users(self):
+ """Does the test database hold as many infractions as we tried to create?"""
+ for _user in range(5):
+ InfractionFactory.create(
+ actor=self.actor,
+ infractions=(
+ {'type': 'kick', 'active': False, 'hidden': True},
+ {'type': 'ban', 'active': True, 'hidden': False},
+ ),
+ infraction_model=self.infraction_model,
+ user_model=self.user_model,
+ )
+
+ # Check if infractions and users are recorded properly in the database
+ database_count = Infraction.objects.all().count()
+ self.assertEqual(database_count, 10)
+
+ user_count = User.objects.all().count()
+ self.assertEqual(user_count, 5 + 1)
+
+ def test_infraction_factory_sets_correct_fields(self):
+ """Does the InfractionFactory set the correct attributes?"""
+ infractions = (
+ {
+ 'type': 'note',
+ 'active': False,
+ 'hidden': True,
+ 'expires_at': timezone.now()
+ },
+ {'type': 'warning', 'active': False, 'hidden': False, 'expires_at': None},
+ {'type': 'watch', 'active': False, 'hidden': True, 'expires_at': None},
+ {'type': 'mute', 'active': True, 'hidden': False, 'expires_at': None},
+ {'type': 'kick', 'active': True, 'hidden': True, 'expires_at': None},
+ {'type': 'ban', 'active': True, 'hidden': False, 'expires_at': None},
+ {
+ 'type': 'superstar',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now()
+ },
+ )
+
+ InfractionFactory.create(
+ actor=self.actor,
+ infractions=infractions,
+ infraction_model=self.infraction_model,
+ user_model=self.user_model,
+ )
+
+ for infraction in infractions:
+ with self.subTest(**infraction):
+ self.assertTrue(Infraction.objects.filter(**infraction).exists())
+
+
+class ActiveInfractionMigrationTests(MigrationsTestCase):
+ """
+ Tests the active infraction data migration.
+
+ The active infraction data migration should do the following things:
+
+ 1. migrates all active notes, warnings, and kicks to an inactive status;
+ 2. migrates all users with multiple active infractions of a single type to have only one active
+ infraction of that type. The infraction with the longest duration stays active.
+ """
+
+ app = "api"
+ migration_prior = "0046_reminder_jump_url"
+ migration_target = "0047_active_infractions_migration"
+
+ @classmethod
+ def setUpMigrationData(cls, apps):
+ """Sets up an initial database state that contains the relevant test cases."""
+ # Fetch the Infraction and User model in the current migration state
+ cls.infraction_model = apps.get_model('api', 'Infraction')
+ cls.user_model = apps.get_model('api', 'User')
+
+ cls.created_infractions = {}
+
+ # Moderator that serves as actor for all infractions
+ cls.user_moderator = cls.user_model.objects.create(
+ id=9999,
+ name="Olivier de Vienne",
+ discriminator=1040,
+ avatar_hash=None,
+ )
+
+ # User #1: clean user with no infractions
+ cls.created_infractions["no infractions"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=[],
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #2: One inactive note infraction
+ cls.created_infractions["one inactive note"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'note', 'active': False, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #3: One active note infraction
+ cls.created_infractions["one active note"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'note', 'active': True, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #4: One active and one inactive note infraction
+ cls.created_infractions["one active and one inactive note"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'note', 'active': False, 'hidden': True},
+ {'type': 'note', 'active': True, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #5: Once active note, one active kick, once active warning
+ cls.created_infractions["active note, kick, warning"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'note', 'active': True, 'hidden': True},
+ {'type': 'kick', 'active': True, 'hidden': True},
+ {'type': 'warning', 'active': True, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #6: One inactive ban and one active ban
+ cls.created_infractions["one inactive and one active ban"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'ban', 'active': False, 'hidden': True},
+ {'type': 'ban', 'active': True, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #7: Two active permanent bans
+ cls.created_infractions["two active perm bans"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'ban', 'active': True, 'hidden': True},
+ {'type': 'ban', 'active': True, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #8: Multiple active temporary bans
+ cls.created_infractions["multiple active temp bans"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=1)
+ },
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=10)
+ },
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=20)
+ },
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=5)
+ },
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #9: One active permanent ban, two active temporary bans
+ cls.created_infractions["active perm, two active temp bans"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=10)
+ },
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': None,
+ },
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=7)
+ },
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #10: One inactive permanent ban, two active temporary bans
+ cls.created_infractions["one inactive perm ban, two active temp bans"] = (
+ InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=10)
+ },
+ {
+ 'type': 'ban',
+ 'active': False,
+ 'hidden': True,
+ 'expires_at': None,
+ },
+ {
+ 'type': 'ban',
+ 'active': True,
+ 'hidden': True,
+ 'expires_at': timezone.now() + timedelta(days=7)
+ },
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+ )
+
+ # User #11: Active ban, active mute, active superstar
+ cls.created_infractions["active ban, mute, and superstar"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'ban', 'active': True, 'hidden': True},
+ {'type': 'mute', 'active': True, 'hidden': True},
+ {'type': 'superstar', 'active': True, 'hidden': True},
+ {'type': 'watch', 'active': True, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ # User #12: Multiple active bans, active mutes, active superstars
+ cls.created_infractions["multiple active bans, mutes, stars"] = InfractionFactory.create(
+ actor=cls.user_moderator,
+ infractions=(
+ {'type': 'ban', 'active': True, 'hidden': True},
+ {'type': 'ban', 'active': True, 'hidden': True},
+ {'type': 'ban', 'active': True, 'hidden': True},
+ {'type': 'mute', 'active': True, 'hidden': True},
+ {'type': 'mute', 'active': True, 'hidden': True},
+ {'type': 'mute', 'active': True, 'hidden': True},
+ {'type': 'superstar', 'active': True, 'hidden': True},
+ {'type': 'superstar', 'active': True, 'hidden': True},
+ {'type': 'superstar', 'active': True, 'hidden': True},
+ {'type': 'watch', 'active': True, 'hidden': True},
+ {'type': 'watch', 'active': True, 'hidden': True},
+ {'type': 'watch', 'active': True, 'hidden': True},
+ ),
+ infraction_model=cls.infraction_model,
+ user_model=cls.user_model,
+ )
+
+ def test_all_never_active_types_became_inactive(self):
+ """Are all infractions of a non-active type inactive after the migration?"""
+ inactive_type_query = Q(type="note") | Q(type="warning") | Q(type="kick")
+ self.assertFalse(
+ self.infraction_model.objects.filter(inactive_type_query, active=True).exists()
+ )
+
+ def test_migration_left_clean_user_without_infractions(self):
+ """Do users without infractions have no infractions after the migration?"""
+ user_id, infraction_history = self.created_infractions["no infractions"]
+ self.assertFalse(
+ self.infraction_model.objects.filter(user__id=user_id).exists()
+ )
+
+ def test_migration_left_user_with_inactive_note_untouched(self):
+ """Did the migration leave users with only an inactive note untouched?"""
+ user_id, infraction_history = self.created_infractions["one inactive note"]
+ inactive_note = infraction_history[0]
+ self.assertTrue(
+ self.infraction_model.objects.filter(**model_to_dict(inactive_note)).exists()
+ )
+
+ def test_migration_only_touched_active_field_of_active_note(self):
+ """Does the migration only change the `active` field?"""
+ user_id, infraction_history = self.created_infractions["one active note"]
+ note = model_to_dict(infraction_history[0])
+ note['active'] = False
+ self.assertTrue(
+ self.infraction_model.objects.filter(**note).exists()
+ )
+
+ def test_migration_only_touched_active_field_of_active_note_left_inactive_untouched(self):
+ """Does the migration only change the `active` field of active notes?"""
+ user_id, infraction_history = self.created_infractions["one active and one inactive note"]
+ for note in infraction_history:
+ with self.subTest(active=note.active):
+ note = model_to_dict(note)
+ note['active'] = False
+ self.assertTrue(
+ self.infraction_model.objects.filter(**note).exists()
+ )
+
+ def test_migration_migrates_all_nonactive_types_to_inactive(self):
+ """Do we set the `active` field of all non-active infractions to `False`?"""
+ user_id, infraction_history = self.created_infractions["active note, kick, warning"]
+ self.assertFalse(
+ self.infraction_model.objects.filter(user__id=user_id, active=True).exists()
+ )
+
+ def test_migration_leaves_user_with_one_active_ban_untouched(self):
+ """Do we leave a user with one active and one inactive ban untouched?"""
+ user_id, infraction_history = self.created_infractions["one inactive and one active ban"]
+ for infraction in infraction_history:
+ with self.subTest(active=infraction.active):
+ self.assertTrue(
+ self.infraction_model.objects.filter(**model_to_dict(infraction)).exists()
+ )
+
+ def test_migration_turns_double_active_perm_ban_into_single_active_perm_ban(self):
+ """Does the migration turn two active permanent bans into one active permanent ban?"""
+ user_id, infraction_history = self.created_infractions["two active perm bans"]
+ active_count = self.infraction_model.objects.filter(user__id=user_id, active=True).count()
+ self.assertEqual(active_count, 1)
+
+ def test_migration_leaves_temporary_ban_with_longest_duration_active(self):
+ """Does the migration turn two active permanent bans into one active permanent ban?"""
+ user_id, infraction_history = self.created_infractions["multiple active temp bans"]
+ active_ban = self.infraction_model.objects.get(user__id=user_id, active=True)
+ self.assertEqual(active_ban.expires_at, infraction_history[2].expires_at)
+
+ def test_migration_leaves_permanent_ban_active(self):
+ """Does the migration leave the permanent ban active?"""
+ user_id, infraction_history = self.created_infractions["active perm, two active temp bans"]
+ active_ban = self.infraction_model.objects.get(user__id=user_id, active=True)
+ self.assertIsNone(active_ban.expires_at)
+
+ def test_migration_leaves_longest_temp_ban_active_with_inactive_permanent_ban(self):
+ """Does the longest temp ban stay active, even with an inactive perm ban present?"""
+ user_id, infraction_history = self.created_infractions[
+ "one inactive perm ban, two active temp bans"
+ ]
+ active_ban = self.infraction_model.objects.get(user__id=user_id, active=True)
+ self.assertEqual(active_ban.expires_at, infraction_history[0].expires_at)
+
+ def test_migration_leaves_all_active_types_active_if_one_of_each_exists(self):
+ """Do all active infractions stay active if only one of each is present?"""
+ user_id, infraction_history = self.created_infractions["active ban, mute, and superstar"]
+ active_count = self.infraction_model.objects.filter(user__id=user_id, active=True).count()
+ self.assertEqual(active_count, 4)
+
+ def test_migration_reduces_all_active_types_to_a_single_active_infraction(self):
+ """Do we reduce all of the infraction types to one active infraction?"""
+ user_id, infraction_history = self.created_infractions["multiple active bans, mutes, stars"]
+ active_infractions = self.infraction_model.objects.filter(user__id=user_id, active=True)
+ self.assertEqual(len(active_infractions), 4)
+ types_observed = [infraction.type for infraction in active_infractions]
+
+ for infraction_type in ('ban', 'mute', 'superstar', 'watch'):
+ with self.subTest(type=infraction_type):
+ self.assertIn(infraction_type, types_observed)
diff --git a/pydis_site/apps/api/tests/migrations/test_base.py b/pydis_site/apps/api/tests/migrations/test_base.py
new file mode 100644
index 00000000..f69bc92c
--- /dev/null
+++ b/pydis_site/apps/api/tests/migrations/test_base.py
@@ -0,0 +1,135 @@
+import logging
+from unittest.mock import call, patch
+
+from django.db.migrations.loader import MigrationLoader
+from django.test import TestCase
+
+from .base import MigrationsTestCase, connection
+
+log = logging.getLogger(__name__)
+
+
+class SpanishInquisition(MigrationsTestCase):
+ app = "api"
+ migration_prior = "scragly"
+ migration_target = "kosa"
+
+
+@patch("pydis_site.apps.api.tests.migrations.base.MigrationExecutor")
+class MigrationsTestCaseNoSideEffectsTests(TestCase):
+ """Tests the MigrationTestCase class with actual migration side effects disabled."""
+
+ def setUp(self):
+ """Set up an instance of MigrationsTestCase for use in tests."""
+ self.test_case = SpanishInquisition()
+
+ def test_missing_app_class_raises_value_error(self, _migration_executor):
+ """A MigrationsTestCase subclass should set the class-attribute `app`."""
+ class Spam(MigrationsTestCase):
+ pass
+
+ spam = Spam()
+ with self.assertRaises(ValueError, msg="The `app` attribute was not set."):
+ spam.setUpTestData()
+
+ def test_missing_migration_class_attributes_raise_value_error(self, _migration_executor):
+ """A MigrationsTestCase subclass should set both `migration_prior` and `migration_target`"""
+ class Eggs(MigrationsTestCase):
+ app = "api"
+ migration_target = "lemon"
+
+ class Bacon(MigrationsTestCase):
+ app = "api"
+ migration_prior = "mark"
+
+ instances = (Eggs(), Bacon())
+
+ exception_message = "Both ` migration_prior` and `migration_target` need to be set."
+ for instance in instances:
+ with self.subTest(
+ migration_prior=instance.migration_prior,
+ migration_target=instance.migration_target,
+ ):
+ with self.assertRaises(ValueError, msg=exception_message):
+ instance.setUpTestData()
+
+ @patch(f"{__name__}.SpanishInquisition.setUpMigrationData")
+ @patch(f"{__name__}.SpanishInquisition.setUpPostMigrationData")
+ def test_migration_data_hooks_are_called_once(self, pre_hook, post_hook, _migration_executor):
+ """The `setUpMigrationData` and `setUpPostMigrationData` hooks should be called once."""
+ self.test_case.setUpTestData()
+ for hook in (pre_hook, post_hook):
+ with self.subTest(hook=repr(hook)):
+ hook.assert_called_once()
+
+ def test_migration_executor_is_instantiated_twice(self, migration_executor):
+ """The `MigrationExecutor` should be instantiated with the database connection twice."""
+ self.test_case.setUpTestData()
+
+ expected_args = [call(connection), call(connection)]
+ self.assertEqual(migration_executor.call_args_list, expected_args)
+
+ def test_project_state_is_loaded_for_correct_migration_files_twice(self, migration_executor):
+ """The `project_state` should first be loaded with `migrate_from`, then `migrate_to`."""
+ self.test_case.setUpTestData()
+
+ expected_args = [call(self.test_case.migrate_from), call(self.test_case.migrate_to)]
+ self.assertEqual(migration_executor().loader.project_state.call_args_list, expected_args)
+
+ def test_loader_build_graph_gets_called_once(self, migration_executor):
+ """We should rebuild the migration graph before applying the second set of migrations."""
+ self.test_case.setUpTestData()
+
+ migration_executor().loader.build_graph.assert_called_once()
+
+ def test_migration_executor_migrate_method_is_called_correctly_twice(self, migration_executor):
+ """The migrate method of the executor should be called twice with the correct arguments."""
+ self.test_case.setUpTestData()
+
+ self.assertEqual(migration_executor().migrate.call_count, 2)
+ calls = [call([('api', 'scragly')]), call([('api', 'kosa')])]
+ migration_executor().migrate.assert_has_calls(calls)
+
+
+class LifeOfBrian(MigrationsTestCase):
+ app = "api"
+ migration_prior = "0046_reminder_jump_url"
+ migration_target = "0048_add_infractions_unique_constraints_active"
+
+ @classmethod
+ def log_last_migration(cls):
+ """Parses the applied migrations dictionary to log the last applied migration."""
+ loader = MigrationLoader(connection)
+ api_migrations = [
+ migration for app, migration in loader.applied_migrations if app == cls.app
+ ]
+ last_migration = max(api_migrations, key=lambda name: int(name[:4]))
+ log.info(f"The last applied migration: {last_migration}")
+
+ @classmethod
+ def setUpMigrationData(cls, apps):
+ """Method that logs the last applied migration at this point."""
+ cls.log_last_migration()
+
+ @classmethod
+ def setUpPostMigrationData(cls, apps):
+ """Method that logs the last applied migration at this point."""
+ cls.log_last_migration()
+
+
+class MigrationsTestCaseMigrationTest(TestCase):
+ """Tests if `MigrationsTestCase` travels to the right points in the migration history."""
+
+ def test_migrations_test_case_travels_to_correct_migrations_in_history(self):
+ """The test case should first revert to `migration_prior`, then go to `migration_target`."""
+ brian = LifeOfBrian()
+
+ with self.assertLogs(log, level=logging.INFO) as logs:
+ brian.setUpTestData()
+
+ self.assertEqual(len(logs.records), 2)
+
+ for time_point, record in zip(("migration_prior", "migration_target"), logs.records):
+ with self.subTest(time_point=time_point):
+ message = f"The last applied migration: {getattr(brian, time_point)}"
+ self.assertEqual(record.getMessage(), message)
diff --git a/pydis_site/apps/api/tests/test_infractions.py b/pydis_site/apps/api/tests/test_infractions.py
index c58c32e2..7a54640e 100644
--- a/pydis_site/apps/api/tests/test_infractions.py
+++ b/pydis_site/apps/api/tests/test_infractions.py
@@ -1,6 +1,8 @@
from datetime import datetime as dt, timedelta, timezone
+from unittest.mock import patch
from urllib.parse import quote
+from django.db.utils import IntegrityError
from django_hosts.resolvers import reverse
from .base import APISubdomainTestCase
@@ -167,6 +169,12 @@ class CreationTests(APISubdomainTestCase):
discriminator=1,
avatar_hash=None
)
+ cls.second_user = User.objects.create(
+ id=6,
+ name='carl',
+ discriminator=2,
+ avatar_hash=None
+ )
def test_accepts_valid_data(self):
url = reverse('bot:infraction-list', host='api')
@@ -305,6 +313,187 @@ class CreationTests(APISubdomainTestCase):
'hidden': [f'{data["type"]} infractions must be hidden.']
})
+ def test_returns_400_for_active_infraction_of_type_that_cannot_be_active(self):
+ """Test if the API rejects active infractions for types that cannot be active."""
+ url = reverse('bot:infraction-list', host='api')
+ restricted_types = (
+ ('note', True),
+ ('warning', False),
+ ('kick', False),
+ )
+
+ for infraction_type, hidden in restricted_types:
+ with self.subTest(infraction_type=infraction_type):
+ invalid_infraction = {
+ 'user': self.user.id,
+ 'actor': self.user.id,
+ 'type': infraction_type,
+ 'reason': 'Take me on!',
+ 'hidden': hidden,
+ 'active': True,
+ 'expires_at': None,
+ }
+ response = self.client.post(url, data=invalid_infraction)
+ self.assertEqual(response.status_code, 400)
+ self.assertEqual(
+ response.json(),
+ {'active': [f'{infraction_type} infractions cannot be active.']}
+ )
+
+ def test_returns_400_for_second_active_infraction_of_the_same_type(self):
+ """Test if the API rejects a second active infraction of the same type for a given user."""
+ url = reverse('bot:infraction-list', host='api')
+ active_infraction_types = ('mute', 'ban', 'superstar')
+
+ for infraction_type in active_infraction_types:
+ with self.subTest(infraction_type=infraction_type):
+ first_active_infraction = {
+ 'user': self.user.id,
+ 'actor': self.user.id,
+ 'type': infraction_type,
+ 'reason': 'Take me on!',
+ 'active': True,
+ 'expires_at': '2019-10-04T12:52:00+00:00'
+ }
+
+ # Post the first active infraction of a type and confirm it's accepted.
+ first_response = self.client.post(url, data=first_active_infraction)
+ self.assertEqual(first_response.status_code, 201)
+
+ second_active_infraction = {
+ 'user': self.user.id,
+ 'actor': self.user.id,
+ 'type': infraction_type,
+ 'reason': 'Take on me!',
+ 'active': True,
+ 'expires_at': '2019-10-04T12:52:00+00:00'
+ }
+ second_response = self.client.post(url, data=second_active_infraction)
+ self.assertEqual(second_response.status_code, 400)
+ self.assertEqual(
+ second_response.json(),
+ {
+ 'non_field_errors': [
+ 'This user already has an active infraction of this type.'
+ ]
+ }
+ )
+
+ def test_returns_201_for_second_active_infraction_of_different_type(self):
+ """Test if the API accepts a second active infraction of a different type than the first."""
+ url = reverse('bot:infraction-list', host='api')
+ first_active_infraction = {
+ 'user': self.user.id,
+ 'actor': self.user.id,
+ 'type': 'mute',
+ 'reason': 'Be silent!',
+ 'hidden': True,
+ 'active': True,
+ 'expires_at': '2019-10-04T12:52:00+00:00'
+ }
+ second_active_infraction = {
+ 'user': self.user.id,
+ 'actor': self.user.id,
+ 'type': 'ban',
+ 'reason': 'Be gone!',
+ 'hidden': True,
+ 'active': True,
+ 'expires_at': '2019-10-05T12:52:00+00:00'
+ }
+ # Post the first active infraction of a type and confirm it's accepted.
+ first_response = self.client.post(url, data=first_active_infraction)
+ self.assertEqual(first_response.status_code, 201)
+
+ # Post the first active infraction of a type and confirm it's accepted.
+ second_response = self.client.post(url, data=second_active_infraction)
+ self.assertEqual(second_response.status_code, 201)
+
+ def test_unique_constraint_raises_integrity_error_on_second_active_of_same_type(self):
+ """Do we raise `IntegrityError` for the second active infraction of a type for a user?"""
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The first active ban"
+ )
+ with self.assertRaises(IntegrityError):
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The second active ban"
+ )
+
+ def test_unique_constraint_accepts_active_infraction_after_inactive_infraction(self):
+ """Do we accept an active infraction if the others of the same type are inactive?"""
+ try:
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=False,
+ reason="The first inactive ban"
+ )
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=False,
+ reason="The second inactive ban"
+ )
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The first active ban"
+ )
+ except IntegrityError:
+ self.fail("An unexpected IntegrityError was raised.")
+
+ @patch(f"{__name__}.Infraction")
+ def test_if_accepts_active_infraction_test_catches_integrity_error(self, infraction_patch):
+ """Does the test properly catch the IntegrityError and raise an AssertionError?"""
+ infraction_patch.objects.create.side_effect = IntegrityError
+ with self.assertRaises(AssertionError, msg="An unexpected IntegrityError was raised."):
+ self.test_unique_constraint_accepts_active_infraction_after_inactive_infraction()
+
+ def test_unique_constraint_accepts_second_active_of_different_type(self):
+ """Do we accept a second active infraction of a different type for a given user?"""
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="The first active ban"
+ )
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="mute",
+ active=True,
+ reason="The first active mute"
+ )
+
+ def test_unique_constraint_accepts_active_infractions_for_different_users(self):
+ """Do we accept two active infractions of the same type for two different users?"""
+ Infraction.objects.create(
+ user=self.user,
+ actor=self.user,
+ type="ban",
+ active=True,
+ reason="An active ban for the first user"
+ )
+ Infraction.objects.create(
+ user=self.second_user,
+ actor=self.second_user,
+ type="ban",
+ active=False,
+ reason="An active ban for the second user"
+ )
+
class ExpandedTests(APISubdomainTestCase):
@classmethod
@@ -318,12 +507,14 @@ class ExpandedTests(APISubdomainTestCase):
cls.kick = Infraction.objects.create(
user_id=cls.user.id,
actor_id=cls.user.id,
- type='kick'
+ type='kick',
+ active=False
)
cls.warning = Infraction.objects.create(
user_id=cls.user.id,
actor_id=cls.user.id,
- type='warning'
+ type='warning',
+ active=False,
)
def check_expanded_fields(self, infraction):
diff --git a/pydis_site/apps/home/forms/__init__.py b/pydis_site/apps/home/forms/__init__.py
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/pydis_site/apps/home/forms/__init__.py
diff --git a/pydis_site/apps/home/forms/account_deletion.py b/pydis_site/apps/home/forms/account_deletion.py
new file mode 100644
index 00000000..17ffe5c1
--- /dev/null
+++ b/pydis_site/apps/home/forms/account_deletion.py
@@ -0,0 +1,24 @@
+from crispy_forms.helper import FormHelper
+from crispy_forms.layout import Layout
+from django.forms import CharField, Form
+from django_crispy_bulma.layout import IconField, Submit
+
+
+class AccountDeletionForm(Form):
+ """Account deletion form, to collect username for confirmation of removal."""
+
+ def __init__(self, *args, **kwargs):
+ super().__init__(*args, **kwargs)
+ self.helper = FormHelper()
+
+ self.helper.form_method = "post"
+ self.helper.add_input(Submit("submit", "I understand, delete my account"))
+
+ self.helper.layout = Layout(
+ IconField("username", icon_prepend="user")
+ )
+
+ username = CharField(
+ label="Username",
+ required=True
+ )
diff --git a/pydis_site/apps/home/signals.py b/pydis_site/apps/home/signals.py
index 9f286882..4cb4564b 100644
--- a/pydis_site/apps/home/signals.py
+++ b/pydis_site/apps/home/signals.py
@@ -1,3 +1,4 @@
+from contextlib import suppress
from typing import List, Optional, Type
from allauth.account.signals import user_logged_in
@@ -8,7 +9,7 @@ from allauth.socialaccount.signals import (
pre_social_login, social_account_added, social_account_removed,
social_account_updated)
from django.contrib.auth.models import Group, User as DjangoUser
-from django.db.models.signals import post_save, pre_delete, pre_save
+from django.db.models.signals import post_delete, post_save, pre_save
from pydis_site.apps.api.models import User as DiscordUser
from pydis_site.apps.staff.models import RoleMapping
@@ -37,7 +38,7 @@ class AllauthSignalListener:
def __init__(self):
post_save.connect(self.user_model_updated, sender=DiscordUser)
- pre_delete.connect(self.mapping_model_deleted, sender=RoleMapping)
+ post_delete.connect(self.mapping_model_deleted, sender=RoleMapping)
pre_save.connect(self.mapping_model_updated, sender=RoleMapping)
pre_social_login.connect(self.social_account_updated)
@@ -133,13 +134,29 @@ class AllauthSignalListener:
Processes deletion signals from the RoleMapping model, removing perms from users.
We need to do this to ensure that users aren't left with permissions groups that
- they shouldn't have assigned to them when a RoleMapping is deleted from the database.
+ they shouldn't have assigned to them when a RoleMapping is deleted from the database,
+ and to remove their staff status if they should no longer have it.
"""
instance: RoleMapping = kwargs["instance"]
for user in instance.group.user_set.all():
+ # Firstly, remove their related user group
user.groups.remove(instance.group)
+ with suppress(SocialAccount.DoesNotExist, DiscordUser.DoesNotExist):
+ # If we get either exception, then the user could not have been assigned staff
+ # with our system in the first place.
+
+ social_account = SocialAccount.objects.get(user=user, provider=DiscordProvider.id)
+ discord_user = DiscordUser.objects.get(id=int(social_account.uid))
+
+ mappings = RoleMapping.objects.filter(role__in=discord_user.roles.all()).all()
+ is_staff = any(m.is_staff for m in mappings)
+
+ if user.is_staff != is_staff:
+ user.is_staff = is_staff
+ user.save(update_fields=("is_staff", ))
+
def mapping_model_updated(self, sender: Type[RoleMapping], **kwargs) -> None:
"""
Processes update signals from the RoleMapping model.
@@ -174,6 +191,21 @@ class AllauthSignalListener:
for account in accounts:
account.user.groups.add(instance.group)
+ if instance.is_staff and not account.user.is_staff:
+ account.user.is_staff = instance.is_staff
+ account.user.save(update_fields=("is_staff", ))
+ else:
+ discord_user = DiscordUser.objects.get(id=int(account.uid))
+
+ mappings = RoleMapping.objects.filter(
+ role__in=discord_user.roles.all()
+ ).exclude(id=instance.id).all()
+ is_staff = any(m.is_staff for m in mappings)
+
+ if account.user.is_staff != is_staff:
+ account.user.is_staff = is_staff
+ account.user.save(update_fields=("is_staff",))
+
def user_model_updated(self, sender: Type[DiscordUser], **kwargs) -> None:
"""
Processes update signals from the Discord User model, assigning perms as required.
@@ -230,31 +262,53 @@ class AllauthSignalListener:
except SocialAccount.user.RelatedObjectDoesNotExist:
return # There's no user account yet, this will be handled by another receiver
+ # Ensure that the username on this account is correct
+ new_username = f"{user.name}#{user.discriminator}"
+
+ if account.user.username != new_username:
+ account.user.username = new_username
+ account.user.first_name = new_username
+
if not user.in_guild:
deletion = True
if deletion:
# They've unlinked Discord or left the server, so we have to remove their groups
+ # and their staff status
- if not current_groups:
- return # They have no groups anyway, no point in processing
+ if current_groups:
+ # They do have groups, so let's remove them
+ account.user.groups.remove(
+ *(mapping.group for mapping in mappings)
+ )
- account.user.groups.remove(
- *(mapping.group for mapping in mappings)
- )
+ if account.user.is_staff:
+ # They're marked as a staff user and they shouldn't be, so let's fix that
+ account.user.is_staff = False
else:
new_groups = []
+ is_staff = False
for role in user.roles.all():
try:
- new_groups.append(mappings.get(role=role).group)
+ mapping = mappings.get(role=role)
except RoleMapping.DoesNotExist:
continue # No mapping exists
- account.user.groups.add(
- *[group for group in new_groups if group not in current_groups]
- )
+ new_groups.append(mapping.group)
- account.user.groups.remove(
- *[mapping.group for mapping in mappings if mapping.group not in new_groups]
- )
+ if mapping.is_staff:
+ is_staff = True
+
+ account.user.groups.add(
+ *[group for group in new_groups if group not in current_groups]
+ )
+
+ account.user.groups.remove(
+ *[mapping.group for mapping in mappings if mapping.group not in new_groups]
+ )
+
+ if account.user.is_staff != is_staff:
+ account.user.is_staff = is_staff
+
+ account.user.save()
diff --git a/pydis_site/apps/home/tests/test_signal_listener.py b/pydis_site/apps/home/tests/test_signal_listener.py
index 27fc7710..66a67252 100644
--- a/pydis_site/apps/home/tests/test_signal_listener.py
+++ b/pydis_site/apps/home/tests/test_signal_listener.py
@@ -67,12 +67,14 @@ class SignalListenerTests(TestCase):
cls.admin_mapping = RoleMapping.objects.create(
role=cls.admin_role,
- group=cls.admin_group
+ group=cls.admin_group,
+ is_staff=True
)
cls.moderator_mapping = RoleMapping.objects.create(
role=cls.moderator_role,
- group=cls.moderator_group
+ group=cls.moderator_group,
+ is_staff=False
)
cls.discord_user = DiscordUser.objects.create(
@@ -166,7 +168,7 @@ class SignalListenerTests(TestCase):
cls.django_moderator = DjangoUser.objects.create(
username="moderator",
- is_staff=True,
+ is_staff=False,
is_superuser=False
)
@@ -339,6 +341,33 @@ class SignalListenerTests(TestCase):
self.discord_admin.roles.add(self.admin_role)
self.discord_admin.save()
+ def test_apply_groups_moderator(self):
+ """Test application of groups by role, relating to a non-`is_staff` moderator user."""
+ handler = AllauthSignalListener()
+
+ self.assertEqual(self.django_user_discordless.groups.all().count(), 0)
+
+ # Apply groups based on moderator role being present on Discord
+ handler._apply_groups(self.discord_moderator, self.social_moderator)
+ self.assertTrue(self.moderator_group in self.django_moderator.groups.all())
+
+ # Remove groups based on the user apparently leaving the server
+ handler._apply_groups(self.discord_moderator, self.social_moderator, True)
+ self.assertEqual(self.django_user_discordless.groups.all().count(), 0)
+
+ # Apply the moderator role again
+ handler._apply_groups(self.discord_moderator, self.social_moderator)
+
+ # Remove all of the roles from the user
+ self.discord_moderator.roles.clear()
+
+ # Remove groups based on the user no longer having the moderator role on Discord
+ handler._apply_groups(self.discord_moderator, self.social_moderator)
+ self.assertEqual(self.django_user_discordless.groups.all().count(), 0)
+
+ self.discord_moderator.roles.add(self.moderator_role)
+ self.discord_moderator.save()
+
def test_apply_groups_other(self):
"""Test application of groups by role, relating to non-standard cases."""
handler = AllauthSignalListener()
@@ -373,10 +402,25 @@ class SignalListenerTests(TestCase):
self.assertEqual(self.django_moderator.groups.all().count(), 1)
self.assertEqual(self.django_admin.groups.all().count(), 1)
+ # Test is_staff changes
+ self.admin_mapping.is_staff = False
+ self.admin_mapping.save()
+
+ self.assertFalse(self.django_moderator.is_staff)
+ self.assertFalse(self.django_admin.is_staff)
+
+ self.admin_mapping.is_staff = True
+ self.admin_mapping.save()
+
+ self.django_admin.refresh_from_db(fields=("is_staff", ))
+ self.assertTrue(self.django_admin.is_staff)
+
# Test mapping deletion
self.admin_mapping.delete()
+ self.django_admin.refresh_from_db(fields=("is_staff",))
self.assertEqual(self.django_admin.groups.all().count(), 0)
+ self.assertFalse(self.django_admin.is_staff)
# Test mapping update
self.moderator_mapping.group = self.admin_group
@@ -388,12 +432,30 @@ class SignalListenerTests(TestCase):
# Test mapping creation
new_mapping = RoleMapping.objects.create(
role=self.admin_role,
- group=self.moderator_group
+ group=self.moderator_group,
+ is_staff=True
+ )
+
+ self.assertEqual(self.django_admin.groups.all().count(), 1)
+ self.assertTrue(self.moderator_group in self.django_admin.groups.all())
+
+ self.django_admin.refresh_from_db(fields=("is_staff",))
+ self.assertTrue(self.django_admin.is_staff)
+
+ new_mapping.delete()
+
+ # Test mapping creation (without is_staff)
+ new_mapping = RoleMapping.objects.create(
+ role=self.admin_role,
+ group=self.moderator_group,
)
self.assertEqual(self.django_admin.groups.all().count(), 1)
self.assertTrue(self.moderator_group in self.django_admin.groups.all())
+ self.django_admin.refresh_from_db(fields=("is_staff",))
+ self.assertFalse(self.django_admin.is_staff)
+
# Test that nothing happens when fixtures are loaded
pre_save.send(RoleMapping, instance=new_mapping, raw=True)
diff --git a/pydis_site/apps/home/tests/test_views.py b/pydis_site/apps/home/tests/test_views.py
index 7aeaddd2..572317a7 100644
--- a/pydis_site/apps/home/tests/test_views.py
+++ b/pydis_site/apps/home/tests/test_views.py
@@ -1,5 +1,198 @@
+from allauth.socialaccount.models import SocialAccount
+from django.contrib.auth.models import User
+from django.http import HttpResponseRedirect
from django.test import TestCase
-from django_hosts.resolvers import reverse
+from django_hosts.resolvers import get_host, reverse, reverse_host
+
+
+def check_redirect_url(
+ response: HttpResponseRedirect, reversed_url: str, strip_params=True
+) -> bool:
+ """
+ Check whether a given redirect response matches a specific reversed URL.
+
+ Arguments:
+ * `response`: The HttpResponseRedirect returned by the test client
+ * `reversed_url`: The URL returned by `reverse()`
+ * `strip_params`: Whether to strip URL parameters (following a "?") from the URL given in the
+ `response` object
+ """
+ host = get_host(None)
+ hostname = reverse_host(host)
+
+ redirect_url = response.url
+
+ if strip_params and "?" in redirect_url:
+ redirect_url = redirect_url.split("?", 1)[0]
+
+ result = reversed_url == f"//{hostname}{redirect_url}"
+ return result
+
+
+class TestAccountDeleteView(TestCase):
+ def setUp(self) -> None:
+ """Create an authorized Django user for testing purposes."""
+ self.user = User.objects.create(
+ username="user#0000"
+ )
+
+ def test_redirect_when_logged_out(self):
+ """Test that the user is redirected to the homepage when not logged in."""
+ url = reverse("account_delete")
+ resp = self.client.get(url)
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
+
+ def test_get_when_logged_in(self):
+ """Test that the view returns a HTTP 200 when the user is logged in."""
+ url = reverse("account_delete")
+
+ self.client.force_login(self.user)
+ resp = self.client.get(url)
+ self.client.logout()
+
+ self.assertEqual(resp.status_code, 200)
+
+ def test_post_invalid(self):
+ """Test that the user is redirected when the form is filled out incorrectly."""
+ url = reverse("account_delete")
+
+ self.client.force_login(self.user)
+
+ resp = self.client.post(url, {})
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, url))
+
+ resp = self.client.post(url, {"username": "user"})
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, url))
+
+ self.client.logout()
+
+ def test_post_valid(self):
+ """Test that the account is deleted when the form is filled out correctly.."""
+ url = reverse("account_delete")
+
+ self.client.force_login(self.user)
+
+ resp = self.client.post(url, {"username": "user#0000"})
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
+
+ with self.assertRaises(User.DoesNotExist):
+ User.objects.get(username=self.user.username)
+
+ self.client.logout()
+
+
+class TestAccountSettingsView(TestCase):
+ def setUp(self) -> None:
+ """Create an authorized Django user for testing purposes."""
+ self.user = User.objects.create(
+ username="user#0000"
+ )
+
+ self.user_unlinked = User.objects.create(
+ username="user#9999"
+ )
+
+ self.user_unlinked_discord = User.objects.create(
+ username="user#1234"
+ )
+
+ self.user_unlinked_github = User.objects.create(
+ username="user#1111"
+ )
+
+ self.github_account = SocialAccount.objects.create(
+ user=self.user,
+ provider="github",
+ uid="0"
+ )
+
+ self.discord_account = SocialAccount.objects.create(
+ user=self.user,
+ provider="discord",
+ uid="0000"
+ )
+
+ self.github_account_secondary = SocialAccount.objects.create(
+ user=self.user_unlinked_discord,
+ provider="github",
+ uid="1"
+ )
+
+ self.discord_account_secondary = SocialAccount.objects.create(
+ user=self.user_unlinked_github,
+ provider="discord",
+ uid="1111"
+ )
+
+ def test_redirect_when_logged_out(self):
+ """Check that the user is redirected to the homepage when not logged in."""
+ url = reverse("account_settings")
+ resp = self.client.get(url)
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
+
+ def test_get_when_logged_in(self):
+ """Test that the view returns a HTTP 200 when the user is logged in."""
+ url = reverse("account_settings")
+
+ self.client.force_login(self.user)
+ resp = self.client.get(url)
+ self.client.logout()
+
+ self.assertEqual(resp.status_code, 200)
+
+ self.client.force_login(self.user_unlinked)
+ resp = self.client.get(url)
+ self.client.logout()
+
+ self.assertEqual(resp.status_code, 200)
+
+ self.client.force_login(self.user_unlinked_discord)
+ resp = self.client.get(url)
+ self.client.logout()
+
+ self.assertEqual(resp.status_code, 200)
+
+ self.client.force_login(self.user_unlinked_github)
+ resp = self.client.get(url)
+ self.client.logout()
+
+ self.assertEqual(resp.status_code, 200)
+
+ def test_post_invalid(self):
+ """Test the behaviour of invalid POST submissions."""
+ url = reverse("account_settings")
+
+ self.client.force_login(self.user_unlinked)
+
+ resp = self.client.post(url, {"provider": "discord"})
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
+
+ resp = self.client.post(url, {"provider": "github"})
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
+
+ self.client.logout()
+
+ def test_post_valid(self):
+ """Ensure that GitHub is unlinked with a valid POST submission."""
+ url = reverse("account_settings")
+
+ self.client.force_login(self.user)
+
+ resp = self.client.post(url, {"provider": "github"})
+ self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
+
+ with self.assertRaises(SocialAccount.DoesNotExist):
+ SocialAccount.objects.get(user=self.user, provider="github")
+
+ self.client.logout()
class TestIndexReturns200(TestCase):
@@ -16,6 +209,7 @@ class TestLoginCancelledReturns302(TestCase):
url = reverse('socialaccount_login_cancelled')
resp = self.client.get(url)
self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
class TestLoginErrorReturns302(TestCase):
@@ -24,3 +218,4 @@ class TestLoginErrorReturns302(TestCase):
url = reverse('socialaccount_login_error')
resp = self.client.get(url)
self.assertEqual(resp.status_code, 302)
+ self.assertTrue(check_redirect_url(resp, reverse("home")))
diff --git a/pydis_site/apps/home/urls.py b/pydis_site/apps/home/urls.py
index 211a7ad1..61e87a39 100644
--- a/pydis_site/apps/home/urls.py
+++ b/pydis_site/apps/home/urls.py
@@ -1,5 +1,4 @@
from allauth.account.views import LogoutView
-from allauth.socialaccount.views import ConnectionsView
from django.conf import settings
from django.conf.urls.static import static
from django.contrib import admin
@@ -7,14 +6,18 @@ from django.contrib.messages import ERROR
from django.urls import include, path
from pydis_site.utils.views import MessageRedirectView
-from .views import HomeView
+from .views import AccountDeleteView, AccountSettingsView, HomeView
app_name = 'home'
urlpatterns = [
+ # We do this twice because Allauth expects specific view names to exist
path('', HomeView.as_view(), name='home'),
+ path('', HomeView.as_view(), name='socialaccount_connections'),
+
path('pages/', include('wiki.urls')),
path('accounts/', include('allauth.socialaccount.providers.discord.urls')),
+ path('accounts/', include('allauth.socialaccount.providers.github.urls')),
path(
'accounts/login/cancelled', MessageRedirectView.as_view(
@@ -28,7 +31,9 @@ urlpatterns = [
), name='socialaccount_login_error'
),
- path('connections', ConnectionsView.as_view()),
+ path('accounts/settings', AccountSettingsView.as_view(), name="account_settings"),
+ path('accounts/delete', AccountDeleteView.as_view(), name="account_delete"),
+
path('logout', LogoutView.as_view(), name="logout"),
path('admin/', admin.site.urls),
diff --git a/pydis_site/apps/home/views/__init__.py b/pydis_site/apps/home/views/__init__.py
index 971d73a3..801fd398 100644
--- a/pydis_site/apps/home/views/__init__.py
+++ b/pydis_site/apps/home/views/__init__.py
@@ -1,3 +1,4 @@
+from .account import DeleteView as AccountDeleteView, SettingsView as AccountSettingsView
from .home import HomeView
-__all__ = ["HomeView"]
+__all__ = ["AccountDeleteView", "AccountSettingsView", "HomeView"]
diff --git a/pydis_site/apps/home/views/account/__init__.py b/pydis_site/apps/home/views/account/__init__.py
new file mode 100644
index 00000000..3b3250ea
--- /dev/null
+++ b/pydis_site/apps/home/views/account/__init__.py
@@ -0,0 +1,4 @@
+from .delete import DeleteView
+from .settings import SettingsView
+
+__all__ = ["DeleteView", "SettingsView"]
diff --git a/pydis_site/apps/home/views/account/delete.py b/pydis_site/apps/home/views/account/delete.py
new file mode 100644
index 00000000..798b8a33
--- /dev/null
+++ b/pydis_site/apps/home/views/account/delete.py
@@ -0,0 +1,37 @@
+from django.contrib.auth.mixins import LoginRequiredMixin
+from django.contrib.messages import ERROR, INFO, add_message
+from django.http import HttpRequest, HttpResponse
+from django.shortcuts import redirect, render
+from django.urls import reverse
+from django.views import View
+
+from pydis_site.apps.home.forms.account_deletion import AccountDeletionForm
+
+
+class DeleteView(LoginRequiredMixin, View):
+ """Account deletion view, for removing linked user accounts from the DB."""
+
+ def __init__(self, *args, **kwargs):
+ self.login_url = reverse("home")
+ super().__init__(*args, **kwargs)
+
+ def get(self, request: HttpRequest) -> HttpResponse:
+ """HTTP GET: Return the view template."""
+ return render(
+ request, "home/account/delete.html",
+ context={"form": AccountDeletionForm()}
+ )
+
+ def post(self, request: HttpRequest) -> HttpResponse:
+ """HTTP POST: Process the deletion, as requested by the user."""
+ form = AccountDeletionForm(request.POST)
+
+ if not form.is_valid() or request.user.username != form.cleaned_data["username"]:
+ add_message(request, ERROR, "Please enter your username exactly as shown.")
+
+ return redirect(reverse("account_delete"))
+
+ request.user.delete()
+ add_message(request, INFO, "Your account has been deleted.")
+
+ return redirect(reverse("home"))
diff --git a/pydis_site/apps/home/views/account/settings.py b/pydis_site/apps/home/views/account/settings.py
new file mode 100644
index 00000000..3a817dbc
--- /dev/null
+++ b/pydis_site/apps/home/views/account/settings.py
@@ -0,0 +1,59 @@
+from allauth.socialaccount.models import SocialAccount
+from allauth.socialaccount.providers import registry
+from django.contrib.auth.mixins import LoginRequiredMixin
+from django.contrib.messages import ERROR, INFO, add_message
+from django.http import HttpRequest, HttpResponse
+from django.shortcuts import redirect, render
+from django.urls import reverse
+from django.views import View
+
+
+class SettingsView(LoginRequiredMixin, View):
+ """
+ Account settings view, for managing and deleting user accounts and connections.
+
+ This view actually renders a template with a bare modal, and is intended to be
+ inserted into another template using JavaScript.
+ """
+
+ def __init__(self, *args, **kwargs):
+ self.login_url = reverse("home")
+ super().__init__(*args, **kwargs)
+
+ def get(self, request: HttpRequest) -> HttpResponse:
+ """HTTP GET: Return the view template."""
+ context = {
+ "groups": request.user.groups.all(),
+
+ "discord": None,
+ "github": None,
+
+ "discord_provider": registry.provider_map.get("discord"),
+ "github_provider": registry.provider_map.get("github"),
+ }
+
+ for account in SocialAccount.objects.filter(user=request.user).all():
+ if account.provider == "discord":
+ context["discord"] = account
+
+ if account.provider == "github":
+ context["github"] = account
+
+ return render(request, "home/account/settings.html", context=context)
+
+ def post(self, request: HttpRequest) -> HttpResponse:
+ """HTTP POST: Process account disconnections."""
+ provider = request.POST["provider"]
+
+ if provider == "github":
+ try:
+ account = SocialAccount.objects.get(user=request.user, provider=provider)
+ except SocialAccount.DoesNotExist:
+ add_message(request, ERROR, "You do not have a GitHub account linked.")
+ else:
+ account.delete()
+ add_message(request, INFO, "The social account has been disconnected.")
+ else:
+ add_message(request, ERROR, f"Unknown provider: {provider}")
+
+ return redirect(reverse("home"))
diff --git a/pydis_site/apps/staff/migrations/0002_add_is_staff_to_role_mappings.py b/pydis_site/apps/staff/migrations/0002_add_is_staff_to_role_mappings.py
new file mode 100644
index 00000000..0404d270
--- /dev/null
+++ b/pydis_site/apps/staff/migrations/0002_add_is_staff_to_role_mappings.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.6 on 2019-10-20 14:19
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('staff', '0001_initial'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='rolemapping',
+ name='is_staff',
+ field=models.BooleanField(default=False, help_text='Whether this role mapping relates to a Django staff group'),
+ ),
+ ]
diff --git a/pydis_site/apps/staff/models/role_mapping.py b/pydis_site/apps/staff/models/role_mapping.py
index 10c09cf1..8a1fac2e 100644
--- a/pydis_site/apps/staff/models/role_mapping.py
+++ b/pydis_site/apps/staff/models/role_mapping.py
@@ -21,6 +21,11 @@ class RoleMapping(models.Model):
unique=True, # Unique in order to simplify group assignment logic
)
+ is_staff = models.BooleanField(
+ help_text="Whether this role mapping relates to a Django staff group",
+ default=False
+ )
+
def __str__(self):
"""Returns the mapping, for display purposes."""
return f"@{self.role.name} -> {self.group.name}"
diff --git a/pydis_site/settings.py b/pydis_site/settings.py
index 94718ec7..66376c4e 100644
--- a/pydis_site/settings.py
+++ b/pydis_site/settings.py
@@ -92,6 +92,7 @@ INSTALLED_APPS = [
'allauth.socialaccount',
'allauth.socialaccount.providers.discord',
+ 'allauth.socialaccount.providers.github',
'crispy_forms',
'django_crispy_bulma',
@@ -302,14 +303,14 @@ BULMA_SETTINGS = {
"variables": { # If you update these colours, please update the notification.css file
"primary": "#7289DA", # Discord blurple
- "orange": "#ffb39b", # Bulma default, but at a saturation of 100
- "yellow": "#ffea9b", # Bulma default, but at a saturation of 100
- "green": "#7fd19c", # Bulma default, but at a saturation of 100
- "turquoise": "#7289DA", # Blurple, because Bulma uses this as the default primary
- "cyan": "#91cbee", # Bulma default, but at a saturation of 100
- "blue": "#86a7dc", # Bulma default, but at a saturation of 100
- "purple": "#b86bff", # Bulma default, but at a saturation of 100
- "red": "#ffafc2", # Bulma default, but at a saturation of 80
+ # "orange": "", # Apparently unused, but the default is fine
+ # "yellow": "", # The default yellow looks pretty good
+ "green": "#32ac66", # Colour picked after Discord discussion
+ "turquoise": "#7289DA", # Blurple, because Bulma uses this regardless of `primary` above
+ "blue": "#2482c1", # Colour picked after Discord discussion
+ "cyan": "#2482c1", # Colour picked after Discord discussion (matches the blue)
+ "purple": "#aa55e4", # Apparently unused, but changed for consistency
+ "red": "#d63852", # Colour picked after Discord discussion
"link": "$primary",
@@ -407,5 +408,13 @@ AUTHENTICATION_BACKENDS = (
'allauth.account.auth_backends.AuthenticationBackend',
)
+ACCOUNT_ADAPTER = "pydis_site.utils.account.AccountAdapter"
+ACCOUNT_EMAIL_REQUIRED = False # Undocumented allauth setting; don't require emails
ACCOUNT_EMAIL_VERIFICATION = "none" # No verification required; we don't use emails for anything
+
+# We use this validator because Allauth won't let us actually supply a list with no validators
+# in it, and we can't just give it a lambda - that'd be too easy, I suppose.
+ACCOUNT_USERNAME_VALIDATORS = "pydis_site.VALIDATORS"
+
LOGIN_REDIRECT_URL = "home"
+SOCIALACCOUNT_ADAPTER = "pydis_site.utils.account.SocialAccountAdapter"
diff --git a/pydis_site/static/css/base/base.css b/pydis_site/static/css/base/base.css
index 3ca6b2a7..75702d47 100644
--- a/pydis_site/static/css/base/base.css
+++ b/pydis_site/static/css/base/base.css
@@ -84,7 +84,25 @@ div.card.has-equal-height {
/* Fix for logout form submit button in navbar */
-button.is-size-navbar-menu {
+button.is-size-navbar-menu, a.is-size-navbar-menu {
font-size: 14px;
+ padding-left: 1.5rem;
+ padding-right: 1.5rem;
}
+@media screen and (min-width: 1088px) {
+ button.is-size-navbar-menu, a.is-size-navbar-menu {
+ padding-left: 1rem;
+ padding-right: 1rem;
+ }
+}
+
+/* Fix for modals being behind the navbar */
+
+.modal * {
+ z-index: 1020;
+}
+
+.modal-background {
+ z-index: 1010;
+}
diff --git a/pydis_site/static/js/base/modal.js b/pydis_site/static/js/base/modal.js
new file mode 100644
index 00000000..eccc8845
--- /dev/null
+++ b/pydis_site/static/js/base/modal.js
@@ -0,0 +1,100 @@
+/*
+ modal.js: A simple way to wire up Bulma modals.
+
+ This library is intended to be used with Bulma's modals, as described in the
+ official Bulma documentation. It's based on the JavaScript that Bulma
+ themselves use for this purpose on the modals documentation page.
+
+ Note that, just like that piece of JavaScript, this library assumes that
+ you will only ever want to have one modal open at once.
+ */
+
+"use strict";
+
+// Event handler for the "esc" key, for closing modals.
+
+document.addEventListener("keydown", (event) => {
+ const e = event || window.event;
+
+ if (e.code === "Escape" || e.keyCode === 27) {
+ closeModals();
+ }
+});
+
+// An array of all the modal buttons we've already set up
+
+const modal_buttons = [];
+
+// Public API functions
+
+function setupModal(target) {
+ // Set up a modal's events, given a DOM element. This can be
+ // used later in order to set up a modal that was added after
+ // this library has been run.
+
+ // We need to collect a bunch of elements to work with
+ const modal_background = Array.from(target.getElementsByClassName("modal-background"));
+ const modal_close = Array.from(target.getElementsByClassName("modal-close"));
+
+ const modal_head = Array.from(target.getElementsByClassName("modal-card-head"));
+ const modal_foot = Array.from(target.getElementsByClassName("modal-card-foot"));
+
+ const modal_delete = [];
+ const modal_button = [];
+
+ modal_head.forEach((element) => modal_delete.concat(Array.from(element.getElementsByClassName("delete"))));
+ modal_foot.forEach((element) => modal_button.concat(Array.from(element.getElementsByClassName("button"))));
+
+ // Collect all the elements that can be used to close modals
+ const modal_closers = modal_background.concat(modal_close).concat(modal_delete).concat(modal_button);
+
+ // Assign click events for closing modals
+ modal_closers.forEach((element) => {
+ element.addEventListener("click", () => {
+ closeModals();
+ });
+ });
+
+ setupOpeningButtons();
+}
+
+function setupOpeningButtons() {
+ // Wire up all the opening buttons, avoiding buttons we've already wired up.
+ const modal_opening_buttons = Array.from(document.getElementsByClassName("modal-button"));
+
+ modal_opening_buttons.forEach((element) => {
+ if (!modal_buttons.includes(element)) {
+ element.addEventListener("click", () => {
+ openModal(element.dataset.target);
+ });
+
+ modal_buttons.push(element);
+ }
+ });
+}
+
+function openModal(target) {
+ // Open a modal, given a string ID
+ const element = document.getElementById(target);
+
+ document.documentElement.classList.add("is-clipped");
+ element.classList.add("is-active");
+}
+
+function closeModals() {
+ // Close all open modals
+ const modals = Array.from(document.getElementsByClassName("modal"));
+ document.documentElement.classList.remove("is-clipped");
+
+ modals.forEach((element) => {
+ element.classList.remove("is-active");
+ });
+}
+
+(function () {
+ // Set up all the modals currently on the page
+ const modals = Array.from(document.getElementsByClassName("modal"));
+
+ modals.forEach((modal) => setupModal(modal));
+ setupOpeningButtons();
+}());
diff --git a/pydis_site/templates/base/base.html b/pydis_site/templates/base/base.html
index a9b31c0f..4c70d778 100644
--- a/pydis_site/templates/base/base.html
+++ b/pydis_site/templates/base/base.html
@@ -28,6 +28,7 @@
{# Font-awesome here is defined explicitly so that we can have Pro #}
<script src="https://kit.fontawesome.com/ae6a3152d8.js"></script>
+ <script src="{% static "js/base/modal.js" %}"></script>
<link rel="stylesheet" href="{% static "css/base/base.css" %}">
<link rel="stylesheet" href="{% static "css/base/notification.css" %}">
diff --git a/pydis_site/templates/base/navbar.html b/pydis_site/templates/base/navbar.html
index 8cdac0de..2ba5bdd4 100644
--- a/pydis_site/templates/base/navbar.html
+++ b/pydis_site/templates/base/navbar.html
@@ -102,7 +102,15 @@
{% else %}
<form method="post" action="{% url 'logout' %}">
{% csrf_token %}
- <button type="submit" class="navbar-item button is-white is-inline is-fullwidth has-text-left is-size-navbar-menu has-text-grey-dark">Logout</button>
+
+ <div class="field navbar-item is-paddingless is-fullwidth is-grouped">
+ <button type="submit" class="button is-white is-inline is-fullwidth has-text-left is-size-navbar-menu has-text-grey-dark">Logout</button>
+ <a title="Settings" class="button is-white is-inline has-text-right is-size-navbar-menu has-text-grey-dark modal-button" data-target="account-modal">
+ <span class="is-icon">
+ <i class="fas fa-cog"></i>
+ </span>
+ </a>
+ </div>
</form>
{% endif %}
@@ -116,3 +124,24 @@
</a>
</div>
</nav>
+
+{% if user.is_authenticated %}
+ <script defer type="text/javascript">
+ // Script which loads and sets up the account settings modal.
+ // This script must be placed in a template, or rewritten to take the fetch
+ // URL as a function argument, in order to be used.
+
+ "use strict";
+
+ // Create and prepend a new div for this modal
+ let element = document.createElement("div");
+ document.body.prepend(element);
+
+ fetch("{% url "account_settings" %}") // Fetch the URL
+ .then((response) => response.text()) // Read in the data stream as text
+ .then((text) => {
+ element.outerHTML = text; // Replace the div's HTML with the loaded modal HTML
+ setupModal(document.getElementById("account-modal")); // Set up the modal
+ });
+ </script>
+{% endif %}
diff --git a/pydis_site/templates/home/account/delete.html b/pydis_site/templates/home/account/delete.html
new file mode 100644
index 00000000..1020a82b
--- /dev/null
+++ b/pydis_site/templates/home/account/delete.html
@@ -0,0 +1,44 @@
+{% extends 'base/base.html' %}
+
+{% load crispy_forms_tags %}
+{% load static %}
+
+{% block title %}Delete Account{% endblock %}
+
+{% block content %}
+ {% include "base/navbar.html" %}
+
+ <section class="section content">
+ <div class="container">
+ <h2 class="is-size-2 has-text-centered">Account Deletion</h2>
+
+ <div class="columns is-centered">
+ <div class="column is-half-desktop is-full-tablet is-full-mobile">
+
+ <article class="message is-danger">
+ <div class="message-body">
+ <p>
+ You have requested to delete the account with username
+ <strong><span class="has-text-dark is-family-monospace">{{ user.username }}</span></strong>.
+ </p>
+
+ <p>
+ Please note that this <strong>cannot be undone</strong>.
+ </p>
+
+ <p>
+ To verify that you'd like to remove your account, please type your username into the box below.
+ </p>
+ </div>
+ </article>
+ </div>
+ </div>
+
+ <div class="columns is-centered">
+ <div class="column is-half-desktop is-full-tablet is-full-mobile">
+ {% crispy form %}
+ </div>
+ </div>
+ </div>
+ </section>
+{% endblock %}
diff --git a/pydis_site/templates/home/account/settings.html b/pydis_site/templates/home/account/settings.html
new file mode 100644
index 00000000..9f48d736
--- /dev/null
+++ b/pydis_site/templates/home/account/settings.html
@@ -0,0 +1,137 @@
+{% load socialaccount %}
+
+{# This template is just for a modal, which is actually inserted into the navbar #}
+{# template. Take a look at `navbar.html` to see how it's inserted. #}
+
+<div class="modal" id="account-modal">
+ <div class="modal-background"></div>
+ <div class="modal-card">
+ <div class="modal-card-head">
+ <div class="modal-card-title">Settings for {{ user.username }}</div>
+
+ {% if groups %}
+ <div>
+ {% for group in groups %}
+ <span class="tag is-primary">{{ group.name }}</span>
+ {% endfor %}
+ </div>
+ {% else %}
+ <span class="tag is-dark">No groups</span>
+ {% endif %}
+ </div>
+ <div class="modal-card-body">
+ <h3 class="title">Connections</h3>
+ <div class="columns">
+ {% if discord_provider is not None %}
+ <div class="column">
+ <div class="box">
+ {% if not discord %}
+ <div class="media">
+ <div class="media-left">
+ <div class="image">
+ <i class="fab fa-discord fa-3x has-text-primary"></i>
+ </div>
+ </div>
+ <div class="media-content">
+ <div class="title is-5">Discord</div>
+ <div class="subtitle is-6">Not connected</div>
+ </div>
+ </div>
+ <div>
+ <br />
+ <a class="button is-primary" href="{% provider_login_url "discord" process="connect" %}">
+ <span class="icon">
+ <i class="fad fa-link"></i>
+ </span>
+ <span>Connect</span>
+ </a>
+ </div>
+ {% else %}
+ <div class="media">
+ <div class="media-left">
+ <div class="image">
+ <i class="fab fa-discord fa-3x has-text-primary"></i>
+ </div>
+ </div>
+ <div class="media-content">
+ <div class="title is-5">Discord</div>
+ <div class="subtitle is-6">{{ user.username }}</div>
+ </div>
+ </div>
+ <div>
+ <br />
+ <button class="button" disabled>
+ <span class="icon">
+ <i class="fas fa-check"></i>
+ </span>
+ <span>Connected</span>
+ </button>
+ </div>
+ {% endif %}
+ </div>
+ </div>
+ {% endif %}
+
+ {% if github_provider is not None %}
+ <div class="column">
+ <div class="box">
+ {% if not github %}
+ <div class="media">
+ <div class="media-left">
+ <div class="image">
+ <i class="fab fa-github fa-3x"></i>
+ </div>
+ </div>
+ <div class="media-content">
+ <div class="title is-5">GitHub</div>
+ <div class="subtitle is-6">Not connected</div>
+ </div>
+ </div>
+ <div>
+ <br />
+ <a class="button is-primary" href="{% provider_login_url "github" process="connect" %}">
+ <span class="icon">
+ <i class="fad fa-link"></i>
+ </span>
+ <span>Connect</span>
+ </a>
+ </div>
+ {% else %}
+ <div class="media">
+ <div class="media-left">
+ <div class="image">
+ <i class="fab fa-github fa-3x"></i>
+ </div>
+ </div>
+ <div class="media-content">
+ <div class="title is-5">GitHub</div>
+ <div class="subtitle is-6">{{ github.extra_data.name }}</div>
+ </div>
+ </div>
+ <div>
+ <form method="post" action="{% url "account_settings" %}" type="submit">
+ {% csrf_token %}
+
+ <input type="hidden" name="provider" value="github" />
+
+ <br />
+ <button type="submit" class="button is-danger">
+ <span class="icon">
+ <i class="fas fa-times"></i>
+ </span>
+ <span>Disconnect</span>
+ </button>
+ </form>
+ </div>
+ {% endif %}
+ </div>
+ </div>
+ {% endif %}
+ </div>
+ </div>
+ <div class="modal-card-foot">
+ <a class="button is-danger" href="{% url "account_delete" %}">Delete Account</a>
+ </div>
+ </div>
+</div>
+
diff --git a/pydis_site/tests/test_utils_account.py b/pydis_site/tests/test_utils_account.py
new file mode 100644
index 00000000..d2946368
--- /dev/null
+++ b/pydis_site/tests/test_utils_account.py
@@ -0,0 +1,132 @@
+from unittest.mock import patch
+
+from allauth.exceptions import ImmediateHttpResponse
+from allauth.socialaccount.models import SocialAccount, SocialLogin
+from django.contrib.auth.models import User
+from django.contrib.messages.storage.base import BaseStorage
+from django.http import HttpRequest
+from django.test import TestCase
+
+from pydis_site.apps.api.models import Role, User as DiscordUser
+from pydis_site.utils.account import AccountAdapter, SocialAccountAdapter
+
+
+class AccountUtilsTests(TestCase):
+ def setUp(self):
+ self.django_user = User.objects.create(username="user")
+
+ self.discord_account = SocialAccount.objects.create(
+ user=self.django_user, provider="discord", uid=0
+ )
+
+ self.discord_account_role = SocialAccount.objects.create(
+ user=self.django_user, provider="discord", uid=1
+ )
+
+ self.discord_account_two_roles = SocialAccount.objects.create(
+ user=self.django_user, provider="discord", uid=2
+ )
+
+ self.discord_account_not_present = SocialAccount.objects.create(
+ user=self.django_user, provider="discord", uid=3
+ )
+
+ self.github_account = SocialAccount.objects.create(
+ user=self.django_user, provider="github", uid=0
+ )
+
+ self.discord_user = DiscordUser.objects.create(
+ id=0,
+ name="user",
+ discriminator=0
+ )
+
+ self.discord_user_role = DiscordUser.objects.create(
+ id=1,
+ name="user present",
+ discriminator=0
+ )
+
+ self.discord_user_two_roles = DiscordUser.objects.create(
+ id=2,
+ name="user with both roles",
+ discriminator=0
+ )
+
+ everyone_role = Role.objects.create(
+ id=0,
+ name="@everyone",
+ colour=0,
+ permissions=0,
+ position=0
+ )
+
+ self.discord_user_role.roles.add(everyone_role)
+ self.discord_user_two_roles.roles.add(everyone_role)
+
+ self.discord_user_two_roles.roles.add(Role.objects.create(
+ id=1,
+ name="Developers",
+ colour=0,
+ permissions=0,
+ position=1
+ ))
+
+ def test_account_adapter(self):
+ """Test that our Allauth account adapter functions correctly."""
+ adapter = AccountAdapter()
+
+ self.assertFalse(adapter.is_open_for_signup(HttpRequest()))
+
+ def test_social_account_adapter_signup(self):
+ """Test that our Allauth social account adapter correctly handles signups."""
+ adapter = SocialAccountAdapter()
+
+ discord_login = SocialLogin(account=self.discord_account)
+ discord_login_role = SocialLogin(account=self.discord_account_role)
+ discord_login_two_roles = SocialLogin(account=self.discord_account_two_roles)
+ discord_login_not_present = SocialLogin(account=self.discord_account_not_present)
+
+ github_login = SocialLogin(account=self.github_account)
+
+ messages_request = HttpRequest()
+ messages_request._messages = BaseStorage(messages_request)
+
+ with patch("pydis_site.utils.account.reverse") as mock_reverse:
+ with patch("pydis_site.utils.account.redirect") as mock_redirect:
+ with self.assertRaises(ImmediateHttpResponse):
+ adapter.is_open_for_signup(messages_request, github_login)
+
+ with self.assertRaises(ImmediateHttpResponse):
+ adapter.is_open_for_signup(messages_request, discord_login)
+
+ with self.assertRaises(ImmediateHttpResponse):
+ adapter.is_open_for_signup(messages_request, discord_login_role)
+
+ with self.assertRaises(ImmediateHttpResponse):
+ adapter.is_open_for_signup(messages_request, discord_login_not_present)
+
+ self.assertEqual(len(messages_request._messages._queued_messages), 4)
+ self.assertEqual(mock_redirect.call_count, 4)
+ self.assertEqual(mock_reverse.call_count, 4)
+
+ self.assertTrue(adapter.is_open_for_signup(HttpRequest(), discord_login_two_roles))
+
+ def test_social_account_adapter_populate(self):
+ """Test that our Allauth social account adapter correctly handles data population."""
+ adapter = SocialAccountAdapter()
+
+ discord_login = SocialLogin(
+ account=self.discord_account,
+ user=self.django_user
+ )
+
+ discord_login.account.extra_data["discriminator"] = "0000"
+
+ user = adapter.populate_user(
+ HttpRequest(), discord_login,
+ {"username": "user"}
+ )
+
+ self.assertEqual(user.username, "user#0000")
+ self.assertEqual(user.first_name, "user#0000")
diff --git a/pydis_site/utils/account.py b/pydis_site/utils/account.py
new file mode 100644
index 00000000..e64919de
--- /dev/null
+++ b/pydis_site/utils/account.py
@@ -0,0 +1,79 @@
+from typing import Any, Dict
+
+from allauth.account.adapter import DefaultAccountAdapter
+from allauth.exceptions import ImmediateHttpResponse
+from allauth.socialaccount.adapter import DefaultSocialAccountAdapter
+from allauth.socialaccount.models import SocialLogin
+from django.contrib.auth.models import User as DjangoUser
+from django.contrib.messages import ERROR, add_message
+from django.http import HttpRequest
+from django.shortcuts import redirect
+from django.urls import reverse
+
+from pydis_site.apps.api.models import User as DiscordUser
+
+ERROR_CONNECT_DISCORD = ("You must login with Discord before connecting another account. "
+ "Your account details have not been saved.")
+ERROR_JOIN_DISCORD = ("Please join the Discord server and verify that you accept the rules and "
+ "privacy policy.")
+
+
+class AccountAdapter(DefaultAccountAdapter):
+ """An Allauth account adapter that prevents signups via form submission."""
+
+ def is_open_for_signup(self, request: HttpRequest) -> bool:
+ """
+ Checks whether or not the site is open for signups.
+
+ We override this to always return False so that users may never sign up using
+ Allauth's signup form endpoints, to be on the safe side - since we only want users
+ to sign up using their Discord account.
+ """
+ return False
+
+
+class SocialAccountAdapter(DefaultSocialAccountAdapter):
+ """An Allauth SocialAccount adapter that prevents signups via non-Discord connections."""
+
+ def is_open_for_signup(self, request: HttpRequest, social_login: SocialLogin) -> bool:
+ """
+ Checks whether or not the site is open for signups.
+
+ We override this method in order to prevent users from creating a new account using
+ a non-Discord connection, as we require this connection for our users.
+ """
+ if social_login.account.provider != "discord":
+ add_message(request, ERROR, ERROR_CONNECT_DISCORD)
+
+ raise ImmediateHttpResponse(redirect(reverse("home")))
+
+ try:
+ user = DiscordUser.objects.get(id=int(social_login.account.uid))
+ except DiscordUser.DoesNotExist:
+ add_message(request, ERROR, ERROR_JOIN_DISCORD)
+
+ raise ImmediateHttpResponse(redirect(reverse("home")))
+
+ if user.roles.count() <= 1:
+ add_message(request, ERROR, ERROR_JOIN_DISCORD)
+
+ raise ImmediateHttpResponse(redirect(reverse("home")))
+
+ return True
+
+ def populate_user(self, request: HttpRequest,
+ social_login: SocialLogin,
+ data: Dict[str, Any]) -> DjangoUser:
+ """
+ Method used to populate a Django User with data.
+
+ We override this so that the Django user is created with the username#discriminator,
+ instead of just the username, as Django users must have unique usernames. For display
+ purposes, we also set the `name` key, which is used for `first_name` in the database.
+ """
+ if social_login.account.provider == "discord":
+ discriminator = social_login.account.extra_data["discriminator"]
+ data["username"] = f"{data['username']}#{discriminator}"
+ data["name"] = data["username"]
+
+ return super().populate_user(request, social_login, data)