diff options
author | 2024-05-23 22:49:34 +0200 | |
---|---|---|
committer | 2024-05-23 21:49:34 +0100 | |
commit | 089b7f4504c8d1316c5ad97f019735a090848582 (patch) | |
tree | 0364567ae31d79b6fd866a6f82cd89bfa4d2178e /pydis_site/apps/api/viewsets | |
parent | Merge pull request #1325 from python-discord/dependabot/pip/ruff-0.4.5 (diff) |
Add alternate accounts to the user model
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..
Diffstat (limited to 'pydis_site/apps/api/viewsets')
-rw-r--r-- | pydis_site/apps/api/viewsets/bot/user.py | 183 |
1 files changed, 175 insertions, 8 deletions
diff --git a/pydis_site/apps/api/viewsets/bot/user.py b/pydis_site/apps/api/viewsets/bot/user.py index d06eb868..c0b4ca0f 100644 --- a/pydis_site/apps/api/viewsets/bot/user.py +++ b/pydis_site/apps/api/viewsets/bot/user.py @@ -1,5 +1,7 @@ -from collections import OrderedDict +from collections import ChainMap, OrderedDict +from django.core.exceptions import ObjectDoesNotExist +from django.db import IntegrityError, transaction from django.db.models import Q from django_filters.rest_framework import DjangoFilterBackend from rest_framework import fields, status @@ -13,8 +15,12 @@ from rest_framework.viewsets import ModelViewSet from pydis_site.apps.api.models.bot.infraction import Infraction from pydis_site.apps.api.models.bot.metricity import Metricity, NotFoundError -from pydis_site.apps.api.models.bot.user import User -from pydis_site.apps.api.serializers import UserSerializer +from pydis_site.apps.api.models.bot.user import User, UserAltRelationship +from pydis_site.apps.api.serializers import ( + UserSerializer, + UserAltRelationshipSerializer, + UserWithAltsSerializer +) class UserListPagination(PageNumberPagination): @@ -93,6 +99,17 @@ class UserViewSet(ModelViewSet): #### Response format >>> { + ... 'alts': [ + ... { + ... 'actor': 1234, + ... # Note that alt relationships are not transitive. + ... 'alts': [409107086526644234, 9012571029], + ... 'context': "Testing account", + ... 'source': 409107086526644234, + ... 'target': 128025 + ... }, + ... # ... + ... ], ... 'id': 409107086526644234, ... 'name': "python", ... 'display_name': "Python", @@ -208,8 +225,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 >>> { @@ -226,9 +244,61 @@ 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 + ### POST /bot/users/<snowflake:int>/alts + Add the alternative account given in the request body to the alternative + accounts for the given user snowflake. Users will be linked symmetrically. + + #### Request body + >>> { + ... # The target alternate account to associate the user (from the URL) with. + ... 'target': int, + ... # A description for why this relationship was established. + ... 'context': str, + ... # The moderator that associated the accounts together. + ... 'actor': int + ... } + + #### Status codes + - 204: returned on success + - 400: if the request body was invalid, including if the user in the + request body could not be found in the database + - 404: if the user with the given `snowflake` could not be found + + ### PATCH /bot/users/<snowflake:int>/alts + Update the context of the given alt account on the given user with the + context specified in the request body. + + #### Request body + >>> { + ... 'context': str, + ... 'target': int + ... } + + #### Status codes + - 204: returned on success + - 400: if the request body was invalid, including if the alternate + account could not be found as an associated account with the parent + user account record + - 404: if the user with the given `snowflake` could not be found + + ### DELETE /bot/users/<snowflake:int>/alts + Remove the alternative account given in the request body from the + alternative accounts of the given user snowflake. The request body contains + the user ID to remove from the association to this user, as a plain + integer. Users will be linked symmetrically. Returns the updated user. + + #### Request body + >>> int + + #### Status codes + - 204: returned on success + - 400: if the user in the request body was not found as an alt account + - 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 >>> [ @@ -271,10 +341,15 @@ class UserViewSet(ModelViewSet): filterset_fields = ('name', 'discriminator', 'display_name') def get_serializer(self, *args, **kwargs) -> ModelSerializer: - """Set Serializer many attribute to True if request body contains a list.""" + """Customize serializers used based on the requested action.""" if isinstance(kwargs.get('data', {}), list): kwargs['many'] = True + # If we are retrieving a single user, user the expanded serializer + # which also includes context information for each alternate account. + if self.action == 'retrieve' and self.detail: + return UserWithAltsSerializer(*args, **kwargs) + return super().get_serializer(*args, **kwargs) @action(detail=False, methods=["PATCH"], name='user-bulk-patch') @@ -292,6 +367,98 @@ class UserViewSet(ModelViewSet): return Response(serializer.data, status=status.HTTP_200_OK) + @action(detail=True, methods=['POST'], name="Add alternate account", + url_name='alts', url_path='alts') + def add_alt(self, request: Request, pk: str) -> Response: + """Associate the given account to the user's alternate accounts.""" + user_id = self.get_object().id + source_serializer_data = ChainMap({'source': user_id}, request.data) + source_serializer = UserAltRelationshipSerializer(data=source_serializer_data) + source_serializer.is_valid(raise_exception=True) + + # This code is somewhat vulnerable to a race condition, in case the first validation + # from above passes and directly before validating again, one of the users that + # are referenced here are deleted. Unfortunately, since Django on its own does not + # query in "both directions", we just need to live with a race condition for validation. + # For inserts atomicity is guaranteed as we run it in a transaction. + target_id = source_serializer.validated_data['target'].id + target_serializer_data = ChainMap({'source': target_id, 'target': user_id}, request.data) + target_serializer = UserAltRelationshipSerializer(data=target_serializer_data) + target_serializer.is_valid(raise_exception=True) # should not fail, or inconsistent db + + with transaction.atomic(): + try: + source_serializer.save() + target_serializer.save() + except IntegrityError as err: + if err.__cause__.diag.constraint_name == 'api_useraltrelationship_prevent_alt_to_self': + raise ParseError(detail={ + "source": ["The user may not be an alternate account of itself"] + }) + if err.__cause__.diag.constraint_name == 'api_useraltrelationship_unique_relationships': + raise ParseError(detail={ + "source": ["This relationship has already been established"] + }) + + raise # pragma: no cover + + return Response(status=status.HTTP_204_NO_CONTENT) + + # See https://www.django-rest-framework.org/api-guide/viewsets/#routing-additional-http-methods-for-extra-actions + # I would really like to include the alternate account to patch in the URL + # here, but unfortunately it's not possible to do so because the mapping + # does not accept arguments, and if I were to use the same @action decorator, + # it would overwrite the other methods and keep them from working. + @add_alt.mapping.patch + def update_alt(self, request: Request, pk: str) -> Response: + """Update the context of a single alt.""" + for field in ('target', 'context'): + if field not in request.data: + raise ParseError(detail={field: ["This field is required."]}) + + source = self.get_object() + target = request.data['target'] + with transaction.atomic(): + associated_accounts = ( + UserAltRelationship.objects + .filter(Q(source=source, target=target) | Q(source=target, target=source)) + .select_for_update() + ) + count = associated_accounts.count() + # Sanity check + assert count in (0, 2), f"Inconsistent database for alts of {source.id} -> {target}" + + if count == 0: + raise ParseError(detail={'target': ["User is not an associated alt account"]}) + + updated = associated_accounts.update(context=request.data['context']) + assert updated == 2 + + return Response(status=status.HTTP_204_NO_CONTENT) + + @add_alt.mapping.delete + def remove_alt(self, request: Request, pk: str) -> Response: + """Disassociate the given account from the user's alternate accounts.""" + user = self.get_object() + if not isinstance(request.data, int): + raise ParseError(detail={"non_field_errors": ["Request body should be an integer"]}) + try: + alt = user.alts.get(id=request.data) + except ObjectDoesNotExist: + raise ParseError(detail={ + 'non_field_errors': ["Specified account is not a known alternate account of this user"] + }) + + # It is mandatory that this query performs a symmetrical delete, + # because our custom ManyToManyField through model specifies more than + # two foreign keys to the user model, causing Django to no longer + # uphold the symmetry of the model. Correct function is verified by + # the tests, but in case you end up changing it, make sure that it + # works as expected! + user.alts.remove(alt) + + return Response(status=status.HTTP_204_NO_CONTENT) + @action(detail=True) def metricity_data(self, request: Request, pk: str | None = None) -> Response: """Request handler for metricity_data endpoint.""" |