From 82f3fd0876ff09bb11af3d8efa423725726e36ac Mon Sep 17 00:00:00 2001 From: Ronald Moesbergen Date: Wed, 7 Aug 2024 11:52:29 +0200 Subject: [PATCH] ci: add automated PR tests (#74) chore: pylint fixes --- .github/workflows/pull-request.yaml | 29 +++++++ Dockerfile | 5 +- aanmelden/src/admin.py | 8 +- aanmelden/src/api.py | 15 ++-- .../src/migrations/0021_alter_slot_name.py | 18 ++++ aanmelden/src/mixins.py | 2 +- aanmelden/src/models.py | 22 ++--- aanmelden/src/tests.py | 2 - aanmelden/src/utils.py | 12 +-- aanmelden/src/views.py | 85 +++++++++---------- debug.sh | 2 +- format.sh | 2 +- pyproject.toml | 22 +++++ 13 files changed, 147 insertions(+), 77 deletions(-) create mode 100644 .github/workflows/pull-request.yaml create mode 100644 aanmelden/src/migrations/0021_alter_slot_name.py create mode 100644 pyproject.toml diff --git a/.github/workflows/pull-request.yaml b/.github/workflows/pull-request.yaml new file mode 100644 index 0000000..f8f1c76 --- /dev/null +++ b/.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 diff --git a/Dockerfile b/Dockerfile index b1847e4..5aaada0 100644 --- a/Dockerfile +++ b/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 diff --git a/aanmelden/src/admin.py b/aanmelden/src/admin.py index 974f3e7..ba69094 100644 --- a/aanmelden/src/admin.py +++ b/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) diff --git a/aanmelden/src/api.py b/aanmelden/src/api.py index bce919d..898e393 100644 --- a/aanmelden/src/api.py +++ b/aanmelden/src/api.py @@ -1,7 +1,6 @@ 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 @@ -9,9 +8,13 @@ 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}) diff --git a/aanmelden/src/migrations/0021_alter_slot_name.py b/aanmelden/src/migrations/0021_alter_slot_name.py new file mode 100644 index 0000000..ecb0774 --- /dev/null +++ b/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), + ), + ] diff --git a/aanmelden/src/mixins.py b/aanmelden/src/mixins.py index 14f96e6..7942375 100644 --- a/aanmelden/src/mixins.py +++ b/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 diff --git a/aanmelden/src/models.py b/aanmelden/src/models.py index 37a72a1..9e7d352 100644 --- a/aanmelden/src/models.py +++ b/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): diff --git a/aanmelden/src/tests.py b/aanmelden/src/tests.py index 7ce503c..a39b155 100644 --- a/aanmelden/src/tests.py +++ b/aanmelden/src/tests.py @@ -1,3 +1 @@ -from django.test import TestCase - # Create your tests here. diff --git a/aanmelden/src/utils.py b/aanmelden/src/utils.py index 7da8332..6106ae1 100644 --- a/aanmelden/src/utils.py +++ b/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,7 +82,7 @@ def register_future(date, slot, user): try: presence.save() - except IntegrityError as e: + except IntegrityError: # Already registered -> ignore pass @@ -90,7 +90,7 @@ def register_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")) @@ -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")) diff --git a/aanmelden/src/views.py b/aanmelden/src/views.py index 2e78339..aea105d 100644 --- a/aanmelden/src/views.py +++ b/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")) diff --git a/debug.sh b/debug.sh index f3053e1..b5dbed4 100755 --- a/debug.sh +++ b/debug.sh @@ -1,6 +1,6 @@ #!/usr/bin/env sh -exec uvicorn \ +uvicorn \ --host 0.0.0.0 \ --port 8000 \ --log-level=info \ diff --git a/format.sh b/format.sh index e69ebda..f035988 100755 --- a/format.sh +++ b/format.sh @@ -1,3 +1,3 @@ #!/usr/bin/env sh -black aanmelden/*.py aanmelden/src/*.py \ No newline at end of file +black aanmelden diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..52a1663 --- /dev/null +++ b/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" +]