From dc426f0aa5d5f257d7c4d4cfd9f7e3add4b4593c Mon Sep 17 00:00:00 2001
From: Jonas L <jooola@users.noreply.github.com>
Date: Tue, 21 Jun 2022 23:43:03 +0200
Subject: [PATCH] feat(api): rename user model fields (#1902)

---
 api/libretime_api/core/models/__init__.py     |   2 +-
 api/libretime_api/core/models/role.py         |  17 ++-
 api/libretime_api/core/models/user.py         |  92 +++++++++-----
 api/libretime_api/core/serializers/user.py    |  13 +-
 .../core/tests/models/test_user.py            |  22 ++--
 api/libretime_api/permission_constants.py     |  13 +-
 api/libretime_api/permissions.py              |   8 +-
 api/libretime_api/tests/test_permissions.py   |  67 +++++++----
 api/schema.yml                                | 112 ++++++++++--------
 9 files changed, 204 insertions(+), 142 deletions(-)

diff --git a/api/libretime_api/core/models/__init__.py b/api/libretime_api/core/models/__init__.py
index 0071bc99a..0f8d92473 100644
--- a/api/libretime_api/core/models/__init__.py
+++ b/api/libretime_api/core/models/__init__.py
@@ -1,7 +1,7 @@
 from .auth import LoginAttempt, Session, UserToken
 from .country import Country
 from .preference import Preference, StreamSetting
-from .role import ADMIN, DJ, GUEST, PROGRAM_MANAGER, USER_TYPES
+from .role import Role
 from .service import ServiceRegister
 from .user import User, UserManager
 from .worker import CeleryTask, ThirdPartyTrackReference
diff --git a/api/libretime_api/core/models/role.py b/api/libretime_api/core/models/role.py
index fd5689d84..3710fe077 100644
--- a/api/libretime_api/core/models/role.py
+++ b/api/libretime_api/core/models/role.py
@@ -1,11 +1,8 @@
-GUEST = "G"
-DJ = "H"
-PROGRAM_MANAGER = "P"
-ADMIN = "A"
+from django.db import models
 
-USER_TYPES = {
-    GUEST: "Guest",
-    DJ: "DJ",
-    PROGRAM_MANAGER: "Program Manager",
-    ADMIN: "Admin",
-}
+
+class Role(models.TextChoices):
+    GUEST = "G", "Guest"
+    EDITOR = "H", "Editor"
+    MANAGER = "P", "Manager"
+    ADMIN = "A", "Admin"
diff --git a/api/libretime_api/core/models/user.py b/api/libretime_api/core/models/user.py
index 134547989..d20746e17 100644
--- a/api/libretime_api/core/models/user.py
+++ b/api/libretime_api/core/models/user.py
@@ -4,18 +4,14 @@ from django.contrib.auth.models import AbstractBaseUser, BaseUserManager, Permis
 from django.db import models
 
 from ...permission_constants import GROUPS
-from .role import ADMIN, USER_TYPES
-
-USER_TYPE_CHOICES = ()
-for item in USER_TYPES.items():
-    USER_TYPE_CHOICES = USER_TYPE_CHOICES + (item,)
+from .role import Role
 
 
 class UserManager(BaseUserManager):
-    def create_user(self, username, type, email, first_name, last_name, password):
+    def create_user(self, role, username, password, email, first_name, last_name):
         user = self.model(
+            role=role,
             username=username,
-            type=type,
             email=email,
             first_name=first_name,
             last_name=last_name,
@@ -24,33 +20,75 @@ class UserManager(BaseUserManager):
         user.save(using=self._db)
         return user
 
-    def create_superuser(self, username, email, first_name, last_name, password):
-        user = self.create_user(username, "A", email, first_name, last_name, password)
-        return user
+    def create_superuser(self, username, password, email, first_name, last_name):
+        return self.create_user(
+            Role.ADMIN,
+            username,
+            password,
+            email,
+            first_name,
+            last_name,
+        )
 
     def get_by_natural_key(self, username):
         return self.get(username=username)
 
 
 class User(AbstractBaseUser):
+    role = models.CharField(
+        db_column="type",
+        max_length=1,
+        choices=Role.choices,
+    )
+
     username = models.CharField(db_column="login", unique=True, max_length=255)
-    password = models.CharField(
-        db_column="pass", max_length=255
-    )  # Field renamed because it was a Python reserved word.
-    type = models.CharField(max_length=1, choices=USER_TYPE_CHOICES)
+    password = models.CharField(db_column="pass", max_length=255)
+    email = models.CharField(max_length=1024, blank=True, null=True)
     first_name = models.CharField(max_length=255)
     last_name = models.CharField(max_length=255)
-    last_login = models.DateTimeField(db_column="lastlogin", blank=True, null=True)
-    lastfail = models.DateTimeField(blank=True, null=True)
-    skype_contact = models.CharField(max_length=1024, blank=True, null=True)
-    jabber_contact = models.CharField(max_length=1024, blank=True, null=True)
-    email = models.CharField(max_length=1024, blank=True, null=True)
-    cell_phone = models.CharField(max_length=1024, blank=True, null=True)
-    login_attempts = models.IntegerField(blank=True, null=True)
+
+    login_attempts = models.IntegerField(
+        db_column="login_attempts",
+        blank=True,
+        null=True,
+    )
+    last_login = models.DateTimeField(
+        db_column="lastlogin",
+        blank=True,
+        null=True,
+    )
+    last_failed_login = models.DateTimeField(
+        db_column="lastfail",
+        blank=True,
+        null=True,
+    )
+
+    skype = models.CharField(
+        db_column="skype_contact",
+        max_length=1024,
+        blank=True,
+        null=True,
+    )
+    jabber = models.CharField(
+        db_column="jabber_contact",
+        max_length=1024,
+        blank=True,
+        null=True,
+    )
+    phone = models.CharField(
+        db_column="cell_phone",
+        max_length=1024,
+        blank=True,
+        null=True,
+    )
+
+    class Meta:
+        managed = False
+        db_table = "cc_subjs"
 
     USERNAME_FIELD = "username"
     EMAIL_FIELD = "email"
-    REQUIRED_FIELDS = ["type", "email", "first_name", "last_name"]
+    REQUIRED_FIELDS = ["role", "email", "first_name", "last_name"]
     objects = UserManager()
 
     def get_full_name(self):
@@ -66,7 +104,7 @@ class User(AbstractBaseUser):
             self.password = hashlib.md5(raw_password.encode()).hexdigest()
 
     def is_staff(self):
-        return self.type == ADMIN
+        return self.role == Role.ADMIN
 
     def check_password(self, raw_password):
         if self.has_usable_password():
@@ -81,7 +119,7 @@ class User(AbstractBaseUser):
     # django.contrib.auth.models.PermissionMixin.
 
     def is_superuser(self):
-        return self.type == ADMIN
+        return self.role == Role.ADMIN
 
     def get_user_permissions(self, obj=None):
         """
@@ -90,7 +128,7 @@ class User(AbstractBaseUser):
         return []
 
     def get_group_permissions(self, obj=None):
-        permissions = GROUPS[self.type]
+        permissions = GROUPS[self.role]
         if obj:
             obj_name = obj.__class__.__name__.lower()
             permissions = [perm for perm in permissions if obj_name in perm]
@@ -120,7 +158,3 @@ class User(AbstractBaseUser):
         for permission in perm_list:
             result = result and self.has_perm(permission, obj)
         return result
-
-    class Meta:
-        managed = False
-        db_table = "cc_subjs"
diff --git a/api/libretime_api/core/serializers/user.py b/api/libretime_api/core/serializers/user.py
index 445c82431..c8ddb4e30 100644
--- a/api/libretime_api/core/serializers/user.py
+++ b/api/libretime_api/core/serializers/user.py
@@ -7,14 +7,15 @@ class UserSerializer(serializers.HyperlinkedModelSerializer):
         model = get_user_model()
         fields = [
             "item_url",
+            "role",
             "username",
-            "type",
+            "email",
             "first_name",
             "last_name",
-            "lastfail",
-            "skype_contact",
-            "jabber_contact",
-            "email",
-            "cell_phone",
             "login_attempts",
+            "last_login",
+            "last_failed_login",
+            "skype",
+            "jabber",
+            "phone",
         ]
diff --git a/api/libretime_api/core/tests/models/test_user.py b/api/libretime_api/core/tests/models/test_user.py
index 49b4c88e1..4f4538782 100644
--- a/api/libretime_api/core/tests/models/test_user.py
+++ b/api/libretime_api/core/tests/models/test_user.py
@@ -1,16 +1,16 @@
 from rest_framework.test import APITestCase
 
 from ....permission_constants import GROUPS
-from ...models import DJ, GUEST, User
+from ...models import Role, User
 
 
 class TestUserManager(APITestCase):
     def test_create_user(self):
         user = User.objects.create_user(
-            "test",
-            email="test@example.com",
+            role=Role.EDITOR,
+            username="test",
             password="test",
-            type=DJ,
+            email="test@example.com",
             first_name="test",
             last_name="user",
         )
@@ -19,27 +19,29 @@ class TestUserManager(APITestCase):
 
     def test_create_superuser(self):
         user = User.objects.create_superuser(
-            "test",
-            email="test@example.com",
+            username="test",
             password="test",
+            email="test@example.com",
             first_name="test",
             last_name="user",
         )
         db_user = User.objects.get(pk=user.pk)
         self.assertEqual(db_user.username, user.username)
+        self.assertEqual(db_user.role, Role.ADMIN)
 
 
 class TestUser(APITestCase):
     def test_guest_get_group_perms(self):
         user = User.objects.create_user(
-            "test",
-            email="test@example.com",
+            role=Role.GUEST,
+            username="test",
             password="test",
-            type=GUEST,
+            email="test@example.com",
             first_name="test",
             last_name="user",
         )
+
         permissions = user.get_group_permissions()
         # APIRoot permission hardcoded in the check as it isn't a Permission object
         str_perms = [p.codename for p in permissions] + ["view_apiroot"]
-        self.assertCountEqual(str_perms, GROUPS[GUEST])
+        self.assertCountEqual(str_perms, GROUPS[Role.GUEST.value])
diff --git a/api/libretime_api/permission_constants.py b/api/libretime_api/permission_constants.py
index fedc3ae60..fee0f09e7 100644
--- a/api/libretime_api/permission_constants.py
+++ b/api/libretime_api/permission_constants.py
@@ -1,4 +1,4 @@
-from .core.models import DJ, GUEST, PROGRAM_MANAGER
+from .core.models import Role
 
 GUEST_PERMISSIONS = [
     "view_schedule",
@@ -19,7 +19,7 @@ GUEST_PERMISSIONS = [
     "view_apiroot",
 ]
 
-DJ_PERMISSIONS = GUEST_PERMISSIONS + [
+EDITOR_PERMISSIONS = GUEST_PERMISSIONS + [
     "add_file",
     "add_podcast",
     "add_podcastepisode",
@@ -50,7 +50,8 @@ DJ_PERMISSIONS = GUEST_PERMISSIONS + [
     "delete_own_smartblockcriteria",
     "delete_own_webstream",
 ]
-PROGRAM_MANAGER_PERMISSIONS = GUEST_PERMISSIONS + [
+
+MANAGER_PERMISSIONS = GUEST_PERMISSIONS + [
     "add_show",
     "add_showdays",
     "add_showhost",
@@ -98,7 +99,7 @@ PROGRAM_MANAGER_PERMISSIONS = GUEST_PERMISSIONS + [
 ]
 
 GROUPS = {
-    GUEST: GUEST_PERMISSIONS,
-    DJ: DJ_PERMISSIONS,
-    PROGRAM_MANAGER: PROGRAM_MANAGER_PERMISSIONS,
+    Role.GUEST.value: GUEST_PERMISSIONS,
+    Role.EDITOR.value: EDITOR_PERMISSIONS,
+    Role.MANAGER.value: MANAGER_PERMISSIONS,
 }
diff --git a/api/libretime_api/permissions.py b/api/libretime_api/permissions.py
index a478e98ab..f358dbb40 100644
--- a/api/libretime_api/permissions.py
+++ b/api/libretime_api/permissions.py
@@ -3,7 +3,7 @@ from secrets import compare_digest
 from django.conf import settings
 from rest_framework.permissions import BasePermission
 
-from .core.models.role import DJ
+from .core.models import Role
 
 REQUEST_PERMISSION_TYPE_MAP = {
     "GET": "view",
@@ -18,7 +18,7 @@ REQUEST_PERMISSION_TYPE_MAP = {
 
 def get_own_obj(request, view):
     user = request.user
-    if user is None or user.type != DJ:
+    if user is None or user.role != Role.EDITOR:
         return ""
     if request.method == "GET":
         return ""
@@ -67,12 +67,12 @@ class IsAdminOrOwnUser(BasePermission):
     """
 
     def has_permission(self, request, view):
-        if request.user.is_superuser:
+        if request.user.is_superuser():
             return True
         return False
 
     def has_object_permission(self, request, view, obj):
-        if request.user.is_superuser:
+        if request.user.is_superuser():
             return True
         return obj.username == request.user
 
diff --git a/api/libretime_api/tests/test_permissions.py b/api/libretime_api/tests/test_permissions.py
index 8a1ae58db..4a4799ab7 100644
--- a/api/libretime_api/tests/test_permissions.py
+++ b/api/libretime_api/tests/test_permissions.py
@@ -4,7 +4,7 @@ from django.contrib.auth.models import AnonymousUser
 from model_bakery import baker
 from rest_framework.test import APIRequestFactory, APITestCase
 
-from ..core.models import DJ, GUEST
+from ..core.models import Role
 from ..permissions import IsSystemTokenOrUser
 
 
@@ -51,18 +51,18 @@ class TestPermissions(APITestCase):
         "webstreams",
     ]
 
-    def logged_in_test_model(self, model, name, user_type, fn):
+    def logged_in_test_model(self, model, role, username, fn):
         path = self.path.format(model)
-        if not get_user_model().objects.filter(username=name):
+        if not get_user_model().objects.filter(username=username):
             get_user_model().objects.create_user(
-                name,
-                email="test@example.com",
+                role=role,
+                username=username,
                 password="test",
-                type=user_type,
+                email="test@example.com",
                 first_name="test",
                 last_name="user",
             )
-        self.client.login(username=name, password="test")
+        self.client.login(username=username, password="test")
         return fn(path)
 
     @classmethod
@@ -71,55 +71,74 @@ class TestPermissions(APITestCase):
 
     def test_guest_permissions_success(self):
         for model in self.URLS:
-            response = self.logged_in_test_model(model, "guest", GUEST, self.client.get)
+            response = self.logged_in_test_model(
+                model,
+                Role.GUEST,
+                "guest",
+                self.client.get,
+            )
             self.assertEqual(
-                response.status_code, 200, msg=f"Invalid for model {model}"
+                response.status_code,
+                200,
+                msg=f"Invalid for model {model}",
             )
 
     def test_guest_permissions_failure(self):
         for model in self.URLS:
             response = self.logged_in_test_model(
-                model, "guest", GUEST, self.client.post
+                model,
+                Role.GUEST,
+                "guest",
+                self.client.post,
             )
             self.assertEqual(
-                response.status_code, 403, msg=f"Invalid for model {model}"
+                response.status_code,
+                403,
+                msg=f"Invalid for model {model}",
             )
 
-    def test_dj_get_permissions(self):
+    def test_editor_get_permissions(self):
         for model in self.URLS:
-            response = self.logged_in_test_model(model, "dj", DJ, self.client.get)
+            response = self.logged_in_test_model(
+                model,
+                Role.EDITOR,
+                "editor",
+                self.client.get,
+            )
             self.assertEqual(
-                response.status_code, 200, msg=f"Invalid for model {model}"
+                response.status_code,
+                200,
+                msg=f"Invalid for model {model}",
             )
 
-    def test_dj_post_permissions(self):
+    def test_editor_post_permissions(self):
         user = get_user_model().objects.create_user(
-            "test-dj",
-            email="test@example.com",
+            role=Role.EDITOR,
+            username="editor2",
             password="test",
-            type=DJ,
+            email="test@example.com",
             first_name="test",
             last_name="user",
         )
         file = baker.make("storage.File", owner=user)
         model = f"files/{file.id}"
         path = self.path.format(model)
-        self.client.login(username="test-dj", password="test")
+        self.client.login(username="editor2", password="test")
         response = self.client.patch(path, {"name": "newFilename"})
         self.assertEqual(response.status_code, 200)
 
-    def test_dj_post_permissions_failure(self):
+    def test_editor_post_permissions_failure(self):
         get_user_model().objects.create_user(
-            "test-dj",
-            email="test@example.com",
+            role=Role.EDITOR,
+            username="editor2",
             password="test",
-            type=DJ,
+            email="test@example.com",
             first_name="test",
             last_name="user",
         )
         file = baker.make("storage.File")
         model = f"files/{file.id}"
         path = self.path.format(model)
-        self.client.login(username="test-dj", password="test")
+        self.client.login(username="editor2", password="test")
         response = self.client.patch(path, {"name": "newFilename"})
         self.assertEqual(response.status_code, 403)
diff --git a/api/schema.yml b/api/schema.yml
index 32f688b0f..40d67cc31 100644
--- a/api/schema.yml
+++ b/api/schema.yml
@@ -7468,42 +7468,46 @@ components:
           type: string
           format: uri
           readOnly: true
+        role:
+          $ref: "#/components/schemas/RoleEnum"
         username:
           type: string
           maxLength: 255
-        type:
-          $ref: "#/components/schemas/TypeEnum"
+        email:
+          type: string
+          nullable: true
+          maxLength: 1024
         first_name:
           type: string
           maxLength: 255
         last_name:
           type: string
           maxLength: 255
-        lastfail:
-          type: string
-          format: date-time
-          nullable: true
-        skype_contact:
-          type: string
-          nullable: true
-          maxLength: 1024
-        jabber_contact:
-          type: string
-          nullable: true
-          maxLength: 1024
-        email:
-          type: string
-          nullable: true
-          maxLength: 1024
-        cell_phone:
-          type: string
-          nullable: true
-          maxLength: 1024
         login_attempts:
           type: integer
           maximum: 2147483647
           minimum: -2147483648
           nullable: true
+        last_login:
+          type: string
+          format: date-time
+          nullable: true
+        last_failed_login:
+          type: string
+          format: date-time
+          nullable: true
+        skype:
+          type: string
+          nullable: true
+          maxLength: 1024
+        jabber:
+          type: string
+          nullable: true
+          maxLength: 1024
+        phone:
+          type: string
+          nullable: true
+          maxLength: 1024
     PatchedUserToken:
       type: object
       properties:
@@ -7886,6 +7890,13 @@ components:
         - item_url
         - key
         - user
+    RoleEnum:
+      enum:
+        - G
+        - H
+        - P
+        - A
+      type: string
     Schedule:
       type: object
       properties:
@@ -8429,13 +8440,6 @@ components:
       required:
         - code
         - item_url
-    TypeEnum:
-      enum:
-        - G
-        - H
-        - P
-        - A
-      type: string
     User:
       type: object
       properties:
@@ -8443,47 +8447,51 @@ components:
           type: string
           format: uri
           readOnly: true
+        role:
+          $ref: "#/components/schemas/RoleEnum"
         username:
           type: string
           maxLength: 255
-        type:
-          $ref: "#/components/schemas/TypeEnum"
+        email:
+          type: string
+          nullable: true
+          maxLength: 1024
         first_name:
           type: string
           maxLength: 255
         last_name:
           type: string
           maxLength: 255
-        lastfail:
-          type: string
-          format: date-time
-          nullable: true
-        skype_contact:
-          type: string
-          nullable: true
-          maxLength: 1024
-        jabber_contact:
-          type: string
-          nullable: true
-          maxLength: 1024
-        email:
-          type: string
-          nullable: true
-          maxLength: 1024
-        cell_phone:
-          type: string
-          nullable: true
-          maxLength: 1024
         login_attempts:
           type: integer
           maximum: 2147483647
           minimum: -2147483648
           nullable: true
+        last_login:
+          type: string
+          format: date-time
+          nullable: true
+        last_failed_login:
+          type: string
+          format: date-time
+          nullable: true
+        skype:
+          type: string
+          nullable: true
+          maxLength: 1024
+        jabber:
+          type: string
+          nullable: true
+          maxLength: 1024
+        phone:
+          type: string
+          nullable: true
+          maxLength: 1024
       required:
         - first_name
         - item_url
         - last_name
-        - type
+        - role
         - username
     UserToken:
       type: object