ci: add automated PR tests (#74)

chore: pylint fixes

.github/workflows/pull-request.yaml

@@ -0,0 +1,29 @@
+name: Run PR Tests
+
+on:
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python 3.12
+        uses: actions/setup-python@v3
+        with:
+          python-version: "3.12"
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements.txt
+          pip install pylint==3.2.6 pylint-django==2.5.5 black==24.8.0
+
+      - name: Run pylint and black
+        run: |
+          black --check aanmelden
+          cp aanmelden/settings.example.py aanmelden/settings.py
+          DJANGO_SETTINGS_MODULE="aanmelden.settings" pylint --load-plugins=pylint_django aanmelden

Dockerfile

@@ -1,7 +1,4 @@
-FROM python:3.10-alpine
-
-# Set the file maintainer (your name - the file's author)
-MAINTAINER Ronald Moesbergen
+FROM python:3.12-alpine
 
 COPY requirements.txt /srv/aanmelden/requirements.txt
 

aanmelden/src/admin.py

@@ -1,6 +1,8 @@
 from django.contrib import admin
-from .models import Presence, SpecialDate, MacAddress, UserInfo, User, Slot
 from django.contrib.auth.admin import UserAdmin as BaseUserAdmin
+from django.contrib.auth import get_user_model
+
+from aanmelden.src.models import Presence, SpecialDate, MacAddress, UserInfo, Slot
 
 
 @admin.register(Presence)
@@ -23,8 +25,8 @@ class UserAdmin(BaseUserAdmin):
 
 
 # Re-register UserAdmin
-admin.site.unregister(User)
-admin.site.register(User, UserAdmin)
+admin.site.unregister(get_user_model())
+admin.site.register(get_user_model(), UserAdmin)
 
 admin.site.register(UserInfo)
 admin.site.register(SpecialDate)

aanmelden/src/api.py

@@ -1,17 +1,20 @@
 from datetime import date
 
 from django.conf import settings
-from django.contrib.auth.models import User
 from django.db.models import Value, F
 from django.db.models.functions import Concat
 from django.http import JsonResponse, HttpResponseBadRequest, HttpResponse
 from django.utils.decorators import method_decorator
 from django.views.decorators.csrf import csrf_exempt
 from django.views.generic import View
 
-from .mixins import ClientCredentialsRequiredMixin, SlotContextMixin, AuthenticatedMixin
-from .models import Presence, MacAddress, Slot, UserInfo
-from .utils import (
+from aanmelden.src.mixins import (
+    ClientCredentialsRequiredMixin,
+    SlotContextMixin,
+    AuthenticatedMixin,
+)
+from aanmelden.src.models import Presence, MacAddress, Slot, DjoUser
+from aanmelden.src.utils import (
     register,
     deregister,
     NotEnoughSlotsException,
@@ -127,7 +130,7 @@ def get(self, request):
             )
 
         members = list(
-            User.objects.filter(is_active=True)
+            DjoUser.objects.filter(is_active=True)
             .values("id")
             .annotate(
                 name=Concat("first_name", Value(" "), "last_name"),
@@ -174,7 +177,7 @@ def get(self, request, *args, **kwargs):
         if not self.request.user.is_superuser:
             return HttpResponse(status=403)
 
-        user = User.objects.get(id=int(kwargs.get("pk")))
+        user = DjoUser.objects.get(id=int(kwargs.get("pk")))
         register(self.slot, user, True)
         return JsonResponse({"ok": True})
 

aanmelden/src/migrations/0021_alter_slot_name.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.0.7 on 2024-08-07 09:24
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('src', '0020_userinfo_stripcard_count_userinfo_stripcard_used'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='slot',
+            name='name',
+            field=models.CharField(choices=[('mon', 'Maandag'), ('tue', 'Dinsdag'), ('wed', 'Woensdag'), ('thu', 'Donderdag'), ('fri', 'Vrijdag'), ('sat', 'Zaterdag'), ('sun', 'Zondag')], max_length=3),
+        ),
+    ]

aanmelden/src/mixins.py

@@ -37,7 +37,7 @@ def validate_client_token(self, client_token) -> bool:
                     token_url=settings.IDP_TOKEN_URL,
                     client_secret=settings.INTROSPECTION_CLIENT_SECRET,
                 )
-            except Exception as e:
+            except Exception as e:  # pylint: disable=broad-exception-caught
                 # Failed to get an introspection token -> bail
                 print(e)
                 return False

aanmelden/src/models.py

@@ -1,10 +1,11 @@
-from django.db import models
-from django.contrib.auth.models import User
+import datetime
+
 from django.conf import settings
+from django.contrib.auth import get_user_model
+from django.db import models
 from django.forms.models import model_to_dict
-import datetime
 
-DAY_NUMBERS = {"mon": 0, "tue": "1", "wed": 2, "thu": 3, "fri": 4, "sat": 5, "sun": 6}
+DAY_NUMBERS = {"mon": 0, "tue": 1, "wed": 2, "thu": 3, "fri": 4, "sat": 5, "sun": 6}
 
 POD_CHOICES = (
     ("m", "Ochtend"),
@@ -22,8 +23,10 @@
     ("sun", "Zondag"),
 )
 
+DjangoUser = get_user_model()
+
 
-class DjoUser(User):
+class DjoUser(DjangoUser):
     class Meta:
         proxy = True
         ordering = ["first_name", "last_name"]
@@ -47,14 +50,14 @@ def __str__(self):
 
 
 class UserInfo(models.Model):
-    user: DjoUser = models.OneToOneField(User, on_delete=models.CASCADE)
+    user: DjoUser = models.OneToOneField(get_user_model(), on_delete=models.CASCADE)
     days = models.IntegerField(default=1, null=False)
     account_type = models.CharField(max_length=100, default="", null=False)
     stripcard_used = models.IntegerField(default=0, null=False)
     stripcard_count = models.IntegerField(default=0, null=False)
 
     def __str__(self):
-        return f"User details for {self.user.first_name} {self.user.last_name}"
+        return f"User details for {self.user}"
 
 
 class Slot(models.Model):
@@ -129,7 +132,7 @@ def get_enabled_slots(user=None):
                 "tutors",
                 "announcement",
             ]:
-                available_slot[field] = slot.__getattribute__(field)
+                available_slot[field] = getattr(slot, field)
             if user:
                 available_slot["is_registered"] = slot.is_registered(user)
             available_slots.append(available_slot)
@@ -160,8 +163,7 @@ def get_tutor_count(date, pod=None):
             return Presence.objects.filter(
                 date=date, pod=pod, user__is_superuser=True
             ).count()
-        else:
-            return Presence.objects.filter(date=date, user__is_superuser=True).count()
+        return Presence.objects.filter(date=date, user__is_superuser=True).count()
 
     @staticmethod
     def slots_available(on_date, pod=None):

aanmelden/src/tests.py

@@ -1,3 +1 @@
-from django.test import TestCase
-
 # Create your tests here.

aanmelden/src/utils.py

@@ -58,7 +58,7 @@ def register(slot, user, skip_checks=False):
 
     try:
         presence.save()
-    except IntegrityError as e:
+    except IntegrityError:
         # Already registered -> ignore
         pass
 
@@ -82,15 +82,15 @@ def register_future(date, slot, user):
 
     try:
         presence.save()
-    except IntegrityError as e:
+    except IntegrityError:
         # Already registered -> ignore
         pass
 
     # only update if register date is in current week
     date_start = slot.date - timedelta(days=slot.date.weekday())
     date_end = date_start + timedelta(days=6)
 
-    if date >= date_start and date <= date_end:
+    if date_start <= date <= date_end:
         asyncio.run(sio.emit("update_report_page"))
         asyncio.run(sio.emit("update_main_page"))
 
@@ -126,8 +126,8 @@ def deregister(slot, user):
 
     if presence.seen:
         raise AlreadySeenException()
-    else:
-        presence.delete()
+
+    presence.delete()
 
     asyncio.run(sio.emit("update_report_page"))
     asyncio.run(sio.emit("update_main_page"))
@@ -145,7 +145,7 @@ def deregister_future(date, slot, user):
     date_start = slot.date - timedelta(days=slot.date.weekday())
     date_end = date_start + timedelta(days=6)
 
-    if date >= date_start and date <= date_end:
+    if date_start <= date <= date_end:
         asyncio.run(sio.emit("update_report_page"))
         asyncio.run(sio.emit("update_main_page"))
 

aanmelden/src/views.py

@@ -39,7 +39,7 @@ def get(self, request, *args, **kwargs):
             redirect_uri=settings.IDP_REDIRECT_URL,
             scope=settings.IDP_API_SCOPES,
         )
-        auth_url, state = oauth.authorization_url(settings.IDP_AUTHORIZE_URL)
+        auth_url, _ = oauth.authorization_url(settings.IDP_AUTHORIZE_URL)
         return HttpResponseRedirect(auth_url)
 
 
@@ -56,49 +56,48 @@ def get(self, request, *args, **kwargs):
                 authorization_response=full_response_url,
                 client_secret=settings.IDP_CLIENT_SECRET,
             )
-        except Exception:
+        except Exception:  # pylint: disable=broad-exception-caught
             # Something went wrong getting the token
-            return HttpResponseForbidden(f"Geen toegang!")
-
-        if "access_token" in access_token and access_token["access_token"] != "":
-            user_profile = oauth.get(settings.IDP_API_URL).json()
-            username = "idp-{0}".format(user_profile["id"])
-
-            try:
-                found_user = DjoUser.objects.get(username=username)
-            except DjoUser.DoesNotExist:
-                found_user = DjoUser()
-                found_user.username = username
-                found_user.set_unusable_password()
-
-            if not hasattr(found_user, "userinfo"):
-                found_user.userinfo = UserInfo()
-            found_user.email = user_profile["email"]
-            found_user.first_name = user_profile["firstName"]
-            found_user.last_name = user_profile["lastName"]
-            account_type = user_profile["accountType"]
-            found_user.is_superuser = DjoUser.is_begeleider(account_type)
-            found_user.userinfo.days = user_profile["days"]
-            found_user.userinfo.account_type = account_type
-            if "stripcard_used" in user_profile:
-                found_user.userinfo.stripcard_used = user_profile["stripcard_used"]
-                found_user.userinfo.stripcard_count = user_profile["stripcard_count"]
-            else:
-                # No active stripcard -> reset counters
-                found_user.userinfo.stripcard_used = 0
-                found_user.userinfo.stripcard_count = 0
-            found_user.save()
-            found_user.userinfo.save()
-
-            auth_login(request, found_user)
-
-            if found_user.is_superuser:
-                return HttpResponseRedirect(reverse("report"))
-            else:
-                return HttpResponseRedirect(reverse("main"))
-        else:
+            return HttpResponseForbidden("Geen toegang!")
+
+        if "access_token" not in access_token or access_token["access_token"] == "":
             return HttpResponseForbidden("IDP Login mislukt")
 
+        user_profile = oauth.get(settings.IDP_API_URL).json()
+        username = f"idp-{user_profile['id']}"
+
+        try:
+            found_user = DjoUser.objects.get(username=username)
+        except DjoUser.DoesNotExist:
+            found_user = DjoUser()
+            found_user.username = username
+            found_user.set_unusable_password()
+
+        if not hasattr(found_user, "userinfo"):
+            found_user.userinfo = UserInfo()
+        found_user.email = user_profile["email"]
+        found_user.first_name = user_profile["firstName"]
+        found_user.last_name = user_profile["lastName"]
+        account_type = user_profile["accountType"]
+        found_user.is_superuser = DjoUser.is_begeleider(account_type)
+        found_user.userinfo.days = user_profile["days"]
+        found_user.userinfo.account_type = account_type
+        if "stripcard_used" in user_profile:
+            found_user.userinfo.stripcard_used = user_profile["stripcard_used"]
+            found_user.userinfo.stripcard_count = user_profile["stripcard_count"]
+        else:
+            # No active stripcard -> reset counters
+            found_user.userinfo.stripcard_used = 0
+            found_user.userinfo.stripcard_count = 0
+        found_user.save()
+        found_user.userinfo.save()
+
+        auth_login(request, found_user)
+
+        if found_user.is_superuser:
+            return HttpResponseRedirect(reverse("report"))
+        return HttpResponseRedirect(reverse("main"))
+
 
 class LogoffView(LoginRequiredMixin, View):
     def get(self, request, *args, **kwargs):
@@ -111,7 +110,7 @@ def get(self, request, *args, **kwargs):
 class Main(LoginRequiredMixin, TemplateView):
     template_name = "main.html"
 
-    def get_context_data(self, *, object_list=None, **kwargs):
+    def get_context_data(self, **kwargs):
         context = super().get_context_data()
         context.update({"slots": Slot.get_enabled_slots(self.request.user)})
         return context
@@ -248,7 +247,7 @@ def post(self, request, *args, **kwargs):
 
         try:
             response = super().post(request, *args, **kwargs)
-        except IntegrityError as e:
+        except IntegrityError:
             # Already registered -> ignore
             response = HttpResponseRedirect(reverse("report"))
 

debug.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env sh
 
-exec uvicorn \
+uvicorn \
   --host 0.0.0.0 \
   --port 8000 \
   --log-level=info \

format.sh

@@ -1,3 +1,3 @@
 #!/usr/bin/env sh
 
-black aanmelden/*.py aanmelden/src/*.py
+black aanmelden

pyproject.toml

@@ -0,0 +1,22 @@
+[tool.black]
+force-exclude = '''
+aanmelden/src/migrations/
+'''
+
+[tool.pylint.format]
+max-line-length = 120
+
+[tool.pylint.MASTER]
+load-plugins = 'pylint_django'
+disable = [
+    "missing-module-docstring",
+    "missing-class-docstring",
+    "missing-function-docstring",
+    "line-too-long",
+    "too-few-public-methods",
+    "too-many-ancestors",
+]
+ignore = [
+    "migrations/*",
+    "settings.example.py"
+]