Merge pull request #236 from python-discord/django-roles-api-add-position

Add position field to Role model

8 files changed, 321 insertions, 10 deletions

diff --git a/pydis_site/apps/api/migrations/0039_add_position_field_to_role.py b/pydis_site/apps/api/migrations/0039_add_position_field_to_role.py
new file mode 100644
index 00000000..b6b27ff2
--- /dev/null
+++ b/pydis_site/apps/api/migrations/0039_add_position_field_to_role.py
@@ -0,0 +1,23 @@
+# Generated by Django 2.2.3 on 2019-08-15 11:42
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0038_merge_20190719_1817'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='role',
+            name='position',
+            field=models.IntegerField(default=-1, help_text='The position of the role in the role hierarchy of the Discord Guild.'),
+        ),
+        migrations.AlterField(
+            model_name='role',
+            name='position',
+            field=models.IntegerField(help_text='The position of the role in the role hierarchy of the Discord Guild.'),
+        )
+    ]
diff --git a/pydis_site/apps/api/models/bot/role.py b/pydis_site/apps/api/models/bot/role.py
index 34e74009..58bbf8b4 100644
--- a/pydis_site/apps/api/models/bot/role.py
+++ b/pydis_site/apps/api/models/bot/role.py
@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from django.core.validators import MaxValueValidator, MinValueValidator
 from django.db import models
 
@@ -5,7 +7,12 @@ from pydis_site.apps.api.models.utils import ModelReprMixin
 
 
 class Role(ModelReprMixin, models.Model):
-    """A role on our Discord server."""
+    """
+    A role on our Discord server.
+
+    The comparison operators <, <=, >, >=, ==, != act the same as they do with Role-objects of the
+    discord.py library, see https://discordpy.readthedocs.io/en/latest/api.html#discord.Role
+    """
 
     id = models.BigIntegerField(
         primary_key=True,
@@ -43,7 +50,18 @@ class Role(ModelReprMixin, models.Model):
         ),
         help_text="The integer value of the permission bitset of this role from Discord."
     )
+    position = models.IntegerField(
+        help_text="The position of the role in the role hierarchy of the Discord Guild."
+    )
 
-    def __str__(self):
+    def __str__(self) -> str:
         """Returns the name of the current role, for display purposes."""
         return self.name
+
+    def __lt__(self, other: Role) -> bool:
+        """Compares the roles based on their position in the role hierarchy of the guild."""
+        return self.position < other.position
+
+    def __le__(self, other: Role) -> bool:
+        """Compares the roles based on their position in the role hierarchy of the guild."""
+        return self.position <= other.position
diff --git a/pydis_site/apps/api/models/bot/user.py b/pydis_site/apps/api/models/bot/user.py
index d4deb630..00c24d3d 100644
--- a/pydis_site/apps/api/models/bot/user.py
+++ b/pydis_site/apps/api/models/bot/user.py
@@ -51,3 +51,15 @@ 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}"
+
+    @property
+    def top_role(self) -> Role:
+        """
+        Attribute that returns the user's top role.
+
+        This will fall back to the Developers role if the user does not have any roles.
+        """
+        roles = self.roles.all()
+        if not roles:
+            return Role.objects.get(name="Developers")
+        return max(self.roles.all())
diff --git a/pydis_site/apps/api/serializers.py b/pydis_site/apps/api/serializers.py
index 24533dd6..40ca755d 100644
--- a/pydis_site/apps/api/serializers.py
+++ b/pydis_site/apps/api/serializers.py
@@ -196,7 +196,7 @@ class RoleSerializer(ModelSerializer):
         """Metadata defined for the Django REST Framework."""
 
         model = Role
-        fields = ('id', 'name', 'colour', 'permissions')
+        fields = ('id', 'name', 'colour', 'permissions', 'position')
 
 
 class TagSerializer(ModelSerializer):
diff --git a/pydis_site/apps/api/tests/test_models.py b/pydis_site/apps/api/tests/test_models.py
index cfc8464e..2120b056 100644
--- a/pydis_site/apps/api/tests/test_models.py
+++ b/pydis_site/apps/api/tests/test_models.py
@@ -51,7 +51,8 @@ class StringDunderMethodTests(SimpleTestCase):
             OffTopicChannelName(name='bob-the-builders-playground'),
             Role(
                 id=5, name='test role',
-                colour=0x5, permissions=0
+                colour=0x5, permissions=0,
+                position=10,
             ),
             Message(
                 id=45,
diff --git a/pydis_site/apps/api/tests/test_roles.py b/pydis_site/apps/api/tests/test_roles.py
new file mode 100644
index 00000000..0a6cea9e
--- /dev/null
+++ b/pydis_site/apps/api/tests/test_roles.py
@@ -0,0 +1,191 @@
+from django_hosts.resolvers import reverse
+
+from .base import APISubdomainTestCase
+from ..models import Role
+
+
+class CreationTests(APISubdomainTestCase):
+    @classmethod
+    def setUpTestData(cls):  # noqa
+        cls.admins_role = Role.objects.create(
+            id=1,
+            name="Admins",
+            colour=1,
+            permissions=1,
+            position=4,
+        )
+        cls.developers_role = Role.objects.create(
+            id=4,
+            name="Helpers",
+            colour=4,
+            permissions=4,
+            position=1,
+        )
+        cls.everyone_role = Role.objects.create(
+            id=5,
+            name="@everyone",
+            colour=5,
+            permissions=5,
+            position=0,
+        )
+        cls.lowest_position_duplicate = Role.objects.create(
+            id=6,
+            name="lowest position duplicate",
+            colour=6,
+            permissions=6,
+            position=0,
+        )
+
+    def _validate_roledict(self, role_dict: dict) -> None:
+        """Helper method to validate a dict representing a role."""
+        self.assertIsInstance(role_dict, dict)
+        self.assertEqual(len(role_dict), 5)
+        attributes = ('id', 'name', 'colour', 'permissions', 'position')
+        self.assertTrue(all(attribute in role_dict for attribute in attributes))
+
+    def test_role_ordering_lt(self):
+        """Tests the __lt__ comparisons based on role position in the hierarchy."""
+        self.assertTrue(self.everyone_role < self.developers_role)
+        self.assertFalse(self.developers_role > self.admins_role)
+
+    def test_role_ordering_le(self):
+        """Tests the __le__ comparisons based on role position in the hierarchy."""
+        self.assertTrue(self.everyone_role <= self.developers_role)
+        self.assertTrue(self.everyone_role <= self.lowest_position_duplicate)
+        self.assertTrue(self.everyone_role >= self.lowest_position_duplicate)
+        self.assertTrue(self.developers_role >= self.everyone_role)
+
+        self.assertFalse(self.developers_role >= self.admins_role)
+        self.assertFalse(self.developers_role <= self.everyone_role)
+
+    def test_role_min_max_ordering(self):
+        """Tests the `min` and `max` operations based on the role hierarchy."""
+        top_role_no_duplicates = max([self.developers_role, self.admins_role, self.everyone_role])
+        self.assertIs(top_role_no_duplicates, self.admins_role)
+
+        top_role_duplicates = max([self.developers_role, self.admins_role, self.admins_role])
+        self.assertIs(top_role_duplicates, self.admins_role)
+
+        bottom_role_no_duplicates = min(
+            [self.developers_role, self.admins_role, self.everyone_role]
+        )
+        self.assertIs(bottom_role_no_duplicates, self.everyone_role)
+
+        bottom_role_duplicates = min(
+            [self.lowest_position_duplicate, self.admins_role, self.everyone_role]
+        )
+        self.assertIs(bottom_role_duplicates, self.lowest_position_duplicate)
+
+    def test_role_list(self):
+        """Tests the GET list-view and validates the contents."""
+        url = reverse('bot:role-list', host='api')
+
+        response = self.client.get(url)
+        self.assertContains(response, text="id", count=4, status_code=200)
+
+        roles = response.json()
+        self.assertIsInstance(roles, list)
+        self.assertEqual(len(roles), 4)
+
+        for role in roles:
+            self._validate_roledict(role)
+
+    def test_role_get_detail_success(self):
+        """Tests GET detail view of an existing role."""
+        url = reverse('bot:role-detail', host='api', args=(self.admins_role.id, ))
+        response = self.client.get(url)
+        self.assertContains(response, text="id", count=1, status_code=200)
+
+        role = response.json()
+        self._validate_roledict(role)
+
+        admins_role = Role.objects.get(id=role["id"])
+        self.assertEqual(admins_role.name, role["name"])
+        self.assertEqual(admins_role.colour, role["colour"])
+        self.assertEqual(admins_role.permissions, role["permissions"])
+        self.assertEqual(admins_role.position, role["position"])
+
+    def test_role_post_201(self):
+        """Tests creation of a role with a valid request."""
+        url = reverse('bot:role-list', host='api')
+        data = {
+            "id": 1234567890,
+            "name": "Role Creation Test",
+            "permissions": 0b01010010101,
+            "colour": 1,
+            "position": 10,
+        }
+        response = self.client.post(url, data=data)
+        self.assertEqual(response.status_code, 201)
+
+    def test_role_post_invalid_request_body(self):
+        """Tests creation of a role with an invalid request body."""
+        url = reverse('bot:role-list', host='api')
+        data = {
+            "name": "Role Creation Test",
+            "permissions": 0b01010010101,
+            "colour": 1,
+            "position": 10,
+        }
+        response = self.client.post(url, data=data)
+        self.assertEqual(response.status_code, 400)
+        self.assertJSONEqual(response.content, '{"id": ["This field is required."]}')
+
+    def test_role_put_200(self):
+        """Tests PUT role request with valid request body."""
+        url = reverse('bot:role-detail', host='api', args=(self.admins_role.id,))
+        data = {
+            "id": 123454321,
+            "name": "Role Put Alteration Test",
+            "permissions": 255,
+            "colour": 999,
+            "position": 20,
+        }
+
+        response = self.client.put(url, data=data)
+        self.assertEqual(response.status_code, 200)
+
+        admins_role = Role.objects.get(id=data["id"])
+        self.assertEqual(admins_role.name, data["name"])
+        self.assertEqual(admins_role.permissions, data["permissions"])
+        self.assertEqual(admins_role.colour, data["colour"])
+        self.assertEqual(admins_role.position, data["position"])
+
+    def test_role_put_invalid_request_body(self):
+        """Tests PUT role request with invalid request body."""
+        url = reverse('bot:role-detail', host='api', args=(self.admins_role.id,))
+        data = {
+            "name": "Role Put Alteration Test",
+            "permissions": 255,
+            "colour": 999,
+            "position": 20,
+        }
+        response = self.client.put(url, data=data)
+        self.assertEqual(response.status_code, 400)
+
+    def test_role_patch_200(self):
+        """Tests PATCH role request with valid request body."""
+        url = reverse('bot:role-detail', host='api', args=(self.admins_role.id,))
+        data = {
+            "name": "Owners"
+        }
+        response = self.client.patch(url, data=data)
+        self.assertEqual(response.status_code, 200)
+
+        admins_role = Role.objects.get(id=self.admins_role.id)
+        self.assertEqual(admins_role.name, data["name"])
+
+    def test_role_delete_200(self):
+        """Tests DELETE requests for existing role."""
+        url = reverse('bot:role-detail', host='api', args=(self.admins_role.id,))
+        response = self.client.delete(url)
+        self.assertEqual(response.status_code, 204)
+
+    def test_role_detail_404_all_methods(self):
+        """Tests detail view with non-existing ID."""
+        url = reverse('bot:role-detail', host='api', args=(20190815,))
+
+        for method in ('get', 'put', 'patch', 'delete'):
+            response = getattr(self.client, method)(url)
+            self.assertEqual(response.status_code, 404)
+            self.assertJSONEqual(response.content, '{"detail": "Not found."}')
diff --git a/pydis_site/apps/api/tests/test_users.py b/pydis_site/apps/api/tests/test_users.py
index 90bc3d30..bbdd3ff4 100644
--- a/pydis_site/apps/api/tests/test_users.py
+++ b/pydis_site/apps/api/tests/test_users.py
@@ -41,7 +41,8 @@ class CreationTests(APISubdomainTestCase):
             id=5,
             name="Test role pls ignore",
             colour=2,
-            permissions=0b01010010101
+            permissions=0b01010010101,
+            position=1
         )
 
     def test_accepts_valid_data(self):
@@ -119,3 +120,57 @@ class CreationTests(APISubdomainTestCase):
 
         response = self.client.post(url, data=data)
         self.assertEqual(response.status_code, 400)
+
+
+class UserModelTests(APISubdomainTestCase):
+    @classmethod
+    def setUpTestData(cls):  # noqa
+        cls.role_top = Role.objects.create(
+            id=777,
+            name="High test role",
+            colour=2,
+            permissions=0b01010010101,
+            position=10,
+        )
+        cls.role_bottom = Role.objects.create(
+            id=888,
+            name="Low test role",
+            colour=2,
+            permissions=0b01010010101,
+            position=1,
+        )
+        cls.developers_role = Role.objects.create(
+            id=1234567,
+            name="Developers",
+            colour=1234,
+            permissions=0b01010010101,
+            position=2,
+        )
+        cls.user_with_roles = User.objects.create(
+            id=1,
+            avatar_hash="coolavatarhash",
+            name="Test User with two roles",
+            discriminator=1111,
+            in_guild=True,
+        )
+        cls.user_with_roles.roles.add(cls.role_bottom, cls.role_top)
+
+        cls.user_without_roles = User.objects.create(
+            id=2,
+            avatar_hash="coolavatarhash",
+            name="Test User without roles",
+            discriminator=2222,
+            in_guild=True,
+        )
+
+    def test_correct_top_role_property_user_with_roles(self):
+        """Tests if the top_role property returns the correct role."""
+        top_role = self.user_with_roles.top_role
+        self.assertIsInstance(top_role, Role)
+        self.assertEqual(top_role.id, self.role_top.id)
+
+    def test_correct_top_role_property_user_without_roles(self):
+        """Tests if the top_role property returns the correct role."""
+        top_role = self.user_without_roles.top_role
+        self.assertIsInstance(top_role, Role)
+        self.assertEqual(top_role.id, self.developers_role.id)
diff --git a/pydis_site/apps/api/viewsets/bot/role.py b/pydis_site/apps/api/viewsets/bot/role.py
index 213f0a19..1f82208d 100644
--- a/pydis_site/apps/api/viewsets/bot/role.py
+++ b/pydis_site/apps/api/viewsets/bot/role.py
@@ -20,7 +20,8 @@ class RoleViewSet(ModelViewSet):
     ...         'id': 267628507062992896,
     ...         'name': "Admins",
     ...         'colour': 1337,
-    ...         'permissions': 8
+    ...         'permissions': 8,
+    ...         'position': 1
     ...     }
     ... ]
 
@@ -35,7 +36,8 @@ class RoleViewSet(ModelViewSet):
     ...     'id': 267628507062992896,
     ...     'name': "Admins",
     ...     'colour': 1337,
-    ...     'permissions': 8
+    ...     'permissions': 8,
+    ...     'position': 1
     ... }
 
     #### Status codes
@@ -51,6 +53,7 @@ class RoleViewSet(ModelViewSet):
     ...     'name': str,
     ...     'colour': int,
     ...     'permissions': int,
+    ...     'position': 1,
     ... }
 
     #### Status codes
@@ -66,24 +69,32 @@ class RoleViewSet(ModelViewSet):
     ...     'id': int,
     ...     'name': str,
     ...     'colour': int,
-    ...     'permissions': int
+    ...     'permissions': int,
+    ...     'position': 1,
     ... }
 
     #### Status codes
     - 200: returned on success
     - 400: if the request body was invalid
+    - 404: if a role with the given `snowflake` does not exist
 
     ### PATCH /bot/roles/<snowflake:int>
     Update the role with the given `snowflake`.
-    All fields in the request body are required.
 
+    #### Request body
     >>> {
     ...     'id': int,
     ...     'name': str,
     ...     'colour': int,
-    ...     'permissions': int
+    ...     'permissions': int,
+    ...     'position': 1,
     ... }
 
+    #### Status codes
+    - 200: returned on success
+    - 400: if the request body was invalid
+    - 404: if a role with the given `snowflake` does not exist
+
     ### DELETE /bot/roles/<snowflake:int>
     Deletes the role with the given `snowflake`.