Skip to content

Commit

Permalink
Add alternate accounts to the user model
Browse files Browse the repository at this point in the history
Introduce a way to store alternate accounts on the user, and add the
`PATCH /bot/users/<id:str>/alts` endpoint, which allows updating the
user's alt accounts to the alt accounts in the request..
  • Loading branch information
jchristgit committed Dec 12, 2023
1 parent 6ec6fff commit 54ab4af
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 7 deletions.
37 changes: 37 additions & 0 deletions pydis_site/apps/api/migrations/0093_user_alts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Generated by Django 5.0 on 2023-12-12 09:01

import django.db.models.deletion
import pydis_site.apps.api.models.mixins
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('api', '0092_remove_redirect_filter_list'),
]

operations = [
migrations.CreateModel(
name='UserAltRelationship',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('source', models.ForeignKey(help_text='The source of this user to alternate account relationship', on_delete=django.db.models.deletion.CASCADE, to='api.user', verbose_name='Source')),
('target', models.ForeignKey(help_text='The target of this user to alternate account relationship', on_delete=django.db.models.deletion.CASCADE, related_name='+', to='api.user', verbose_name='Target')),
],
bases=(pydis_site.apps.api.models.mixins.ModelReprMixin, models.Model),
),
migrations.AddField(
model_name='user',
name='alts',
field=models.ManyToManyField(help_text='Known alternate accounts of this user. Manually linked.', through='api.UserAltRelationship', to='api.user', verbose_name='Alternative accounts'),
),
migrations.AddConstraint(
model_name='useraltrelationship',
constraint=models.UniqueConstraint(fields=('source', 'target'), name='api_useraltrelationship_unique_relationships'),
),
migrations.AddConstraint(
model_name='useraltrelationship',
constraint=models.CheckConstraint(check=models.Q(('source', models.F('target')), _negated=True), name='api_useraltrelationship_prevent_alt_to_self'),
),
]
37 changes: 37 additions & 0 deletions pydis_site/apps/api/models/bot/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ class User(ModelReprMixin, models.Model):
help_text="Whether this user is in our server.",
verbose_name="In Guild"
)
alts = models.ManyToManyField(
'self',
through='UserAltRelationship',
help_text="Known alternate accounts of this user. Manually linked.",
verbose_name="Alternative accounts"
)

def __str__(self):
"""Returns the name and discriminator for the current user, for display purposes."""
Expand All @@ -86,3 +92,34 @@ def username(self) -> str:
For usability in read-only fields such as Django Admin.
"""
return str(self)

class UserAltRelationship(ModelReprMixin, models.Model):
"""A relationship between a Discord user and its alts."""

source = models.ForeignKey(
User,
on_delete=models.CASCADE,
verbose_name="Source",
help_text="The source of this user to alternate account relationship",
)
target = models.ForeignKey(
User,
on_delete=models.CASCADE,
verbose_name="Target",
related_name='+',
help_text="The target of this user to alternate account relationship",
)

class Meta:
"""Add constraints to prevent users from being an alt of themselves."""

constraints = [
models.UniqueConstraint(
name="%(app_label)s_%(class)s_unique_relationships",
fields=["source", "target"]
),
models.CheckConstraint(
name="%(app_label)s_%(class)s_prevent_alt_to_self",
check=~models.Q(source=models.F("target")),
),
]
6 changes: 4 additions & 2 deletions pydis_site/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ def update(self, queryset: QuerySet, validated_data: list) -> list:
return updated


class UserSerializer(ModelSerializer):
class UserSerializer(FrozenFieldsMixin, ModelSerializer):
"""A class providing (de-)serialization of `User` instances."""

# ID field must be explicitly set as the default id field is read-only.
Expand All @@ -679,7 +679,9 @@ class Meta:
"""Metadata defined for the Django REST Framework."""

model = User
fields = ('id', 'name', 'discriminator', 'roles', 'in_guild')
fields = ('id', 'name', 'discriminator', 'roles', 'in_guild', 'alts')
# This should be edited on a separate endpoint only
frozen_fields = ('alts',)
depth = 1
list_serializer_class = UserListSerializer

Expand Down
2 changes: 1 addition & 1 deletion pydis_site/apps/api/tests/test_infractions.py
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ def setUpTestData(cls):
def check_expanded_fields(self, infraction):
for key in ('user', 'actor'):
obj = infraction[key]
for field in ('id', 'name', 'discriminator', 'roles', 'in_guild'):
for field in ('id', 'name', 'discriminator', 'roles', 'in_guild', 'alts'):
self.assertTrue(field in obj, msg=f'field "{field}" missing from {key}')

def test_list_expanded(self):
Expand Down
120 changes: 120 additions & 0 deletions pydis_site/apps/api/tests/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def setUpTestData(cls):
def test_accepts_valid_data(self):
url = reverse('api:bot:user-list')
data = {
'alts': [],
'id': 42,
'name': "Test",
'discriminator': 42,
Expand Down Expand Up @@ -199,13 +200,15 @@ def test_multiple_users_patch(self):
url = reverse("api:bot:user-bulk-patch")
data = [
{
"alts": [],
"id": 1,
"name": "User 1 patched!",
"discriminator": 1010,
"roles": [self.role_developer.id],
"in_guild": False
},
{
"alts": [],
"id": 2,
"name": "User 2 patched!"
}
Expand Down Expand Up @@ -646,3 +649,120 @@ def test_search_lookup_of_unknown_user(self) -> None:
result = response.json()
self.assertEqual(result['count'], 0)
self.assertEqual(result['results'], [])


class UserAltUpdateTests(AuthenticatedAPITestCase):
@classmethod
def setUpTestData(cls):
cls.user_1 = User.objects.create(
id=12095219,
name=f"Test user {random.randint(100, 1000)}",
discriminator=random.randint(1, 9999),
in_guild=True,
)
cls.user_2 = User.objects.create(
id=18259125,
name=f"Test user {random.randint(100, 1000)}",
discriminator=random.randint(1, 9999),
in_guild=True,
)

def test_adding_existing_alts(self) -> None:
url = reverse('api:bot:user-alts', args=(self.user_1.id,))
data = [self.user_2.id]
response = self.client.patch(url, data)
self.assertEqual(response.status_code, 200)

updated_user = response.json()
self.assertEqual(len(updated_user['alts']), 1)
self.assertEqual(updated_user['alts'][0]['id'], self.user_2.id)

self.user_1.refresh_from_db()
self.user_2.refresh_from_db()

self.assertQuerySetEqual(self.user_1.alts.all(), [self.user_2])
self.assertQuerySetEqual(self.user_2.alts.all(), [self.user_1])

def test_removing_existing_alts(self) -> None:
self.user_1.alts.set((self.user_2.id,))
self.user_1.save()

data = []
url = reverse('api:bot:user-alts', args=(self.user_1.id,))
response = self.client.patch(url, data)

self.assertEqual(response.status_code, 200)
updated_user = response.json()
self.assertEqual(updated_user['alts'], [])

self.user_1.refresh_from_db()
self.user_2.refresh_from_db()

self.assertFalse(self.user_1.alts.all().exists())
self.assertFalse(self.user_2.alts.all().exists())

def test_adding_alt_to_user_that_does_not_exist(self) -> None:
"""Patching a user's alts for a user that doesn't exist should return a 404."""
url = reverse('api:bot:user-alts', args=(self.user_1.id + self.user_2.id,))
data = [self.user_2.id]
response = self.client.patch(url, data)
self.assertEqual(response.status_code, 404)

def test_adding_alt_that_does_not_exist_to_user(self) -> None:
"""Patching a user's alts with an alt that is unknown should return a 400."""
url = reverse('api:bot:user-alts', args=(self.user_1.id,))
data = [self.user_1.id + self.user_2.id]
response = self.client.patch(url, data)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json(), {
'non_field_errors': ["One or more unknown user IDs were passed"]
})

def test_cannot_add_self_as_alt_account(self) -> None:
"""The existing account may not be an alt of itself."""
url = reverse('api:bot:user-alts', args=(self.user_1.id,))
data = [self.user_1.id]
response = self.client.patch(url, data)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json(), {
'non_field_errors': ["The user may not be an alternate account of itself"]
})

def test_cannot_update_alts_on_regular_user_patch_route(self) -> None:
"""The regular user update route does not allow editing the alts."""
url = reverse('api:bot:user-detail', args=(self.user_1.id,))
data = {'alts': [self.user_2.id]}
response = self.client.patch(url, data)
self.assertEqual(response.status_code, 200) # XXX: This seems to be a DRF bug
self.assertEqual(response.json()['alts'], [])

self.user_1.refresh_from_db()
self.assertQuerySetEqual(self.user_1.alts.all(), ())
self.user_2.refresh_from_db()
self.assertQuerySetEqual(self.user_2.alts.all(), ())

def test_cannot_update_alts_on_bulk_user_patch_route(self) -> None:
"""The bulk user update route does not allow editing the alts."""
url = reverse('api:bot:user-bulk-patch')
data = [{'id': self.user_1.id, 'alts': [self.user_2.id]}]
response = self.client.patch(url, data)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json(), {'non_field_errors': ['Insufficient data provided.']})

self.user_1.refresh_from_db()
self.assertQuerySetEqual(self.user_1.alts.all(), ())
self.user_2.refresh_from_db()
self.assertQuerySetEqual(self.user_2.alts.all(), ())

def test_user_bulk_patch_does_not_discard_alts(self) -> None:
"""The bulk user update route should not modify the alts."""
url = reverse('api:bot:user-bulk-patch')
self.user_1.alts.set((self.user_2,))
data = [{'id': self.user_1.id, 'name': "Joe Armstrong"}]
response = self.client.patch(url, data)
self.assertEqual(response.status_code, 200)

self.user_1.refresh_from_db()
self.assertQuerySetEqual(self.user_1.alts.all(), (self.user_2,))
self.user_2.refresh_from_db()
self.assertQuerySetEqual(self.user_2.alts.all(), (self.user_1,))
49 changes: 45 additions & 4 deletions pydis_site/apps/api/viewsets/bot/user.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from collections import OrderedDict

from django.db import IntegrityError
from django.db.models import Q
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework import fields, status
Expand Down Expand Up @@ -203,8 +204,9 @@ class UserViewSet(ModelViewSet):
- 404: if the user with the given `snowflake` could not be found
### PATCH /bot/users/<snowflake:int>
Update the user with the given `snowflake`.
All fields in the request body are optional.
Update the user with the given `snowflake`. All fields in the request body
are optional. Note that editing the `'alts'` field is not possible this
way, use the `alts` endpoint documented below for this.
#### Request body
>>> {
Expand All @@ -220,9 +222,24 @@ class UserViewSet(ModelViewSet):
- 400: if the request body was invalid, see response body for details
- 404: if the user with the given `snowflake` could not be found
### PATCH /bot/users/<snowflake:int>/alts
Update the alternative accounts for the given user snowflake. The request
body contains the user IDs to link to this user, as a plain list. IDs will
be linked symmetrically. Returns the updated user.
#### Request body
>>> [int, ...]
#### Status codes
- 200: returned on success
- 400: if the request body was invalid, including if one of the users in
the request body could not be found in the database
- 404: if the user with the given `snowflake` could not be found
### BULK PATCH /bot/users/bulk_patch
Update users with the given `ids` and `details`.
`id` field and at least one other field is mandatory.
Update users with the given `ids` and `details`. `id` field and at least
one other field is mandatory. Note that editing the `'alts'` field is not
possible using this endpoint.
#### Request body
>>> [
Expand Down Expand Up @@ -284,6 +301,30 @@ def bulk_patch(self, request: Request) -> Response:

return Response(serializer.data, status=status.HTTP_200_OK)

@action(detail=True, methods=['PATCH'], name='user-alts-update', url_name='alts', url_path='alts')
def update_alts(self, request: Request, pk: str) -> Response:
"""Update the user's alternate accounts to the given list."""
user = self.get_object()
if not isinstance(request.data, list):
raise ParseError(detail={"non_field_errors": ["Request body should be a list"]})
if not all(isinstance(item, int) for item in request.data):
raise ParseError(detail={"non_field_errors": ["List payload should only contain numbers"]})
users = User.objects.filter(id__in=request.data)
if len(users) != len(request.data):
raise ParseError(detail={"non_field_errors": ["One or more unknown user IDs were passed"]})
try:
user.alts.set(users)
except IntegrityError as err:
if err.__cause__.diag.constraint_name == 'api_useraltrelationship_prevent_alt_to_self':
raise ParseError(detail={
"non_field_errors": ["The user may not be an alternate account of itself"]
})

raise

serializer = self.get_serializer(instance=user)
return Response(serializer.data, status=status.HTTP_200_OK)

@action(detail=True)
def metricity_data(self, request: Request, pk: str | None = None) -> Response:
"""Request handler for metricity_data endpoint."""
Expand Down

0 comments on commit 54ab4af

Please sign in to comment.