diff --git a/evap/cms/json_importer.py b/evap/cms/json_importer.py index 7fae5decf1..cadc7c0fc9 100644 --- a/evap/cms/json_importer.py +++ b/evap/cms/json_importer.py @@ -168,7 +168,7 @@ def get_log(self) -> str: return log - def send_mail(self): + def send_mail(self) -> None: subject = "[EvaP] JSON importer log" recipients = settings.JSON_IMPORTER_LOG_RECIPIENTS diff --git a/evap/cms/models.py b/evap/cms/models.py index 7efef3272e..ba2802b628 100644 --- a/evap/cms/models.py +++ b/evap/cms/models.py @@ -23,7 +23,7 @@ class Meta: verbose_name = _("ignored evaluation") verbose_name_plural = _("ignored evaluations") - def __str__(self): + def __str__(self) -> str: if self.name: return f"{self.course.name} – {self.name}" return self.course.name diff --git a/evap/cms/views.py b/evap/cms/views.py index dfa6088dc5..a04b926120 100644 --- a/evap/cms/views.py +++ b/evap/cms/views.py @@ -3,6 +3,7 @@ from evap.cms.models import IgnoredEvaluation from evap.evaluation.auth import manager_required from evap.evaluation.tools import ( + HttpRequest, HttpResponseNoContent, get_object_from_dict_pk_entry_or_logged_40x, ) @@ -10,7 +11,7 @@ @require_POST @manager_required -def ignored_evaluation_delete(request): +def ignored_evaluation_delete(request: HttpRequest) -> HttpResponseNoContent: ignored_evaluation = get_object_from_dict_pk_entry_or_logged_40x( IgnoredEvaluation, request.POST, "ignored_evaluation_id" ) diff --git a/evap/context_processors.py b/evap/context_processors.py index e09f01e8cb..21e2b22381 100644 --- a/evap/context_processors.py +++ b/evap/context_processors.py @@ -1,26 +1,27 @@ import random from django.conf import settings +from django.http import HttpRequest from django.utils.translation import get_language from evap.evaluation.forms import NotebookForm -def slogan(request): +def slogan(request: HttpRequest): if get_language() == "de": return {"slogan": random.choice(settings.SLOGANS_DE)} # nosec return {"slogan": random.choice(settings.SLOGANS_EN)} # nosec -def debug(request): +def debug(request: HttpRequest): return {"debug": settings.DEBUG} -def notebook_form(request): +def notebook_form(request: HttpRequest): if request.user.is_authenticated: return {"notebook_form": NotebookForm(instance=request.user)} return {} -def allow_anonymous_feedback_messages(request): +def allow_anonymous_feedback_messages(request: HttpRequest): return {"allow_anonymous_feedback_messages": settings.ALLOW_ANONYMOUS_FEEDBACK_MESSAGES} diff --git a/evap/development/views.py b/evap/development/views.py index d83ce28518..b69f12f371 100644 --- a/evap/development/views.py +++ b/evap/development/views.py @@ -1,10 +1,10 @@ from django.conf import settings from django.core.exceptions import BadRequest -from django.http import HttpResponse +from django.http import HttpRequest, HttpResponse from django.shortcuts import render -def development_components(request): +def development_components(request: HttpRequest) -> HttpResponse: theme_colors = ["primary", "secondary", "success", "info", "warning", "danger", "light", "dark"] template_data = { "theme_colors": theme_colors, @@ -13,7 +13,7 @@ def development_components(request): return render(request, "development_components.html", template_data) -def development_rendered(request, filename): +def development_rendered(request: HttpRequest, filename: str) -> HttpResponse: fixtures_directory = settings.STATICFILES_DIRS[0] / "ts" / "rendered" try: with open(fixtures_directory / filename, encoding="utf-8") as fixture: diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index 581e8b9ae4..28fd033b63 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -1,11 +1,13 @@ import inspect from collections.abc import Callable, Iterable from functools import wraps +from typing import Any from django.conf import settings from django.contrib.auth.backends import ModelBackend from django.core.exceptions import PermissionDenied from django.core.mail import EmailMessage +from django.http import HttpRequest from django.utils.decorators import method_decorator from mozilla_django_oidc.auth import OIDCAuthenticationBackend @@ -25,7 +27,7 @@ class RequestAuthUserBackend(ModelBackend): # Having a different method signature is okay according to django documentation: # https://docs.djangoproject.com/en/3.0/topics/auth/customizing/#writing-an-authentication-backend - def authenticate(self, request, key): # pylint: disable=arguments-differ + def authenticate(self, request: HttpRequest, key: int) -> UserProfile | None: # type: ignore[override] # pylint: disable=arguments-differ if not key: return None @@ -37,7 +39,9 @@ def authenticate(self, request, key): # pylint: disable=arguments-differ class EmailAuthenticationBackend(ModelBackend): # https://docs.djangoproject.com/en/3.1/topics/auth/customizing/#writing-an-authentication-backend - def authenticate(self, request, email=None, password=None): # pylint: disable=arguments-differ,arguments-renamed + def authenticate( # type: ignore[override] # pylint: disable=arguments-differ,arguments-renamed + self, request: HttpRequest | None, email: str | None = None, password: str | None = None + ) -> UserProfile | None: assert password_login_is_active() try: user = UserProfile.objects.get(email=email) @@ -60,7 +64,8 @@ def class_or_function_check_decorator(test_func: Callable[[UserProfile], bool]): def function_decorator(func): @wraps(func) - def wrapped(request, *args, **kwargs): + def wrapped(request: HttpRequest, *args, **kwargs) -> Any: + assert isinstance(request.user, UserProfile) if not test_func(request.user): raise PermissionDenied return func(request, *args, **kwargs) @@ -79,57 +84,57 @@ def decorator(class_or_function): @class_or_function_check_decorator -def internal_required(user): +def internal_required(user: UserProfile) -> bool: return not user.is_external @class_or_function_check_decorator -def staff_permission_required(user): +def staff_permission_required(user: UserProfile) -> bool: return user.has_staff_permission @class_or_function_check_decorator -def manager_required(user): +def manager_required(user: UserProfile) -> bool: return user.is_manager @class_or_function_check_decorator -def reviewer_required(user): +def reviewer_required(user: UserProfile) -> bool: return user.is_reviewer @class_or_function_check_decorator -def grade_publisher_required(user): +def grade_publisher_required(user: UserProfile) -> bool: return user.is_grade_publisher @class_or_function_check_decorator -def grade_publisher_or_manager_required(user): +def grade_publisher_or_manager_required(user: UserProfile) -> bool: return user.is_grade_publisher or user.is_manager @class_or_function_check_decorator -def grade_downloader_required(user): +def grade_downloader_required(user: UserProfile) -> bool: return user.can_download_grades @class_or_function_check_decorator -def responsible_or_contributor_or_delegate_required(user): +def responsible_or_contributor_or_delegate_required(user: UserProfile) -> bool: return user.is_responsible_or_contributor_or_delegate @class_or_function_check_decorator -def editor_or_delegate_required(user): +def editor_or_delegate_required(user: UserProfile) -> bool: return user.is_editor_or_delegate @class_or_function_check_decorator -def participant_required(user): +def participant_required(user: UserProfile) -> bool: return user.is_participant @class_or_function_check_decorator -def reward_user_required(user): +def reward_user_required(user: UserProfile) -> bool: return can_reward_points_be_used_by(user) @@ -142,13 +147,13 @@ def possible_existing_account_emails(email: str) -> Iterable[str]: if previous_domain := settings.OIDC_EMAIL_TRANSITIONS.get(domain): yield f"{name}@{previous_domain}" - def get_userinfo(self, *args, **kwargs): + def get_userinfo(self, *args, **kwargs) -> dict[str, Any]: claims = super().get_userinfo(*args, **kwargs) if "email" in claims: claims["email"] = clean_email(claims["email"]) return claims - def filter_users_by_claims(self, claims): + def filter_users_by_claims(self, claims: dict[str, Any]) -> list[UserProfile]: assert openid_login_is_active() email = claims.get("email") if not email: @@ -162,7 +167,7 @@ def filter_users_by_claims(self, claims): return [] - def create_user(self, claims): + def create_user(self, claims: dict[str, Any]) -> UserProfile: assert openid_login_is_active() return self.UserModel.objects.create( email=claims.get("email"), @@ -170,7 +175,7 @@ def create_user(self, claims): last_name=claims.get("family_name", ""), ) - def update_user(self, user, claims): + def update_user(self, user: UserProfile, claims: dict[str, Any]) -> UserProfile: assert openid_login_is_active() if not user.first_name_given: user.first_name_given = claims.get("given_name", "") diff --git a/evap/evaluation/forms.py b/evap/evaluation/forms.py index 99b3c7717d..add92a32da 100644 --- a/evap/evaluation/forms.py +++ b/evap/evaluation/forms.py @@ -4,11 +4,13 @@ from django.conf import settings from django.contrib.auth import authenticate from django.core.exceptions import ValidationError +from django.http import HttpRequest from django.utils.translation import gettext_lazy as _ from django.views.decorators.debug import sensitive_variables from evap.evaluation.models import Evaluation, UserProfile from evap.results.tools import STATES_WITH_RESULTS_CACHING, cache_results +from evap.tools import assert_not_none logger = logging.getLogger(__name__) @@ -19,7 +21,7 @@ class LoginEmailForm(forms.Form): email = forms.CharField(label=_("Email"), max_length=254, widget=forms.EmailInput(attrs={"autofocus": True})) password = forms.CharField(label=_("Password"), widget=forms.PasswordInput) - def __init__(self, request, *args, **kwargs): + def __init__(self, request: HttpRequest, *args, **kwargs) -> None: """ If request is passed in, the form will validate that cookies are enabled. Note that the request (a HttpRequest object) must have set a @@ -27,13 +29,13 @@ def __init__(self, request, *args, **kwargs): running this validation. """ self.request = request - self.user_cache = None + self.user_cache: UserProfile | None = None super().__init__(*args, **kwargs) @sensitive_variables("password") - def clean_password(self): - email = self.cleaned_data.get("email") - password = self.cleaned_data.get("password") + def clean_password(self) -> str | None: + email: str = self.cleaned_data["email"] + password: str | None = self.cleaned_data.get("password") email = email.lower() @@ -44,31 +46,31 @@ def clean_password(self): self.check_for_test_cookie() return password - def check_for_test_cookie(self): + def check_for_test_cookie(self) -> None: if self.request and not self.request.session.test_cookie_worked(): raise forms.ValidationError( _("Your Web browser doesn't appear to have cookies enabled. Cookies are required for logging in.") ) - def get_user_id(self): + def get_user_id(self) -> int | None: if self.user_cache: return self.user_cache.id return None - def get_user(self): + def get_user(self) -> UserProfile | None: return self.user_cache class NewKeyForm(forms.Form): email = forms.EmailField(label=_("Email address")) - def __init__(self, *args, **kwargs): - self.user_cache = None + def __init__(self, *args, **kwargs) -> None: + self.user_cache: UserProfile | None = None super().__init__(*args, **kwargs) - def clean_email(self): - email = self.cleaned_data.get("email") + def clean_email(self) -> str: + email: str = assert_not_none(self.cleaned_data.get("email")) if not UserProfile.email_needs_login_key(email): raise forms.ValidationError( @@ -90,19 +92,19 @@ def clean_email(self): return email - def get_user(self): + def get_user(self) -> UserProfile | None: return self.user_cache class UserModelChoiceField(forms.ModelChoiceField): - def label_from_instance(self, obj): + def label_from_instance(self, obj: UserProfile) -> str: return obj.full_name_with_additional_info class UserModelMultipleChoiceField(forms.ModelMultipleChoiceField): widget = forms.SelectMultiple(attrs={"data-tomselect-fullwidth": ""}) - def label_from_instance(self, obj): + def label_from_instance(self, obj: UserProfile) -> str: return obj.full_name_with_additional_info @@ -118,12 +120,12 @@ class Meta: "delegates": UserModelMultipleChoiceField, } - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) for field in ("title", "first_name_given", "last_name", "email"): self.fields[field].disabled = True - def save(self, *args, **kwargs): + def save(self, *args, **kwargs) -> None: super().save(*args, **kwargs) if "first_name_chosen" in self.changed_data: @@ -138,7 +140,7 @@ def save(self, *args, **kwargs): logger.info('User "%s" edited the settings.', self.instance.email) - def clean_first_name_chosen(self): + def clean_first_name_chosen(self) -> str: name = self.cleaned_data["first_name_chosen"] for character in name: @@ -149,7 +151,7 @@ def clean_first_name_chosen(self): class NotebookForm(forms.ModelForm): - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self.fields["notes"].widget.attrs.update({"class": "notebook-textarea"}) diff --git a/evap/evaluation/management/commands/format.py b/evap/evaluation/management/commands/format.py index 92dfac1b24..1901ce16b2 100644 --- a/evap/evaluation/management/commands/format.py +++ b/evap/evaluation/management/commands/format.py @@ -1,4 +1,5 @@ import subprocess # nosec +from argparse import ArgumentParser from django.core.management.base import BaseCommand @@ -8,24 +9,24 @@ class Command(BaseCommand): help = "Runs code formatting" requires_migrations_checks = False - def add_arguments(self, parser): + def add_arguments(self, parser: ArgumentParser) -> None: parser.add_argument( "formatter", nargs="?", choices=["ruff", "prettier", "python"], help="Specify a formatter to run." ) - def run_ruff(self): + def run_ruff(self) -> None: self.stdout.write("Executing ruff format .") subprocess.run(["ruff", "format", "."], check=False) # nosec self.stdout.write("Executing ruff check --select I --fix .") subprocess.run(["ruff", "check", "--select", "I", "--fix", "."], check=False) # nosec - def run_prettier(self): + def run_prettier(self) -> None: self.stdout.write("Executing npx prettier") subprocess.run( ["npx", "prettier", "--write", "evap/static/ts/**/*.ts", "evap/static/ts/eslint.config.js"], check=False ) # nosec - def handle(self, *args, **options): + def handle(self, *args, **options) -> None: if options["formatter"] in ("ruff", "python", None): self.run_ruff() if options["formatter"] in ("prettier", None): diff --git a/evap/evaluation/management/commands/lint.py b/evap/evaluation/management/commands/lint.py index e0f8a2fd6f..731f650f0a 100644 --- a/evap/evaluation/management/commands/lint.py +++ b/evap/evaluation/management/commands/lint.py @@ -1,4 +1,5 @@ import subprocess # nosec +from argparse import ArgumentParser from django.core.management.base import BaseCommand @@ -7,24 +8,24 @@ class Command(BaseCommand): help = "Runs the code linter" requires_migrations_checks = False - def add_arguments(self, parser): + def add_arguments(self, parser: ArgumentParser) -> None: parser.add_argument( "linter", nargs="?", choices=["ruff", "pylint", "eslint", "python"], help="Specify a linter to run." ) - def run_ruff(self): + def run_ruff(self) -> None: self.stdout.write("Executing ruff check .") subprocess.run(["ruff", "check", "."], check=False) # nosec - def run_pylint(self): + def run_pylint(self) -> None: self.stdout.write("Executing pylint evap") subprocess.run(["pylint", "evap", "tools"], check=False) # nosec - def run_eslint(self): + def run_eslint(self) -> None: self.stdout.write("Executing npx eslint --quiet") subprocess.run(["npx", "eslint", "--quiet"], cwd="evap/static/ts", check=False) # nosec - def handle(self, *args, **options): + def handle(self, *args, **options) -> None: if options["linter"] in ("ruff", "python", None): self.run_ruff() if options["linter"] in ("pylint", "python", None): diff --git a/evap/evaluation/management/commands/tools.py b/evap/evaluation/management/commands/tools.py index c8d87c4334..07f6122c6e 100644 --- a/evap/evaluation/management/commands/tools.py +++ b/evap/evaluation/management/commands/tools.py @@ -8,7 +8,7 @@ logger = logging.getLogger(__name__) -def confirm_harmful_operation(output): +def confirm_harmful_operation(output) -> bool: """Usage: Abort if it does not return true""" if input("Are you sure you want to continue? (yes/no) ") != "yes": diff --git a/evap/evaluation/middleware.py b/evap/evaluation/middleware.py index ed6ba2466f..3e6b83e327 100644 --- a/evap/evaluation/middleware.py +++ b/evap/evaluation/middleware.py @@ -1,4 +1,7 @@ import uuid +from collections.abc import Callable + +from django.http import HttpRequest, HttpResponseBase from evap.evaluation.models_logging import LoggedModel @@ -12,10 +15,10 @@ class LoggingRequestMiddleware: Taken from https://github.com/treyhunner/django-simple-history/ """ - def __init__(self, get_response): + def __init__(self, get_response: Callable[[HttpRequest], HttpResponseBase]) -> None: self.get_response = get_response - def __call__(self, request): + def __call__(self, request: HttpRequest) -> HttpResponseBase: LoggedModel.thread.request = request LoggedModel.thread.request_id = str(uuid.uuid4()) diff --git a/evap/evaluation/models.py b/evap/evaluation/models.py index e558def6f3..e6a380b2fc 100644 --- a/evap/evaluation/models.py +++ b/evap/evaluation/models.py @@ -8,8 +8,7 @@ from datetime import date, datetime, time, timedelta from enum import Enum, auto from functools import partial -from numbers import Real -from typing import Any +from typing import TYPE_CHECKING, Any, cast from django.conf import settings from django.contrib import messages @@ -50,7 +49,12 @@ translate, vote_end_datetime, ) -from evap.tools import date_to_datetime +from evap.tools import assert_not_none, date_to_datetime + +if TYPE_CHECKING: + import builtins + + from evap.grades.models import GradeDocument logger = logging.getLogger(__name__) @@ -99,11 +103,11 @@ class Meta: verbose_name = _("semester") verbose_name_plural = _("semesters") - def __str__(self): + def __str__(self) -> str: return self.name @property - def can_be_deleted_by_manager(self): + def can_be_deleted_by_manager(self) -> bool: if self.is_active: return False @@ -113,21 +117,21 @@ def can_be_deleted_by_manager(self): return self.participations_are_archived and self.grade_documents_are_deleted and self.results_are_archived @property - def participations_can_be_archived(self): + def participations_can_be_archived(self) -> bool: return not self.participations_are_archived and all( evaluation.participations_can_be_archived for evaluation in self.evaluations.all() ) @property - def grade_documents_can_be_deleted(self): + def grade_documents_can_be_deleted(self) -> bool: return not self.grade_documents_are_deleted @property - def results_can_be_archived(self): + def results_can_be_archived(self) -> bool: return not self.results_are_archived @transaction.atomic - def archive(self): + def archive(self) -> None: if not self.participations_can_be_archived: raise NotArchivableError for evaluation in self.evaluations.all(): @@ -136,7 +140,7 @@ def archive(self): self.save() @transaction.atomic - def delete_grade_documents(self): + def delete_grade_documents(self) -> None: # Resolving this circular dependency makes the code more ugly, so we leave it. from evap.grades.models import GradeDocument # noqa: PLC0415 @@ -146,24 +150,24 @@ def delete_grade_documents(self): self.grade_documents_are_deleted = True self.save() - def archive_results(self): + def archive_results(self) -> None: if not self.results_can_be_archived: raise NotArchivableError self.results_are_archived = True self.save() @classmethod - def get_all_with_published_unarchived_results(cls): + def get_all_with_published_unarchived_results(cls) -> "QuerySet[Semester]": return cls.objects.filter( courses__evaluations__state=Evaluation.State.PUBLISHED, results_are_archived=False ).distinct() @classmethod - def active_semester(cls): + def active_semester(cls) -> "Semester | None": return cls.objects.filter(is_active=True).first() @property - def evaluations(self): + def evaluations(self) -> "QuerySet[Evaluation]": return Evaluation.objects.filter(course__semester=self) @@ -229,16 +233,16 @@ class Meta: verbose_name = _("questionnaire") verbose_name_plural = _("questionnaires") - def __str__(self): + def __str__(self) -> str: return self.name - def __lt__(self, other): + def __lt__(self, other: "Questionnaire") -> bool: return (self.type, self.order, self.pk) < (other.type, other.order, other.pk) - def __gt__(self, other): + def __gt__(self, other: "Questionnaire") -> bool: return (self.type, self.order, self.pk) > (other.type, other.order, other.pk) - def clean(self): + def clean(self) -> None: if self.type == self.Type.CONTRIBUTOR and self.is_locked: raise ValidationError({"is_locked": _("Contributor questionnaires cannot be locked.")}) @@ -259,7 +263,7 @@ def is_general(self) -> bool: return self.type in (self.Type.TOP, self.Type.BOTTOM) @property - def can_be_edited_by_manager(self): + def can_be_edited_by_manager(self) -> bool: if is_prefetched(self, "contributions"): if all(is_prefetched(contribution, "evaluation") for contribution in self.contributions.all()): return all( @@ -269,7 +273,7 @@ def can_be_edited_by_manager(self): return not self.contributions.exclude(evaluation__state=Evaluation.State.NEW).exists() @property - def can_be_deleted_by_manager(self): + def can_be_deleted_by_manager(self) -> bool: return not self.contributions.exists() @property @@ -294,10 +298,10 @@ class Program(models.Model): class Meta: ordering = ["order"] - def __str__(self): + def __str__(self) -> str: return self.name - def can_be_deleted_by_manager(self): + def can_be_deleted_by_manager(self) -> bool: if self.pk is None: return True return not self.courses.all().exists() @@ -319,10 +323,10 @@ class CourseType(models.Model): class Meta: ordering = ["order"] - def __str__(self): + def __str__(self) -> str: return self.name - def can_be_deleted_by_manager(self): + def can_be_deleted_by_manager(self) -> bool: if not self.pk: return True return not self.courses.all().exists() @@ -344,7 +348,7 @@ class ExamType(models.Model): class Meta: ordering = ["order"] - def __str__(self): + def __str__(self) -> str: return self.name def can_be_deleted_by_manager(self) -> bool: @@ -392,11 +396,11 @@ class Meta: verbose_name = _("course") verbose_name_plural = _("courses") - def __str__(self): + def __str__(self) -> str: return self.name @classmethod - def objects_with_missing_final_grades(cls): + def objects_with_missing_final_grades(cls) -> "QuerySet[Course]": from evap.grades.models import GradeDocument # noqa: PLC0415 return ( @@ -410,41 +414,41 @@ def objects_with_missing_final_grades(cls): ) @property - def unlogged_fields(self): + def unlogged_fields(self) -> list[str]: return super().unlogged_fields + ["semester", "gets_no_grade_documents"] @property - def can_be_edited_by_manager(self): + def can_be_edited_by_manager(self) -> bool: return not self.semester.participations_are_archived @property - def can_be_deleted_by_manager(self): + def can_be_deleted_by_manager(self) -> bool: return not self.evaluations.exists() @property - def final_grade_documents(self): + def final_grade_documents(self) -> "QuerySet[GradeDocument]": # We think it's better to use the imported constant here instead of using some workaround from evap.grades.models import GradeDocument # noqa: PLC0415 return self.grade_documents.filter(type=GradeDocument.Type.FINAL_GRADES) @property - def midterm_grade_documents(self): + def midterm_grade_documents(self) -> "QuerySet[GradeDocument]": # We think it's better to use the imported constant here instead of using some workaround from evap.grades.models import GradeDocument # noqa: PLC0415 return self.grade_documents.filter(type=GradeDocument.Type.MIDTERM_GRADES) @cached_property - def responsibles_names(self): + def responsibles_names(self) -> str: return ", ".join(responsible.full_name for responsible in self.responsibles.all()) @property - def has_external_responsible(self): + def has_external_responsible(self) -> bool: return any(responsible.is_external for responsible in self.responsibles.all()) @property - def all_evaluations_finished(self): + def all_evaluations_finished(self) -> bool: if is_prefetched(self, "evaluations"): return all(evaluation.state >= Evaluation.State.EVALUATED for evaluation in self.evaluations.all()) @@ -540,15 +544,15 @@ class State(models.IntegerChoices): staff_notes = models.TextField(verbose_name=_("staff notes"), blank=True) @property - def has_exam_evaluation(self): + def has_exam_evaluation(self) -> bool: return self.course.evaluations.filter(exam_type__isnull=False).exists() @property - def earliest_possible_exam_date(self): + def earliest_possible_exam_date(self) -> date: return self.vote_start_datetime.date() + timedelta(days=1) @transaction.atomic - def create_exam_evaluation(self, exam_date: date, exam_type: ExamType): + def create_exam_evaluation(self, exam_date: date, exam_type: ExamType) -> None: self.weight = settings.MAIN_EVALUATION_DEFAULT_WEIGHT self.vote_end_date = exam_date - timedelta(days=1) self.save() @@ -597,10 +601,10 @@ class Meta: ), ] - def __str__(self): + def __str__(self) -> str: return self.full_name - def save(self, *args, **kw): + def save(self, *args, **kw) -> None: super().save(*args, **kw) # make sure there is a general contribution @@ -610,10 +614,10 @@ def save(self, *args, **kw): if hasattr(self, "state_change_source"): - def state_changed_to(self, state_set): + def state_changed_to(self: Any, state_set: Collection[Evaluation.State]) -> bool: return self.state_change_source not in state_set and self.state in state_set - def state_changed_from(self, state_set): + def state_changed_from(self: Any, state_set: Collection[Evaluation.State]) -> bool: return self.state_change_source in state_set and self.state not in state_set # It's clear that results.models will need to reference evaluation.models' classes in ForeignKeys. @@ -651,44 +655,44 @@ def state_changed_from(self, state_set): del self.state_change_source @property - def full_name(self): + def full_name(self) -> str: if self.name: return f"{self.course.name} – {self.name}" return self.course.name @property - def full_name_de(self): + def full_name_de(self) -> str: if self.name_de: return f"{self.course.name_de} – {self.name_de}" return self.course.name_de @property - def full_name_en(self): + def full_name_en(self) -> str: if self.name_en: return f"{self.course.name_en} – {self.name_en}" return self.course.name_en @property - def is_fully_reviewed(self): + def is_fully_reviewed(self) -> bool: if not self.can_publish_text_results: return True return not self.unreviewed_textanswer_set.exists() @property - def display_vote_end_datetime(self): + def display_vote_end_datetime(self) -> datetime: return date_to_datetime(self.vote_end_date) + timedelta(hours=24) @property - def vote_end_datetime(self): + def vote_end_datetime(self) -> datetime: return vote_end_datetime(self.vote_end_date) @property - def runtime(self): + def runtime(self) -> int: delta = self.vote_end_datetime - self.vote_start_datetime return delta.days + 1 @property - def is_in_evaluation_period(self): + def is_in_evaluation_period(self) -> bool: return self.vote_start_datetime <= datetime.now() <= self.vote_end_datetime @property @@ -696,28 +700,32 @@ def is_dropout_allowed(self) -> bool: return self.general_contribution.questionnaires.filter(type=Questionnaire.Type.DROPOUT).exists() @property - def general_contribution_has_questionnaires(self): - return self.general_contribution and self.general_contribution.questionnaires.count() > 0 + def general_contribution_has_questionnaires(self) -> bool: + return self.general_contribution is not None and self.general_contribution.questionnaires.count() > 0 @property - def has_decided_main_language(self): + def has_decided_main_language(self) -> bool: return self.main_language != self.UNDECIDED_MAIN_LANGUAGE @property - def all_contributions_have_questionnaires(self): + def all_contributions_have_questionnaires(self) -> bool: if is_prefetched(self, "contributions"): if not self.contributions: return False - if is_prefetched(self.contributions[0], "questionnaires"): - return all(len(contribution.questionnaires) > 0 for contribution in self.contributions) + contributions = cast("Sequence[Contribution]", self.contributions) + if is_prefetched(contributions[0], "questionnaires"): + return all( + len(cast("Sequence[Questionnaire]", contribution.questionnaires)) > 0 + for contribution in contributions + ) return ( self.general_contribution is not None and not self.contributions.annotate(Count("questionnaires")).filter(questionnaires__count=0).exists() ) - def can_be_voted_for_by(self, user): + def can_be_voted_for_by(self, user: "UserProfile") -> bool: """Returns whether the user is allowed to vote on this evaluation.""" return ( self.state == Evaluation.State.IN_EVALUATION @@ -726,7 +734,7 @@ def can_be_voted_for_by(self, user): and user not in self.voters.all() ) - def can_be_seen_by(self, user): + def can_be_seen_by(self, user: "UserProfile") -> bool: if user.is_manager: return True if self.state == Evaluation.State.NEW: @@ -740,7 +748,7 @@ def can_be_seen_by(self, user): ) return True - def can_results_page_be_seen_by(self, user): + def can_results_page_be_seen_by(self, user: "UserProfile") -> bool: if user.is_manager: return True if user.is_reviewer and not self.course.semester.results_are_archived: @@ -752,19 +760,19 @@ def can_results_page_be_seen_by(self, user): return self.can_be_seen_by(user) @property - def can_reset_to_new(self): + def can_reset_to_new(self) -> bool: return Evaluation.State.PREPARED <= self.state <= Evaluation.State.REVIEWED @property - def can_be_edited_by_manager(self): + def can_be_edited_by_manager(self) -> bool: return not self.participations_are_archived and self.state < Evaluation.State.PUBLISHED @property - def can_be_deleted_by_manager(self): + def can_be_deleted_by_manager(self) -> bool: return self.can_be_edited_by_manager and self.num_voters == 0 @cached_property - def num_participants(self): + def num_participants(self) -> int: if self._participant_count is not None: return self._participant_count @@ -773,7 +781,7 @@ def num_participants(self): return self.participants.count() - def _archive(self): + def _archive(self) -> None: """Should be called only via Semester.archive""" if not self.participations_can_be_archived: raise NotArchivableError @@ -788,29 +796,29 @@ def _archive(self): self.related_logentries().delete() @property - def participations_are_archived(self): + def participations_are_archived(self) -> bool: semester_participations_are_archived = self.course.semester.participations_are_archived if semester_participations_are_archived: assert self._participant_count is not None and self._voter_count is not None return semester_participations_are_archived @property - def participations_can_be_archived(self): + def participations_can_be_archived(self) -> bool: return not self.course.semester.participations_are_archived and self.state in [ Evaluation.State.NEW, Evaluation.State.PUBLISHED, ] @property - def has_external_participant(self): + def has_external_participant(self) -> bool: return any(participant.is_external for participant in self.participants.all()) @property - def can_staff_see_average_grade(self): + def can_staff_see_average_grade(self) -> bool: return self.state >= Evaluation.State.EVALUATED @property - def can_publish_average_grade(self): + def can_publish_average_grade(self) -> bool: # the average grade is only published if at least the configured percentage of participants voted during the evaluation for significance reasons return ( self.can_publish_rating_results @@ -818,34 +826,34 @@ def can_publish_average_grade(self): ) @property - def can_publish_rating_results(self): + def can_publish_rating_results(self) -> bool: # the rating results are only published if at least the configured number of participants voted during the evaluation for anonymity reasons return self.num_voters >= settings.VOTER_COUNT_NEEDED_FOR_PUBLISHING_RATING_RESULTS @transition(field=state, source=[State.NEW, State.EDITOR_APPROVED], target=State.PREPARED) - def ready_for_editors(self): + def ready_for_editors(self) -> None: pass @transition( field=state, source=State.PREPARED, target=State.EDITOR_APPROVED, - conditions=[lambda self: self.has_decided_main_language], + conditions=[lambda self: cast("Any", self).has_decided_main_language], ) - def editor_approve(self): + def editor_approve(self) -> None: pass @transition( field=state, source=[State.NEW, State.PREPARED, State.EDITOR_APPROVED], target=State.APPROVED, - conditions=[lambda self: self.general_contribution_has_questionnaires and self.has_decided_main_language], + conditions=[lambda self: cast("Any", self).general_contribution_has_questionnaires and cast("Any", self).has_decided_main_language], ) - def manager_approve(self): + def manager_approve(self) -> None: pass - @transition(field=state, target=State.NEW, conditions=[lambda self: self.can_reset_to_new]) - def reset_to_new(self, *, delete_previous_answers: bool): + @transition(field=state, target=State.NEW, conditions=[lambda self: cast("Any", self).can_reset_to_new]) + def reset_to_new(self, *, delete_previous_answers: bool) -> None: if delete_previous_answers: for answer_class in Answer.__subclasses__(): answer_class._default_manager.filter(contribution__evaluation=self).delete() @@ -855,38 +863,38 @@ def reset_to_new(self, *, delete_previous_answers: bool): field=state, source=State.APPROVED, target=State.IN_EVALUATION, - conditions=[lambda self: self.is_in_evaluation_period], + conditions=[lambda self: cast("Any", self).is_in_evaluation_period], ) - def begin_evaluation(self): + def begin_evaluation(self) -> None: pass @transition( field=state, source=[State.EVALUATED, State.REVIEWED], target=State.IN_EVALUATION, - conditions=[lambda self: self.is_in_evaluation_period], + conditions=[lambda self: cast("Any", self).is_in_evaluation_period], ) - def reopen_evaluation(self): + def reopen_evaluation(self) -> None: pass @transition(field=state, source=State.IN_EVALUATION, target=State.EVALUATED) - def end_evaluation(self): + def end_evaluation(self) -> None: pass @transition( - field=state, source=State.EVALUATED, target=State.REVIEWED, conditions=[lambda self: self.is_fully_reviewed] + field=state, source=State.EVALUATED, target=State.REVIEWED, conditions=[lambda self: cast("Any", self).is_fully_reviewed] ) - def end_review(self): + def end_review(self) -> None: pass @transition( - field=state, source=State.REVIEWED, target=State.EVALUATED, conditions=[lambda self: not self.is_fully_reviewed] + field=state, source=State.REVIEWED, target=State.EVALUATED, conditions=[lambda self: not cast("Any", self).is_fully_reviewed] ) - def reopen_review(self): + def reopen_review(self) -> None: pass @transition(field=state, source=State.REVIEWED, target=State.PUBLISHED) - def publish(self): + def publish(self) -> None: assert self._voter_count is None and self._participant_count is None self._voter_count = self.num_voters self._participant_count = self.num_participants @@ -904,13 +912,13 @@ def publish(self): self.textanswer_set.update(original_answer=None) @transition(field=state, source=State.PUBLISHED, target=State.REVIEWED) - def unpublish(self): + def unpublish(self) -> None: assert self._voter_count == self.voters.count() and self._participant_count == self.participants.count() self._voter_count = None self._participant_count = None @property - def state_str(self): + def state_str(self) -> StrOrPromise: return Evaluation.State(self.state).label @cached_property @@ -924,68 +932,68 @@ def general_contribution(self): return None @cached_property - def num_voters(self): + def num_voters(self) -> int: if self._voter_count is not None: return self._voter_count return self.voters.count() @property - def voter_ratio(self): + def voter_ratio(self) -> float: if self.num_participants == 0: return 0 return self.num_voters / self.num_participants @property - def due_participants(self): + def due_participants(self) -> "QuerySet[UserProfile]": return self.participants.exclude(pk__in=self.voters.all()) @cached_property - def num_contributors(self): + def num_contributors(self) -> int: return UserProfile.objects.filter(contributions__evaluation=self).count() @property - def days_left_for_evaluation(self): + def days_left_for_evaluation(self) -> int: return (self.vote_end_date - date.today()).days @property - def display_time_left_for_evaluation(self): + def display_time_left_for_evaluation(self) -> timedelta: return self.display_vote_end_datetime - datetime.now() @property - def time_left_for_evaluation(self): + def time_left_for_evaluation(self) -> timedelta: return self.vote_end_datetime - datetime.now() @property - def display_hours_left_for_evaluation(self): + def display_hours_left_for_evaluation(self) -> float: return self.display_time_left_for_evaluation / timedelta(hours=1) @property - def hours_left_for_evaluation(self): + def hours_left_for_evaluation(self) -> float: return self.time_left_for_evaluation / timedelta(hours=1) @property - def ends_soon(self): + def ends_soon(self) -> bool: return 0 < self.time_left_for_evaluation.total_seconds() < settings.EVALUATION_END_WARNING_PERIOD * 3600 @property - def days_until_evaluation(self): + def days_until_evaluation(self) -> int: days_left = (self.vote_start_datetime.date() - date.today()).days if self.vote_start_datetime < datetime.now(): days_left -= 1 return days_left @property - def hours_until_evaluation(self): + def hours_until_evaluation(self) -> float: return (self.vote_start_datetime - datetime.now()) / timedelta(hours=1) - def is_user_editor_or_delegate(self, user): + def is_user_editor_or_delegate(self, user: "UserProfile") -> bool: represented_users = user.represented_users.all() | UserProfile.objects.filter(pk=user.pk) return ( self.contributions.filter(contributor__in=represented_users, role=Contribution.Role.EDITOR).exists() or self.course.responsibles.filter(pk__in=represented_users).exists() ) - def is_user_responsible_or_contributor_or_delegate(self, user): + def is_user_responsible_or_contributor_or_delegate(self, user: "UserProfile") -> bool: # early out that saves database hits since is_responsible_or_contributor_or_delegate is a cached_property if not user.is_responsible_or_contributor_or_delegate: return False @@ -995,33 +1003,33 @@ def is_user_responsible_or_contributor_or_delegate(self, user): or self.course.responsibles.filter(pk__in=represented_users).exists() ) - def is_user_contributor(self, user): + def is_user_contributor(self, user: "UserProfile") -> bool: return self.contributions.filter(contributor=user).exists() @property - def textanswer_set(self): + def textanswer_set(self) -> "QuerySet[TextAnswer]": return TextAnswer.objects.filter(contribution__evaluation=self) @cached_property - def num_textanswers(self): + def num_textanswers(self) -> int: if not self.can_publish_text_results: return 0 return self.textanswer_set.count() @property - def unreviewed_textanswer_set(self): + def unreviewed_textanswer_set(self) -> "QuerySet[TextAnswer]": return self.textanswer_set.filter(review_decision=TextAnswer.ReviewDecision.UNDECIDED) @property - def reviewed_textanswer_set(self): + def reviewed_textanswer_set(self) -> "QuerySet[TextAnswer]": return self.textanswer_set.exclude(review_decision=TextAnswer.ReviewDecision.UNDECIDED) @cached_property - def num_reviewed_textanswers(self): + def num_reviewed_textanswers(self) -> int: return self.reviewed_textanswer_set.count() @property - def textanswer_review_state(self): + def textanswer_review_state(self) -> "Evaluation.TextAnswerReviewState": if self.num_textanswers == 0: return self.TextAnswerReviewState.NO_TEXTANSWERS @@ -1037,15 +1045,15 @@ def textanswer_review_state(self): return self.TextAnswerReviewState.REVIEW_NEEDED @property - def ratinganswer_counters(self): + def ratinganswer_counters(self) -> "QuerySet[RatingAnswerCounter]": return RatingAnswerCounter.objects.filter(contribution__evaluation=self) @property - def all_participants_are_external(self): + def all_participants_are_external(self) -> bool: return all(participant.is_external for participant in self.participants.all()) @property - def grading_process_is_finished(self): + def grading_process_is_finished(self) -> bool: return ( not self.wait_for_grade_upload_before_publishing or self.course.gets_no_grade_documents @@ -1055,7 +1063,7 @@ def grading_process_is_finished(self): ) @classmethod - def update_evaluations(cls): + def update_evaluations(cls) -> None: logger.info("update_evaluations called. Processing evaluations now.") evaluations_new_in_evaluation = [] @@ -1096,24 +1104,26 @@ def update_evaluations(cls): logger.info("update_evaluations finished.") @classmethod - def annotate_with_participant_and_voter_counts(cls, evaluation_query): + def annotate_with_participant_and_voter_counts( + cls, evaluation_query: "QuerySet[Evaluation]" + ) -> "QuerySet[Evaluation]": subquery = Evaluation.objects.filter(pk=OuterRef("pk")) - participant_count_subquery = subquery.annotate( + participant_count_subquery = subquery.annotate( # type: ignore[no-redef,misc] num_participants=Coalesce("_participant_count", Count("participants")), ).values("num_participants") - voter_count_subquery = subquery.annotate( + voter_count_subquery = subquery.annotate( # type: ignore[no-redef,misc] num_voters=Coalesce("_voter_count", Count("voters")), ).values("num_voters") - return evaluation_query.annotate( + return evaluation_query.annotate( # type: ignore[no-redef] num_participants=Subquery(participant_count_subquery), num_voters=Subquery(voter_count_subquery), ) @property - def unlogged_fields(self): + def unlogged_fields(self) -> list[str]: return super().unlogged_fields + [ "voters", "can_publish_text_results", @@ -1124,15 +1134,15 @@ def unlogged_fields(self): @receiver(post_transition, sender=Evaluation) -def evaluation_state_change(instance, source, **_kwargs): +def evaluation_state_change(instance: Evaluation, source: int, **_kwargs: Any) -> None: """Evaluation.save checks whether caches must be updated based on this value""" # if multiple state changes are happening, state_change_source should be the first source if not hasattr(instance, "state_change_source"): - instance.state_change_source = source + cast("Any", instance).state_change_source = source @receiver(post_transition, sender=Evaluation) -def log_state_transition(instance, name, source: int, target: int, **_kwargs): +def log_state_transition(instance: Evaluation, name: str, source: int, target: int, **_kwargs: Any) -> None: logger.info( 'Evaluation "%s" (id %d) moved from state "%s" to state "%s", caused by transition "%s".', instance, @@ -1187,7 +1197,7 @@ class Meta: verbose_name_plural = _("contributions") @property - def unlogged_fields(self): + def unlogged_fields(self) -> list[str]: return ( super().unlogged_fields + ["evaluation"] @@ -1195,19 +1205,19 @@ def unlogged_fields(self): ) @property - def is_general(self): + def is_general(self) -> bool: return self.contributor_id is None @property - def object_to_attach_logentries_to(self): + def object_to_attach_logentries_to(self) -> tuple[type[Evaluation], int]: return Evaluation, self.evaluation_id - def __str__(self): + def __str__(self) -> str: if self.contributor: return _("Contribution by {full_name}").format(full_name=self.contributor.full_name) return str(_("General Contribution")) - def remove_answers_to_questionnaires(self, questionnaires): + def remove_answers_to_questionnaires(self, questionnaires: Iterable[Questionnaire]) -> None: assert set(Answer.__subclasses__()) == {TextAnswer, RatingAnswerCounter} TextAnswer.objects.filter(contribution=self, assignment__questionnaire__in=questionnaires).delete() RatingAnswerCounter.objects.filter(contribution=self, assignment__questionnaire__in=questionnaires).delete() @@ -1286,7 +1296,7 @@ class Meta: ) ] - def save(self, *args, **kwargs): + def save(self, *args, **kwargs) -> None: if self.type in [QuestionType.TEXT, QuestionType.HEADING]: self.allows_additional_textanswers = False if "update_fields" in kwargs: @@ -1295,7 +1305,7 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) @property - def answer_class(self): + def answer_class(self) -> "builtins.type[TextAnswer | RatingAnswerCounter]": if self.is_text_question: return TextAnswer if self.is_rating_question: @@ -1304,15 +1314,15 @@ def answer_class(self): raise AssertionError(f"Unknown answer type: {self.type!r}") @property - def is_positive_likert_question(self): + def is_positive_likert_question(self) -> bool: return self.type == QuestionType.POSITIVE_LIKERT @property - def is_negative_likert_question(self): + def is_negative_likert_question(self) -> bool: return self.type == QuestionType.NEGATIVE_LIKERT @property - def is_bipolar_likert_question(self): + def is_bipolar_likert_question(self) -> bool: return self.type in ( QuestionType.EASY_DIFFICULT, QuestionType.FEW_MANY, @@ -1323,27 +1333,27 @@ def is_bipolar_likert_question(self): ) @property - def is_text_question(self): + def is_text_question(self) -> bool: return self.type == QuestionType.TEXT @property - def is_grade_question(self): + def is_grade_question(self) -> bool: return self.type == QuestionType.GRADE @property - def is_positive_yes_no_question(self): + def is_positive_yes_no_question(self) -> bool: return self.type == QuestionType.POSITIVE_YES_NO @property - def is_negative_yes_no_question(self): + def is_negative_yes_no_question(self) -> bool: return self.type == QuestionType.NEGATIVE_YES_NO @property - def is_yes_no_question(self): + def is_yes_no_question(self) -> bool: return self.is_positive_yes_no_question or self.is_negative_yes_no_question @property - def is_rating_question(self): + def is_rating_question(self) -> bool: return ( self.is_grade_question or self.is_bipolar_likert_question @@ -1353,15 +1363,15 @@ def is_rating_question(self): ) @property - def is_non_grade_rating_question(self): + def is_non_grade_rating_question(self) -> bool: return self.is_rating_question and not self.is_grade_question @property - def is_heading_question(self): + def is_heading_question(self) -> bool: return self.type == QuestionType.HEADING @property - def can_have_textanswers(self): + def can_have_textanswers(self) -> bool: return self.is_text_question or self.is_rating_question and self.allows_additional_textanswers @@ -1377,7 +1387,7 @@ class Meta: @receiver(post_delete, sender=QuestionAssignment) @transaction.atomic -def _question_assignment_post_delete(*, instance: QuestionAssignment, **_kwargs) -> None: +def _question_assignment_post_delete(*, instance: QuestionAssignment, **_kwargs: Any) -> None: """Garbage-collect unused questions once no questionnaires reference them anymore.""" if not QuestionAssignment.objects.filter(question=instance.question.pk).exists(): @@ -1387,13 +1397,13 @@ def _question_assignment_post_delete(*, instance: QuestionAssignment, **_kwargs) @dataclass class Choices: css_class: str - values: tuple[Real] + values: tuple[int] colors: tuple[str] - grades: tuple[Real] + grades: tuple[int] names: list[StrOrPromise] is_inverted: bool - def as_name_color_value_tuples(self): + def as_name_color_value_tuples(self) -> "Iterable[tuple[StrOrPromise, str, int]]": return zip(self.names, self.colors, self.values, strict=True) @@ -1655,29 +1665,29 @@ class Meta: ] @property - def will_be_deleted(self): + def will_be_deleted(self) -> bool: return self.review_decision == self.ReviewDecision.DELETED @property - def will_be_private(self): + def will_be_private(self) -> bool: return self.review_decision == self.ReviewDecision.PRIVATE @property - def will_be_public(self): + def will_be_public(self) -> bool: return self.review_decision == self.ReviewDecision.PUBLIC # Once evaluation results are published, the review decision is executed # and thus, an answer _is_ private or _is_ public from that point on. @property - def is_public(self): + def is_public(self) -> bool: return self.will_be_public @property - def is_private(self): + def is_private(self) -> bool: return self.will_be_private @property - def is_reviewed(self): + def is_reviewed(self) -> bool: return self.review_decision != self.ReviewDecision.UNDECIDED @@ -1722,7 +1732,7 @@ class NotHalfEmptyConstraint(CheckConstraint): fields: list[str] = [] - def __init__(self, *, fields: list[str], name: str, **kwargs): + def __init__(self, *, fields: list[str], name: str, **kwargs: Any) -> None: self.fields = fields assert "condition" not in kwargs @@ -1743,7 +1753,9 @@ def validate(self, model, instance, exclude=None, using=None): super().validate(model, instance, exclude, using) except ValidationError as e: e.error_dict = { - field_name: ValidationError(instance._meta.get_field(field_name).error_messages["blank"], code="blank") + field_name: [ + ValidationError(instance._meta.get_field(field_name).error_messages["blank"], code="blank") + ] for field_name in self.fields if getattr(instance, field_name) == "" } @@ -1786,19 +1798,34 @@ class Meta: ), ) - def is_empty(self): + def is_empty(self) -> bool: return not (self.title or self.content) -class UserProfileManager(BaseUserManager): - def create_user(self, *, email, password=None, first_name_given=None, last_name=None): +class UserProfileManager(BaseUserManager["UserProfile"]): + def create_user( + self, + *, + email: str, + password: str | None = None, + first_name_given: str | None = None, + last_name: str | None = None, + ) -> "UserProfile": user = self.model(email=self.normalize_email(email), first_name_given=first_name_given, last_name=last_name) - validate_password(password, user=user) + if password is not None: + validate_password(password, user=user) user.set_password(password) user.save() return user - def create_superuser(self, *, email, password=None, first_name_given=None, last_name=None): + def create_superuser( + self, + *, + email: str, + password: str | None = None, + first_name_given: str | None = None, + last_name: str | None = None, + ) -> "UserProfile": user = self.create_user( password=password, email=self.normalize_email(email), @@ -1826,31 +1853,31 @@ class EvapBaseUser(models.Model): class Meta: abstract = True - def get_username(self): + def get_username(self) -> str: # required for django-webtest. See https://github.com/django-webtest/django-webtest/issues/134. return getattr(self, self.USERNAME_FIELD) @property - def is_anonymous(self): + def is_anonymous(self) -> bool: # django.contrib.auth.checks requires this to be MethodType return False @property - def is_authenticated(self): + def is_authenticated(self) -> bool: return True - def set_password(self, raw_password): + def set_password(self, raw_password: str | None) -> None: self.password = make_password(raw_password) - def check_password(self, raw_password): - def setter(raw_password): + def check_password(self, raw_password: str | None) -> bool: + def setter(raw_password: str) -> None: self.set_password(raw_password) # Password hash upgrades shouldn't be considered password changes. self.save(update_fields=["password"]) return check_password(raw_password, self.password, setter) - def has_usable_password(self): + def has_usable_password(self) -> bool: return is_password_usable(self.password) @@ -1933,10 +1960,10 @@ class Meta: USERNAME_FIELD = "email" REQUIRED_FIELDS: list[str] = ["first_name_given", "last_name"] - def __str__(self): + def __str__(self) -> str: return self.full_name - def save(self, *args, **kwargs): + def save(self, *args, **kwargs) -> None: # This is not guaranteed to be called on every insert. For example, the importers use bulk insertion. if self.has_usable_password() and not password_login_is_active(): @@ -1948,10 +1975,10 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) @property - def first_name(self): + def first_name(self) -> str: return self.first_name_chosen or self.first_name_given - def ordering_key(self): + def ordering_key(self) -> tuple[str, str, str]: # keep in sync with Meta.ordering lower_last_name = (self.last_name or "").lower() lower_first_name = (self.first_name or "").lower() @@ -1959,7 +1986,7 @@ def ordering_key(self): return (lower_last_name, lower_first_name, lower_email) @property - def full_name(self): + def full_name(self) -> str: if self.last_name: name = self.last_name if self.first_name: @@ -1976,35 +2003,35 @@ def full_name(self): return name @property - def full_name_with_additional_info(self): + def full_name_with_additional_info(self) -> str: name = self.full_name if self.is_external: return name + " [ext.]" return f"{name} ({self.email})" @cached_property - def is_staff(self): + def is_staff(self) -> bool: return self.is_manager or self.is_reviewer # Required for staff mode to work, since several other cached properties (including is_staff) are overwritten @property - def has_staff_permission(self): + def has_staff_permission(self) -> bool: return self.groups.filter(name="Manager").exists() or self.groups.filter(name="Reviewer").exists() @cached_property - def is_manager(self): + def is_manager(self) -> bool: return self.groups.filter(name="Manager").exists() @cached_property - def is_reviewer(self): + def is_reviewer(self) -> bool: return self.is_manager or self.groups.filter(name="Reviewer").exists() @cached_property - def is_grade_publisher(self): + def is_grade_publisher(self) -> bool: return self.groups.filter(name="Grade publisher").exists() @property - def can_be_marked_inactive_by_manager(self): + def can_be_marked_inactive_by_manager(self) -> bool: if self.is_reviewer or self.is_grade_publisher or self.is_superuser: return False if any(not evaluation.participations_are_archived for evaluation in self.evaluations_participating_in.all()): @@ -2016,7 +2043,7 @@ def can_be_marked_inactive_by_manager(self): return True @property - def can_be_deleted_by_manager(self): + def can_be_deleted_by_manager(self) -> bool: if ( self.is_responsible or self.is_contributor @@ -2032,11 +2059,11 @@ def can_be_deleted_by_manager(self): return True @cached_property - def is_participant(self): + def is_participant(self) -> bool: return self.evaluations_participating_in.exists() @cached_property - def is_student(self): + def is_student(self) -> bool: """ A UserProfile is not considered to be a student anymore if the newest contribution is newer than the newest participation. @@ -2048,48 +2075,46 @@ def is_student(self): return True last_semester_participated = ( - Semester.objects.filter(courses__evaluations__participants=self).order_by("-created_at").first() + Semester.objects.filter(courses__evaluations__participants=self).order_by("-created_at").get() ) last_semester_contributed = ( - Semester.objects.filter(courses__evaluations__contributions__contributor=self) - .order_by("-created_at") - .first() + Semester.objects.filter(courses__evaluations__contributions__contributor=self).order_by("-created_at").get() ) return last_semester_participated.created_at >= last_semester_contributed.created_at @cached_property - def is_contributor(self): + def is_contributor(self) -> bool: return self.contributions.exists() @cached_property - def is_editor(self): + def is_editor(self) -> bool: return self.contributions.filter(role=Contribution.Role.EDITOR).exists() or self.is_responsible @cached_property - def is_responsible(self): + def is_responsible(self) -> bool: return self.courses_responsible_for.exists() @cached_property - def is_delegate(self): + def is_delegate(self) -> bool: return self.represented_users.exists() @cached_property - def is_editor_or_delegate(self): + def is_editor_or_delegate(self) -> bool: return self.is_editor or self.is_delegate @cached_property - def is_responsible_or_contributor_or_delegate(self): + def is_responsible_or_contributor_or_delegate(self) -> bool: return self.is_responsible or self.is_contributor or self.is_delegate @cached_property - def show_startpage_button(self): + def show_startpage_button(self) -> bool: return [self.is_participant, self.is_responsible_or_contributor_or_delegate, self.is_grade_publisher].count( True ) > 1 @property - def is_external(self): + def is_external(self) -> bool: if self.is_proxy_user and not self.email: return False if not self.email: @@ -2097,19 +2122,19 @@ def is_external(self): return is_external_email(self.email) @property - def can_download_grades(self): + def can_download_grades(self) -> bool: return not self.is_external @staticmethod - def email_needs_login_key(email): + def email_needs_login_key(email: str) -> bool: return is_external_email(email) @property - def needs_login_key(self): - return UserProfile.email_needs_login_key(self.email) + def needs_login_key(self) -> bool: + return self.email is not None and UserProfile.email_needs_login_key(self.email) - def ensure_valid_login_key(self): - if self.login_key and self.login_key_valid_until > date.today(): + def ensure_valid_login_key(self) -> None: + if self.login_key and assert_not_none(self.login_key_valid_until) > date.today(): self.reset_login_key_validity() return @@ -2123,29 +2148,29 @@ def ensure_valid_login_key(self): # unique constraint failed, the login key was already in use. Generate another one. continue - def reset_login_key_validity(self): + def reset_login_key_validity(self) -> None: self.login_key_valid_until = date.today() + timedelta(settings.LOGIN_KEY_VALIDITY) self.save() @property - def login_url(self): + def login_url(self) -> str: if not self.needs_login_key: return "" return settings.PAGE_URL + reverse("evaluation:login_key_authentication", args=[self.login_key]) - def get_sorted_courses_responsible_for(self): + def get_sorted_courses_responsible_for(self) -> QuerySet[Course]: return self.courses_responsible_for.order_by("semester__created_at", "name_de") - def get_sorted_contributions(self): + def get_sorted_contributions(self) -> QuerySet[Contribution]: return self.contributions.order_by("evaluation__course__semester__created_at", "evaluation__name_de") - def get_sorted_evaluations_participating_in(self): + def get_sorted_evaluations_participating_in(self) -> QuerySet[Evaluation]: return self.evaluations_participating_in.order_by("course__semester__created_at", "name_de") - def get_sorted_evaluations_voted_for(self): + def get_sorted_evaluations_voted_for(self) -> QuerySet[Evaluation]: return self.evaluations_voted_for.order_by("course__semester__created_at", "name_de") - def get_sorted_due_evaluations(self): + def get_sorted_due_evaluations(self) -> list[tuple[Evaluation, int]]: evaluations_and_days_left = ( (evaluation, evaluation.days_left_for_evaluation) for evaluation in Evaluation.objects.filter( @@ -2155,7 +2180,7 @@ def get_sorted_due_evaluations(self): return sorted(evaluations_and_days_left, key=lambda tup: (tup[1], tup[0].full_name)) -def validate_template(value): +def validate_template(value: str) -> None: """Field validator which ensures that the value can be compiled into a Django Template.""" try: @@ -2250,7 +2275,7 @@ def send_to_users_in_evaluations( evaluations: Iterable[Evaluation], recipient_groups: Container[Recipients], use_cc: bool, - request: HttpRequest, + request: HttpRequest | None, ) -> None: user_evaluation_map: dict[UserProfile, list[Evaluation]] = {} for evaluation in evaluations: diff --git a/evap/evaluation/models_logging.py b/evap/evaluation/models_logging.py index 1a0ee18b70..ec951eda98 100644 --- a/evap/evaluation/models_logging.py +++ b/evap/evaluation/models_logging.py @@ -1,12 +1,12 @@ import itertools import threading from collections import defaultdict, namedtuple -from collections.abc import Iterator +from collections.abc import Iterable, Iterator, Sequence from contextlib import contextmanager from datetime import date, datetime, time from enum import StrEnum from json import JSONEncoder -from typing import assert_never +from typing import Any, assert_never, cast from django.conf import settings from django.contrib.contenttypes.fields import GenericForeignKey @@ -19,7 +19,8 @@ from django.utils.formats import localize from django.utils.translation import gettext_lazy as _ -from evap.evaluation.tools import capitalize_first, inject_choices_constraint +from evap.evaluation.tools import StrOrPromise, capitalize_first, inject_choices_constraint +from evap.tools import assert_not_none CREATE_LOGENTRIES = True @@ -58,28 +59,29 @@ class LogJSONEncoder(JSONEncoder): As JSON can't store datetime objects, we localize them to strings. """ - def default(self, o): + def default(self, o: Any) -> str: # o is the object to serialize -- we can't rename the argument in JSONEncoder if isinstance(o, date | time | datetime): return localize(o) return super().default(o) -def _choice_to_display(field, choice): # does not support nested choices +def _choice_to_display(field: models.Field, choice: Any) -> Any: + assert isinstance(field.choices, Sequence) # does not support nested choices for key, label in field.choices: if key == choice: return label return choice -def _field_actions_for_field(field, actions): +def _field_actions_for_field(field: Any, actions: dict[str, Any]) -> Iterable[FieldAction]: label = capitalize_first(getattr(field, "verbose_name", field.name)) for field_action_type, items in actions.items(): if field.many_to_many or field.many_to_one or field.one_to_one: # convert item values from primary keys to string-representation for relation-based fields - pk_to_obj = {obj.pk: obj for obj in field.related_model.objects.filter(pk__in=items)} + pk_to_obj = {obj.pk: obj for obj in assert_not_none(field.related_model).objects.filter(pk__in=items)} items = [_("") if item is None else str(pk_to_obj.get(item, _(""))) for item in items] elif hasattr(field, "choices") and field.choices: @@ -109,16 +111,17 @@ class Meta: ordering = ["-datetime", "-id"] @property - def field_context_data(self): + def field_context_data(self) -> dict[str, list[FieldAction]]: model = self.content_type.model_class() + assert model is not None return { field_name: list(_field_actions_for_field(model._meta.get_field(field_name), actions)) for field_name, actions in self.data.items() } @property - def message(self): - match self.action_type: + def message(self) -> str: + match cast("InstanceActionType", self.action_type): case InstanceActionType.CHANGE: if self.content_object: message = _("The {cls} {obj} was changed.") @@ -134,8 +137,12 @@ def message(self): case _: assert_never(self.action_type) + model = self.content_type.model_class() + assert model is not None + assert isinstance(model._meta.verbose_name, StrOrPromise) + return message.format( - cls=capitalize_first(self.content_type.model_class()._meta.verbose_name), + cls=capitalize_first(model._meta.verbose_name), obj=f'"{str(self.content_object)}"' if self.content_object else "", ) @@ -146,12 +153,12 @@ class LoggedModel(models.Model): class Meta: abstract = True - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) - self._logentry = None - self._m2m_changes = defaultdict(lambda: defaultdict(list)) + self._logentry: LogEntry | None = None + self._m2m_changes: dict[str, dict[FieldActionType, list]] = defaultdict(lambda: defaultdict(list)) - def save(self, *args, **kw): + def save(self, *args, **kw) -> None: # Are we creating a new instance? # https://docs.djangoproject.com/en/3.0/ref/models/instances/#customizing-model-loading if self._state.adding: @@ -164,7 +171,7 @@ def save(self, *args, **kw): self.log_instance_change() super().save(*args, **kw) - def _as_dict(self): + def _as_dict(self) -> dict[str, Any]: """ Return a dict mapping field names to values saved in this instance. Only include field names that are not to be ignored for logging and @@ -177,7 +184,7 @@ def _as_dict(self): ] return model_to_dict(self, fields) - def _get_change_data(self, action_type: InstanceActionType): + def _get_change_data(self, action_type: InstanceActionType) -> dict[str, Any]: """ Return a dict mapping field names to changes that happened in this model instance, depending on the action that is being done to the instance. @@ -216,26 +223,26 @@ def _get_change_data(self, action_type: InstanceActionType): return changes - def log_m2m_change(self, field_name, action_type: FieldActionType, change_list, **kwargs): + def log_m2m_change(self, field_name: str, action_type: FieldActionType, change_list: list, **kwargs: Any) -> None: # This might be called multiple times with cumulating changes # But this is fine, since the old changes will be included in the latest log update # See https://github.com/e-valuation/EvaP/issues/1594 self._m2m_changes[field_name][action_type] += change_list self._update_log(self._m2m_changes, InstanceActionType.CHANGE, **kwargs) - def log_instance_create(self): + def log_instance_create(self) -> None: changes = self._get_change_data(InstanceActionType.CREATE) self._update_log(changes, InstanceActionType.CREATE) - def log_instance_change(self): + def log_instance_change(self) -> None: changes = self._get_change_data(InstanceActionType.CHANGE) self._update_log(changes, InstanceActionType.CHANGE) - def log_instance_delete(self): + def log_instance_delete(self) -> None: changes = self._get_change_data(InstanceActionType.DELETE) self._update_log(changes, InstanceActionType.DELETE) - def _create_log_entry(self, action_type=InstanceActionType.CREATE): + def _create_log_entry(self, action_type: InstanceActionType = InstanceActionType.CREATE) -> LogEntry: try: user = self.thread.request.user request_id = self.thread.request_id @@ -254,42 +261,48 @@ def _create_log_entry(self, action_type=InstanceActionType.CREATE): action_type=action_type, ) - def _attach_log_entry_if_not_exists(self, *args, **kwargs): + def _attach_log_entry_if_not_exists(self, *args, **kwargs) -> LogEntry: if not self._logentry: self._logentry = self._create_log_entry(*args, **kwargs) - def _update_log(self, changes, action_type: InstanceActionType, store_in_db=True): + return self._logentry + + def _update_log(self, changes, action_type: InstanceActionType, store_in_db: bool = True) -> None: if not CREATE_LOGENTRIES: return if action_type == InstanceActionType.CHANGE and not changes: # All changes are on unlogged fields return - self._attach_log_entry_if_not_exists(action_type) + logentry = self._attach_log_entry_if_not_exists(action_type) # if adding more changes here, make sure you update all bulk_update calls that explicitly need to list changed # fields, too. - self._logentry.data.update(changes) + logentry.data.update(changes) if store_in_db: - self._logentry.save() + logentry.save() - def delete(self, *args, **kw): + def delete(self, *args, **kw) -> tuple[int, dict[str, int]]: self.log_instance_delete() self.related_logentries().delete() - super().delete(*args, **kw) + return super().delete(*args, **kw) @staticmethod - def update_log_after_bulk_create(instances): + def update_log_after_bulk_create(instances: "Iterable[LoggedModel]") -> None: for instance in instances: instance._logentry = instance._create_log_entry() - log_entries = [instance._logentry for instance in instances] + log_entries = [assert_not_none(instance._logentry) for instance in instances] LogEntry.objects.bulk_create(log_entries) @staticmethod def update_log_after_m2m_bulk_create( - from_instances, through_instances, from_pk_attribute: str, to_pk_attribute: str, m2m_field: str - ): + from_instances: "Iterable[LoggedModel]", + through_instances: Iterable[models.Model], + from_pk_attribute: str, + to_pk_attribute: str, + m2m_field: str, + ) -> None: added_related = defaultdict(list) for instance in through_instances: from_pk = getattr(instance, from_pk_attribute) @@ -299,7 +312,7 @@ def update_log_after_m2m_bulk_create( for instance in from_instances: instance.log_m2m_change(m2m_field, FieldActionType.M2M_ADD, added_related[instance.pk], store_in_db=False) - logentries = [instance._logentry for instance in from_instances] + logentries = [assert_not_none(instance._logentry) for instance in from_instances] to_create = [logentry for logentry in logentries if logentry.pk is None] to_update = [logentry for logentry in logentries if logentry.pk is not None] @@ -307,7 +320,7 @@ def update_log_after_m2m_bulk_create( LogEntry.objects.bulk_create(to_create) LogEntry.objects.bulk_update(to_update, ["data"]) - def related_logentries(self): + def related_logentries(self) -> models.QuerySet[LogEntry]: """ Return a queryset with all logentries that should be shown with this model. """ @@ -316,7 +329,7 @@ def related_logentries(self): attached_to_object_id=self.pk, ) - def grouped_logentries(self): + def grouped_logentries(self) -> Iterable[list[LogEntry]]: """ Returns a list of lists of logentries for display. The order is not changed. Logentries are grouped if they have a matching request_id. @@ -330,7 +343,7 @@ def grouped_logentries(self): ) @property - def object_to_attach_logentries_to(self): + def object_to_attach_logentries_to(self) -> tuple[type[models.Model], int]: """ Return a model class and primary key for the object for which this logentry should be shown. By default, show it to the object described by the logentry itself. @@ -341,14 +354,22 @@ def object_to_attach_logentries_to(self): return type(self), self.pk @property - def unlogged_fields(self): + def unlogged_fields(self) -> list[str]: """Specify a list of field names so that these fields don't get logged.""" return ["id", "order"] @receiver(m2m_changed) -def _m2m_changed(sender, instance, action, reverse, model, pk_set, **kwargs): # noqa: PLR0912 - model_class = model if reverse else type(instance) +def _m2m_changed( # noqa: PLR0912 + sender: Any, + instance: models.Model | None, + action: str | None, + reverse: bool, + model: type[models.Model], + pk_set: set[int] | None, + **kwargs: Any, +) -> None: + model_class = cast("type[models.Model]", model if reverse else type(instance)) field_name = next( (field.name for field in model_class._meta.many_to_many if getattr(model_class, field.name).through == sender), None, @@ -379,22 +400,22 @@ def _m2m_changed(sender, instance, action, reverse, model, pk_set, **kwargs): # if action_type == FieldActionType.M2M_CLEAR: action_type = FieldActionType.M2M_REMOVE if pk_set: - related_instances = model.objects.filter(pk__in=pk_set) + related_instances = cast("Any", model_class).objects.filter(pk__in=pk_set) else: # When action is pre_clear, pk_set is None, so we need to get the related instances from the instance itself - field = model._meta.get_field(field_name) - related_name = field.remote_field.get_accessor_name() + related_name = assert_not_none(model._meta.get_field(field_name).remote_field.get_accessor_name()) # type: ignore[union-attr] related_instances = getattr(instance, related_name).all() for related_instance in related_instances: if field_name in related_instance.unlogged_fields: continue - related_instance.log_m2m_change(field_name, action_type, [instance.pk]) + related_instance.log_m2m_change(field_name, action_type, [assert_not_none(instance).pk]) else: + assert isinstance(instance, model_class) if field_name in instance.unlogged_fields: return if action_type == FieldActionType.M2M_CLEAR: instance.log_m2m_change(field_name, FieldActionType.M2M_CLEAR, []) else: - instance.log_m2m_change(field_name, action_type, list(pk_set)) + instance.log_m2m_change(field_name, action_type, list(assert_not_none(pk_set))) diff --git a/evap/evaluation/templatetags/infotext_templatetags.py b/evap/evaluation/templatetags/infotext_templatetags.py index c85b5065aa..abaaf534d5 100644 --- a/evap/evaluation/templatetags/infotext_templatetags.py +++ b/evap/evaluation/templatetags/infotext_templatetags.py @@ -6,7 +6,7 @@ @register.inclusion_tag("infobox.html") -def show_infotext(page_name): +def show_infotext(page_name: str): to_page_id = { "contributor_index": Infotext.Page.CONTRIBUTOR_INDEX, "grades_pages": Infotext.Page.GRADES_PAGES, diff --git a/evap/evaluation/templatetags/navbar_templatetags.py b/evap/evaluation/templatetags/navbar_templatetags.py index a391694198..47cca5cbd4 100644 --- a/evap/evaluation/templatetags/navbar_templatetags.py +++ b/evap/evaluation/templatetags/navbar_templatetags.py @@ -1,14 +1,15 @@ from django.conf import settings +from django.contrib.auth.models import AnonymousUser from django.db.models import Q from django.template import Library -from evap.evaluation.models import Semester +from evap.evaluation.models import Semester, UserProfile register = Library() @register.inclusion_tag("navbar.html") -def include_navbar(user, language): +def include_navbar(user: UserProfile | AnonymousUser, language: str): semesters_with_unarchived_results_or_grade_documents = Semester.objects.filter( Q(results_are_archived=False) | Q(grade_documents_are_deleted=False) ) diff --git a/evap/evaluation/tests/tools.py b/evap/evaluation/tests/tools.py index e2b4de82bc..ec60a12aac 100644 --- a/evap/evaluation/tests/tools.py +++ b/evap/evaluation/tests/tools.py @@ -3,7 +3,6 @@ from contextlib import contextmanager from datetime import timedelta from importlib import import_module -from typing import Any import django.test import django_webtest @@ -43,7 +42,7 @@ class EvapTestRunner(DiscoverRunner): """Skips selenium tests by default, if no other tags are specified.""" - def __init__(self, *args: Any, headed=False, **kwargs: Any) -> None: + def __init__(self, *args, headed=False, **kwargs) -> None: super().__init__(*args, **kwargs) self.__headed = headed diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index 8ff1cb777b..b2abfa30d6 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -2,18 +2,20 @@ import re import typing from abc import ABC, abstractmethod -from collections.abc import Callable, Iterable, Mapping +from collections.abc import Callable, Iterable, Iterator, Mapping from contextlib import contextmanager -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, Never from urllib.parse import quote import xlwt +from django import forms from django.conf import settings from django.core.exceptions import SuspiciousOperation, ValidationError from django.db import DEFAULT_DB_ALIAS, connections from django.db.models import Field, Model, Q from django.db.models.constraints import CheckConstraint from django.db.models.fields.mixins import FieldCacheMixin +from django.dispatch.dispatcher import Signal from django.forms.formsets import BaseFormSet from django.http import HttpRequest, HttpResponse from django.shortcuts import get_object_or_404 @@ -83,7 +85,7 @@ def decorator(meta_cls: type) -> type: @contextmanager -def temporary_receiver(signal, func, **kwargs): +def temporary_receiver(signal: Signal, func: Callable, **kwargs: Any) -> Iterator[None]: # semi-copy of https://github.com/django/django/blob/3266f2516c27dd25abebe8e8f7b8778650ab4f18/django/dispatch/dispatcher.py#L472 # with sane contextmanager behavior (receivers are unlinked on exit). We don't support lists of signals (for now). signal.connect(receiver=func, **kwargs) @@ -111,7 +113,7 @@ def get_object_from_dict_pk_entry_or_logged_40x[M: Model]( raise SuspiciousOperation from e -def is_prefetched(instance, attribute_name: str) -> bool: +def is_prefetched(instance: Model, attribute_name: str) -> bool: """ Is the given related attribute prefetched? Can be used to do ordering or counting in python and avoid additional database queries @@ -170,7 +172,7 @@ def vote_end_datetime(vote_end_date: datetime.date) -> datetime.datetime: return date_to_datetime(vote_end_date) + datetime.timedelta(hours=24 + settings.EVALUATION_END_OFFSET_HOURS) -def get_parameter_from_url_or_session(request: HttpRequest, parameter: str, default=False) -> bool: +def get_parameter_from_url_or_session(request: HttpRequest, parameter: str, default: bool = False) -> bool: result_str = request.GET.get(parameter, None) if result_str is None: # if no parameter is given take session value result = request.session.get(parameter, default) @@ -180,14 +182,14 @@ def get_parameter_from_url_or_session(request: HttpRequest, parameter: str, defa return result -def translate(**kwargs): +def translate(**kwargs: Any) -> property: # pylint is really buggy with this method. # pylint: disable=unused-variable, useless-suppression # get_language may return None if there is no session (e.g. during management commands) return property(lambda self: getattr(self, kwargs[get_language() or "en"])) -def clean_email[EmailT: (str, None)](email: EmailT) -> EmailT: +def clean_email[EmailT: (str, str | None)](email: EmailT) -> EmailT: if email is None: return None @@ -210,11 +212,13 @@ class FormsetView(FormView): Just like `FormView`, but with a renaming from "form" to "formset". """ + formset_class: type[BaseFormSet] + @property def form_class(self): return self.formset_class - def get_context_data(self, **kwargs) -> dict[str, Any]: + def get_context_data(self, **kwargs: Any) -> dict[str, Any]: context = super().get_context_data(**kwargs) context["formset"] = context.pop("form") return context @@ -229,10 +233,10 @@ def get_form_kwargs(self) -> dict: def get_formset_kwargs(self) -> dict: return super().get_form_kwargs() - def form_valid(self, form) -> HttpResponse: + def form_valid(self, form: BaseFormSet) -> HttpResponse: return self.formset_valid(form) - def formset_valid(self, formset) -> HttpResponse: + def formset_valid(self, formset: BaseFormSet) -> HttpResponse: return super().form_valid(formset) @@ -244,7 +248,7 @@ class SaveValidFormMixin: example if a formset for a collection of objects is submitted. """ - def form_valid(self, form) -> HttpResponse: + def form_valid(self, form: forms.ModelForm | forms.BaseModelFormSet) -> HttpResponse: form.save() return super().form_valid(form) # type: ignore[misc] # there is no valid way to annotate this @@ -357,7 +361,7 @@ def write_empty_row_with_styles(self, styles: Iterable[str]) -> None: def export_impl(self, *args, **kwargs) -> None: """Specify the logic to insert the data into the sheet here.""" - def export(self, response, *args, **kwargs) -> None: + def export(self, response: HttpResponse | typing.BinaryIO, *args, **kwargs) -> None: """Convenience method to avoid some boilerplate.""" self.export_impl(*args, **kwargs) self.workbook.save(response) diff --git a/evap/evaluation/views.py b/evap/evaluation/views.py index d94eddea23..6afcb04922 100644 --- a/evap/evaluation/views.py +++ b/evap/evaluation/views.py @@ -5,7 +5,7 @@ from django.contrib import auth, messages from django.core.exceptions import SuspiciousOperation from django.core.mail import EmailMessage -from django.http import HttpResponse, HttpResponseBadRequest +from django.http import HttpRequest, HttpResponse, HttpResponseBadRequest from django.shortcuts import redirect, render from django.urls import reverse from django.utils.encoding import iri_to_uri @@ -19,12 +19,13 @@ from evap.evaluation.forms import LoginEmailForm, NewKeyForm, NotebookForm, ProfileForm from evap.evaluation.models import EmailTemplate, FaqSection, Semester, UserProfile from evap.evaluation.tools import HttpResponseNoContent, openid_login_is_active, password_login_is_active +from evap.tools import assert_not_none from evap.middleware import no_login_required logger = logging.getLogger(__name__) -def redirect_user_to_start_page(user): # noqa: PLR0911 +def redirect_user_to_start_page(user: UserProfile) -> HttpResponse: # noqa: PLR0911 active_semester = Semester.active_semester() if user.is_reviewer: @@ -53,7 +54,7 @@ def redirect_user_to_start_page(user): # noqa: PLR0911 @no_login_required @sensitive_post_parameters("password") -def index(request): +def index(request: HttpRequest) -> HttpResponse: """Main entry page into EvaP providing all the login options available. The OpenID login is thought to be used for internal users. The login key mechanism is meant to be used to include external participants, e.g. visiting students or visiting contributors. A login with email and password is available if OpenID is deactivated. @@ -72,11 +73,11 @@ def index(request): if request.method == "POST": if new_key_form.is_valid(): # user wants a new login key - profile = new_key_form.get_user() + profile = assert_not_none(new_key_form.get_user()) profile.ensure_valid_login_key() profile.save() - EmailTemplate.send_login_url_to_user(new_key_form.get_user()) + EmailTemplate.send_login_url_to_user(profile) messages.success(request, _("We sent you an email with a one-time login URL. Please check your inbox.")) return redirect("evaluation:index") @@ -116,7 +117,7 @@ def index(request): @no_login_required -def login_key_authentication(request, key): +def login_key_authentication(request: HttpRequest, key: int) -> HttpResponse: user = auth.authenticate(request, key=key) if user and not user.is_active: @@ -132,7 +133,7 @@ def login_key_authentication(request, key): ) return redirect("evaluation:index") - if user and user.login_key_valid_until >= date.today(): + if user and assert_not_none(user.login_key_valid_until) >= date.today(): if request.method != "POST": template_data = {"username": user.full_name} return render(request, "external_user_confirm_login.html", template_data) @@ -156,7 +157,7 @@ def login_key_authentication(request, key): @no_login_required -def faq(request): +def faq(request: HttpRequest) -> HttpResponse: return render(request, "faq.html", {"sections": FaqSection.objects.all()}) @@ -167,7 +168,7 @@ class LegalNoticeView(TemplateView): @require_POST -def contact(request): +def contact(request: HttpRequest) -> HttpResponse: sent_anonymous = request.POST.get("anonymous") == "true" if sent_anonymous and not settings.ALLOW_ANONYMOUS_FEEDBACK_MESSAGES: raise SuspiciousOperation("Anonymous feedback messages are not allowed, however received one from user!") @@ -177,6 +178,7 @@ def contact(request): sender = "anonymous user" subject = "[EvaP] Anonymous message" else: + assert isinstance(request.user, UserProfile) sender = request.user.email or f"User {request.user.id}" subject = f"[EvaP] Message from {sender}" if message: @@ -199,7 +201,7 @@ def contact(request): @no_login_required @require_POST -def set_lang(request): +def set_lang(request: HttpRequest) -> HttpResponse: if request.user.is_authenticated: user = request.user user.language = request.POST.get("language", "en") @@ -208,7 +210,7 @@ def set_lang(request): return set_language(request) -def profile_edit(request): +def profile_edit(request: HttpRequest) -> HttpResponse: user = request.user profile_form = ProfileForm(request.POST or None, request.FILES or None, instance=user) @@ -218,6 +220,7 @@ def profile_edit(request): messages.success(request, _("Successfully updated your profile.")) return redirect("evaluation:profile_edit") + assert isinstance(user, UserProfile) editor_context = { "delegate_of": user.represented_users.all(), "cc_users": user.cc_users.all(), @@ -230,7 +233,7 @@ def profile_edit(request): @require_POST -def set_notes(request): +def set_notes(request: HttpRequest) -> HttpResponse: form = NotebookForm(request.POST, instance=request.user) if form.is_valid(): form.save() @@ -238,8 +241,9 @@ def set_notes(request): return HttpResponseBadRequest() -def set_startpage(request): +def set_startpage(request: HttpRequest) -> HttpResponse: user = request.user + assert isinstance(user, UserProfile) startpage = request.POST.get("page") if startpage not in UserProfile.StartPage.values: return HttpResponseBadRequest() diff --git a/evap/grades/forms.py b/evap/grades/forms.py index fb52c6ac4a..465283b5c5 100644 --- a/evap/grades/forms.py +++ b/evap/grades/forms.py @@ -2,6 +2,7 @@ from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ +from evap.evaluation.models import UserProfile from evap.grades.models import GradeDocument @@ -28,7 +29,7 @@ def __init__(self, *args, **kwargs): self.fields["last_modified_time_2"].widget = forms.HiddenInput() self.fields["last_modified_user_2"].widget = forms.HiddenInput() - def clean_description_de(self): + def clean_description_de(self) -> str | None: description_de = self.cleaned_data.get("description_de") if ( GradeDocument.objects.filter(course=self.instance.course, description_de=description_de) @@ -38,7 +39,7 @@ def clean_description_de(self): raise ValidationError(_("This description for a grade document was already used for this course.")) return description_de - def clean_description_en(self): + def clean_description_en(self) -> str | None: description_en = self.cleaned_data.get("description_en") if ( GradeDocument.objects.filter(course=self.instance.course, description_en=description_en) @@ -48,6 +49,6 @@ def clean_description_en(self): raise ValidationError(_("This description for a grade document was already used for this course.")) return description_en - def save(self, *args, modifying_user, **kwargs): # pylint: disable=arguments-differ + def save(self, *args, modifying_user: UserProfile | None, **kwargs) -> None: # type: ignore[override] # pylint: disable=arguments-differ self.instance.last_modified_user = modifying_user super().save(*args, **kwargs) diff --git a/evap/grades/models.py b/evap/grades/models.py index b4b41f384c..806e4383f9 100644 --- a/evap/grades/models.py +++ b/evap/grades/models.py @@ -10,7 +10,7 @@ from evap.evaluation.tools import inject_choices_constraint, translate -def helper_upload_path(instance, filename): +def helper_upload_path(instance: "GradeDocument", filename: str) -> str: return f"grades/{instance.course.id}/{filename}" @@ -41,7 +41,7 @@ class Meta: verbose_name_plural = _("Grade Documents") unique_together = [["course", "description_de"], ["course", "description_en"]] - def __str__(self): + def __str__(self) -> str: return self.description def filename(self): @@ -49,14 +49,14 @@ def filename(self): @receiver(pre_delete, sender=GradeDocument) -def delete_file_pre_delete(instance, **_kwargs): +def delete_file_pre_delete(instance: GradeDocument, **_kwargs) -> None: if instance.file: instance.file.delete(False) # Changing should lead to the removal of the old file @receiver(pre_save, sender=GradeDocument) -def delete_file_pre_save(instance, **_kwargs): +def delete_file_pre_save(instance: GradeDocument, **_kwargs) -> None: if not instance.pk: # We do not want to trigger document creation return try: diff --git a/evap/grades/views.py b/evap/grades/views.py index 3337c6708b..5cd31aac3e 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -4,7 +4,7 @@ from django.contrib import messages from django.core.exceptions import PermissionDenied, SuspiciousOperation from django.db.models.query import QuerySet -from django.http import FileResponse, HttpResponse +from django.http import FileResponse, HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect, render from django.utils.translation import gettext as _ from django.views.decorators.http import require_GET, require_POST @@ -15,7 +15,7 @@ grade_publisher_or_manager_required, grade_publisher_required, ) -from evap.evaluation.models import Course, EmailTemplate, Evaluation, Semester +from evap.evaluation.models import Course, EmailTemplate, Evaluation, Semester, UserProfile from evap.evaluation.tools import get_object_from_dict_pk_entry_or_logged_40x from evap.grades.forms import GradeDocumentForm from evap.grades.models import GradeDocument @@ -95,7 +95,7 @@ def get_context_data(self, **kwargs) -> dict[str, Any]: } -def on_grading_process_finished(course): +def on_grading_process_finished(course: Course) -> None: evaluations = course.evaluations.all() if all(evaluation.state == Evaluation.State.REVIEWED for evaluation in evaluations): for evaluation in evaluations: @@ -109,7 +109,7 @@ def on_grading_process_finished(course): @grade_publisher_required -def upload_grades(request, course_id): +def upload_grades(request: HttpRequest, course_id: int) -> HttpResponse: course = get_object_or_404(Course, id=course_id) semester = course.semester if semester.grade_documents_are_deleted: @@ -130,6 +130,7 @@ def upload_grades(request, course_id): form = GradeDocumentForm(request.POST or None, request.FILES or None, instance=grade_document) if form.is_valid(): + assert isinstance(request.user, UserProfile) form.save(modifying_user=request.user) if final_grades: @@ -150,7 +151,7 @@ def upload_grades(request, course_id): @require_POST @grade_publisher_required -def set_no_grades(request): +def set_no_grades(request: HttpRequest) -> HttpResponse: course = get_object_from_dict_pk_entry_or_logged_40x(Course, request.POST, "course_id") try: @@ -172,7 +173,7 @@ def set_no_grades(request): @require_GET @grade_downloader_required -def download_grades(request, grade_document_id): +def download_grades(request: HttpRequest, grade_document_id: int) -> FileResponse: grade_document = get_object_or_404(GradeDocument, id=grade_document_id) if grade_document.course.semester.grade_documents_are_deleted: raise PermissionDenied @@ -181,7 +182,7 @@ def download_grades(request, grade_document_id): @grade_publisher_required -def edit_grades(request, grade_document_id): +def edit_grades(request: HttpRequest, grade_document_id: int) -> HttpResponse: grade_document = get_object_or_404(GradeDocument, id=grade_document_id) course = grade_document.course semester = course.semester @@ -195,6 +196,7 @@ def edit_grades(request, grade_document_id): ) # if parameter is not given, assume midterm grades if form.is_valid(): + assert isinstance(request.user, UserProfile) form.save(modifying_user=request.user) messages.success(request, _("Successfully updated grades.")) return redirect("grades:course_view", course.id) @@ -211,7 +213,7 @@ def edit_grades(request, grade_document_id): @require_POST @grade_publisher_required -def delete_grades(request): +def delete_grades(request: HttpRequest) -> HttpResponse: grade_document = get_object_from_dict_pk_entry_or_logged_40x(GradeDocument, request.POST, "grade_document_id") grade_document.delete() return HttpResponse() # 200 OK diff --git a/evap/middleware.py b/evap/middleware.py index d9bc6d44f8..de60285c47 100644 --- a/evap/middleware.py +++ b/evap/middleware.py @@ -3,6 +3,7 @@ from django.conf import settings from django.contrib.auth.views import redirect_to_login +from django.http import HttpRequest, HttpResponse from django.utils import translation from django.views import View from mozilla_django_oidc.views import OIDCAuthenticationCallbackView, OIDCAuthenticationRequestView @@ -15,10 +16,10 @@ class RequireLoginMiddleware: - def __init__(self, get_response): + def __init__(self, get_response: Callable[[HttpRequest], HttpResponse]) -> None: self.get_response = get_response - def __call__(self, request): + def __call__(self, request: HttpRequest) -> HttpResponse: return self.get_response(request) @staticmethod @@ -32,7 +33,7 @@ def _is_or_wraps_view_not_requiring_login(view: ViewFuncOrClass) -> bool: return False @classmethod - def process_view(cls, request, view_func, _view_args, _view_kwargs): + def process_view(cls, request: HttpRequest, view_func, _view_args, _view_kwargs) -> HttpResponse | None: # Returning None tells django to pass the request on if request.user.is_authenticated: return None @@ -44,7 +45,7 @@ def process_view(cls, request, view_func, _view_args, _view_kwargs): return redirect_to_login(request.get_full_path()) -def no_login_required(class_or_function: ViewFuncOrClass): +def no_login_required[ViewT: ViewFuncOrClass](class_or_function: ViewT) -> ViewT: # view funcs of class based views are shared, so we cannot track them here. Use the decorator on the class instead. assert not hasattr(class_or_function, "view_class"), "unexpected called with a view function of a class based view" @@ -52,8 +53,10 @@ def no_login_required(class_or_function: ViewFuncOrClass): return class_or_function -def user_language_middleware(get_response): - def middleware(request): +def user_language_middleware( + get_response: Callable[[HttpRequest], HttpResponse], +) -> Callable[[HttpRequest], HttpResponse]: + def middleware(request: HttpRequest) -> HttpResponse: if not (request.user and request.user.is_authenticated): return get_response(request) if request.user.language == translation.get_language(): diff --git a/evap/results/exporters.py b/evap/results/exporters.py index bc48e4d746..4db2a3e05a 100644 --- a/evap/results/exporters.py +++ b/evap/results/exporters.py @@ -331,7 +331,7 @@ def export_impl( include_unpublished: bool = False, contributor: UserProfile | None = None, verbose_heading: bool = True, - ): + ) -> None: # We want to throw early here, since workbook.save() will throw an IndexError otherwise. assert len(selection_list) > 0 @@ -385,7 +385,14 @@ def __init__(self, contribution_results: list[ContributionResult]) -> None: default_sheet_name = _("Text Answers") - def __init__(self, evaluation_name, semester_name, responsibles, results, contributor_name): + def __init__( + self, + evaluation_name: str, + semester_name: str, + responsibles: str, + results: "TextAnswerExporter.InputData", + contributor_name: str | None, + ) -> None: super().__init__() self.evaluation_name = evaluation_name self.semester_name = semester_name @@ -394,7 +401,7 @@ def __init__(self, evaluation_name, semester_name, responsibles, results, contri self.results = results self.contributor_name = contributor_name - def export_impl(self): # pylint: disable=arguments-differ + def export_impl(self) -> None: # pylint: disable=arguments-differ self.cur_sheet.col(0).width = 10000 self.cur_sheet.col(1).width = 40000 diff --git a/evap/results/templatetags/results_templatetags.py b/evap/results/templatetags/results_templatetags.py index 38d9f41162..50185bbe59 100644 --- a/evap/results/templatetags/results_templatetags.py +++ b/evap/results/templatetags/results_templatetags.py @@ -3,8 +3,10 @@ from django.template import Library +from evap.evaluation.models import Evaluation from evap.results.tools import ( STATES_WITH_RESULT_TEMPLATE_CACHING, + Distribution, RatingResult, get_grade_color, normalized_distribution, @@ -14,39 +16,39 @@ @register.filter(name="gradecolor") -def gradecolor(grade): +def gradecolor(grade: float) -> str: return "rgb({}, {}, {})".format(*get_grade_color(grade)) # pylint: disable=consider-using-f-string @register.filter(name="normalized_distribution") -def norm_distribution(distribution): +def norm_distribution(distribution: Distribution) -> Distribution: return normalized_distribution(distribution) @register.filter(name="evaluation_results_cache_timeout") -def evaluation_results_cache_timeout(evaluation): +def evaluation_results_cache_timeout(evaluation: Evaluation) -> int | None: if evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING: return None # cache forever return 0 # don't cache at all @register.filter(name="participationclass") -def participationclass(number_of_voters, number_of_participants): +def participationclass(number_of_voters: int, number_of_participants: int) -> int: return round((number_of_voters / number_of_participants) * 10) @register.filter -def has_answers(rating_result: RatingResult): +def has_answers(rating_result: RatingResult) -> bool: return RatingResult.has_answers(rating_result) @register.filter -def is_published(rating_result: RatingResult): +def is_published(rating_result: RatingResult) -> bool: return RatingResult.is_published(rating_result) @register.filter -def voters_order(evaluation) -> str: +def voters_order(evaluation: Evaluation) -> str: """float to string conversion done in python to circumvent localization breaking number parsing""" return str(evaluation.voter_ratio) diff --git a/evap/results/tools.py b/evap/results/tools.py index 6fdf8ae064..23b9e8935b 100644 --- a/evap/results/tools.py +++ b/evap/results/tools.py @@ -4,15 +4,17 @@ from copy import copy from enum import Enum from math import ceil, modf -from typing import TypeGuard, cast +from typing import TYPE_CHECKING, TypeGuard, cast, Any from django.conf import settings from django.core.cache import caches -from django.db.models import Exists, OuterRef, Sum, prefetch_related_objects +from django.db.models import Exists, OuterRef, QuerySet, Sum, prefetch_related_objects from evap.evaluation.models import ( CHOICES, NO_ANSWER, + BipolarChoices, + Choices, Contribution, Course, Evaluation, @@ -25,6 +27,9 @@ from evap.evaluation.tools import discard_cached_related_objects from evap.tools import unordered_groupby +if TYPE_CHECKING: + from numbers import Real + STATES_WITH_RESULTS_CACHING = {Evaluation.State.EVALUATED, Evaluation.State.REVIEWED, Evaluation.State.PUBLISHED} STATES_WITH_RESULT_TEMPLATE_CACHING = {Evaluation.State.PUBLISHED} @@ -59,7 +64,11 @@ def __init__(self, visible_by_contribution: Iterable[UserProfile], visible_by_de self.visible_by_delegation_count = visible_by_delegation_count -def create_rating_result(question: Question, answer_counters, additional_text_result=None): +def create_rating_result( + question: Question, + answer_counters: Iterable[RatingAnswerCounter] | None, + additional_text_result: "TextResult | None" = None, +) -> "RatingResult": if answer_counters is None: return RatingResult(question, additional_text_result) if any(counter.count != 0 for counter in answer_counters): @@ -69,14 +78,18 @@ def create_rating_result(question: Question, answer_counters, additional_text_re class RatingResult: @classmethod - def is_published(cls, rating_result) -> TypeGuard["PublishedRatingResult"]: + def is_published( + cls, rating_result: "RatingResult | TextResult | HeadingResult" + ) -> TypeGuard["PublishedRatingResult"]: return isinstance(rating_result, PublishedRatingResult) @classmethod - def has_answers(cls, rating_result) -> TypeGuard["AnsweredRatingResult"]: + def has_answers( + cls, rating_result: "RatingResult | TextResult | HeadingResult" + ) -> TypeGuard["AnsweredRatingResult"]: return isinstance(rating_result, AnsweredRatingResult) - def __init__(self, question: Question, additional_text_result=None) -> None: + def __init__(self, question: Question, additional_text_result: "TextResult | None" = None) -> None: assert question.is_rating_question self.question = discard_cached_related_objects(copy(question)) self.additional_text_result = additional_text_result @@ -86,12 +99,17 @@ def __init__(self, question: Question, additional_text_result=None) -> None: self.warning = False @property - def choices(self): + def choices(self) -> Choices | BipolarChoices: return CHOICES[self.question.type] class PublishedRatingResult(RatingResult): - def __init__(self, question: Question, answer_counters, additional_text_result=None) -> None: + def __init__( + self, + question: Question, + answer_counters: Iterable[RatingAnswerCounter], + additional_text_result: "TextResult | None" = None, + ) -> None: super().__init__(question, additional_text_result) counts = OrderedDict( (value, [0, name, color, value]) for (name, color, value) in self.choices.as_name_color_value_tuples() @@ -100,7 +118,7 @@ def __init__(self, question: Question, answer_counters, additional_text_result=N for answer_counter in answer_counters: assert counts[answer_counter.answer][0] == 0 counts[answer_counter.answer][0] = answer_counter.count - self.counts = tuple(count for count, _, _, _ in counts.values()) + self.counts = tuple(cast("int", count) for count, _, _, _ in counts.values()) self.zipped_choices = tuple(counts.values()) @property @@ -194,7 +212,7 @@ def get_results_cache_key(evaluation: Evaluation) -> str: return f"evap.staff.results.tools.get_results-{evaluation.id:d}" -def cache_results(evaluation, *, refetch_related_objects=True): +def cache_results(evaluation: Evaluation, *, refetch_related_objects: bool = True) -> None: assert evaluation.state in STATES_WITH_RESULTS_CACHING cache_key = get_results_cache_key(evaluation) caches["results"].set(cache_key, _get_results_impl(evaluation, refetch_related_objects=refetch_related_objects)) @@ -273,13 +291,16 @@ def _get_results_impl(evaluation: Evaluation, *, refetch_related_objects: bool = return EvaluationResult(contributor_contribution_results) -def annotate_distributions_and_grades(evaluations): +def annotate_distributions_and_grades(evaluations: Any) -> None: for evaluation in evaluations: evaluation.distribution = calculate_average_distribution(evaluation) evaluation.avg_grade = distribution_to_grade(evaluation.distribution) -def normalized_distribution(distribution): +type Distribution = tuple[float, ...] | None + + +def normalized_distribution(distribution: Iterable[float] | None) -> Distribution: """Returns a normalized distribution with the individual values adding up to 1. Can also be used to convert counts to a distribution.""" if distribution is None: @@ -292,12 +313,13 @@ def normalized_distribution(distribution): return tuple((value / distribution_sum) for value in distribution) -def unipolarized_distribution(result): - summed_distribution = [0, 0, 0, 0, 0] +def unipolarized_distribution(result: PublishedRatingResult) -> Distribution: + summed_distribution: list[float] = [0, 0, 0, 0, 0] if not result.counts: return None + grade: float | Real for counts, grade in zip(result.counts, result.choices.grades, strict=True): grade_fraction, grade = modf(grade) grade = int(grade) @@ -308,11 +330,11 @@ def unipolarized_distribution(result): return normalized_distribution(summed_distribution) -def avg_distribution(weighted_distributions): +def avg_distribution(weighted_distributions: Iterable[tuple[tuple[float, ...] | None, float]]) -> Distribution: if all(distribution is None for distribution, __ in weighted_distributions): return None - summed_distribution = [0, 0, 0, 0, 0] + summed_distribution: list[float] = [0, 0, 0, 0, 0] for distribution, weight in weighted_distributions: if distribution: for index, value in enumerate(distribution): @@ -320,27 +342,37 @@ def avg_distribution(weighted_distributions): return normalized_distribution(summed_distribution) -def average_grade_questions_distribution(results): +def average_grade_questions_distribution(results: Iterable[RatingResult | HeadingResult | TextResult]) -> Distribution: return avg_distribution( [ - (unipolarized_distribution(result), result.count_sum) + ( + unipolarized_distribution(cast("PublishedRatingResult", result)), + cast("PublishedRatingResult", result).count_sum, + ) for result in results if result.question.is_grade_question ] ) -def average_non_grade_rating_questions_distribution(results): +def average_non_grade_rating_questions_distribution( + results: Iterable[RatingResult | HeadingResult | TextResult], +) -> Distribution: return avg_distribution( [ - (unipolarized_distribution(result), result.count_sum) + ( + unipolarized_distribution(cast("PublishedRatingResult", result)), + cast("PublishedRatingResult", result).count_sum, + ) for result in results if result.question.is_non_grade_rating_question ] ) -def calculate_average_course_distribution(course, check_for_unpublished_evaluations=True): +def calculate_average_course_distribution( + course: Course, check_for_unpublished_evaluations: bool = True +) -> Distribution: if check_for_unpublished_evaluations and course.evaluations.exclude(state=Evaluation.State.PUBLISHED).exists(): return None @@ -355,7 +387,7 @@ def calculate_average_course_distribution(course, check_for_unpublished_evaluati ) -def get_evaluations_with_course_result_attributes(evaluations): +def get_evaluations_with_course_result_attributes[T: (QuerySet[Evaluation], list[Evaluation])](evaluations: T) -> T: courses_with_unpublished_evaluations = set( Course.objects.filter(evaluations__in=evaluations) .filter(Exists(Evaluation.objects.filter(course=OuterRef("pk")).exclude(state=Evaluation.State.PUBLISHED))) @@ -371,20 +403,21 @@ def get_evaluations_with_course_result_attributes(evaluations): evaluation_weight_sum_per_course_id = {entry[0]: entry[1] for entry in course_id_evaluation_weight_sum_pairs} for evaluation in evaluations: - if evaluation.course.id in courses_with_unpublished_evaluations: - evaluation.course.not_all_evaluations_are_published = True - evaluation.course.distribution = None + course = cast("Any", evaluation.course) + if course.id in courses_with_unpublished_evaluations: + course.not_all_evaluations_are_published = True + course.distribution = None else: - evaluation.course.distribution = calculate_average_course_distribution(evaluation.course, False) + course.distribution = calculate_average_course_distribution(evaluation.course, False) - evaluation.course.evaluation_count = evaluation.course.evaluations.count() - evaluation.course.avg_grade = distribution_to_grade(evaluation.course.distribution) - evaluation.course.evaluation_weight_sum = evaluation_weight_sum_per_course_id[evaluation.course.id] + course.evaluation_count = evaluation.course.evaluations.count() + course.avg_grade = distribution_to_grade(course.distribution) + course.evaluation_weight_sum = evaluation_weight_sum_per_course_id[evaluation.course.id] return evaluations -def calculate_average_distribution(evaluation): +def calculate_average_distribution(evaluation: Evaluation) -> Distribution: assert evaluation.state >= Evaluation.State.IN_EVALUATION if not evaluation.can_staff_see_average_grade or not evaluation.can_publish_average_grade: @@ -415,7 +448,11 @@ def calculate_average_distribution(evaluation): ] ), max( - (result.count_sum for result in contributor_results if result.question.is_rating_question), + ( + cast("PublishedRatingResult", result).count_sum + for result in contributor_results + if result.question.is_rating_question + ), default=0, ), ) @@ -435,19 +472,22 @@ def calculate_average_distribution(evaluation): ) -def distribution_to_grade(distribution): +def distribution_to_grade(distribution: Distribution) -> float | None: if distribution is None: return None return sum(answer * percentage for answer, percentage in enumerate(distribution, start=1)) -def color_mix(color1, color2, fraction): +type Color = tuple[int, int, int] + + +def color_mix(color1: Color, color2: Color, fraction: float) -> Color: return cast( "tuple[int, int, int]", tuple(int(round(color1[i] * (1 - fraction) + color2[i] * fraction)) for i in range(3)) ) -def get_grade_color(grade): +def get_grade_color(grade: float) -> Color: # Can happen if no one leaves any grades. Return white because it least likely causes problems. if not grade: return (255, 255, 255) diff --git a/evap/results/views.py b/evap/results/views.py index 4aba0e696f..96f422d936 100644 --- a/evap/results/views.py +++ b/evap/results/views.py @@ -1,11 +1,13 @@ from collections import defaultdict from statistics import median +from typing import TYPE_CHECKING, cast from django.conf import settings from django.core.cache import caches from django.core.cache.utils import make_template_fragment_key from django.core.exceptions import BadRequest, PermissionDenied from django.db.models import Count, QuerySet +from django.http import HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, render from django.template.loader import get_template from django.utils import translation @@ -16,7 +18,10 @@ from evap.results.exporters import TextAnswerExporter from evap.results.tools import ( STATES_WITH_RESULT_TEMPLATE_CACHING, + ContributionResult, + EvaluationResult, HeadingResult, + QuestionnaireResult, RatingResult, TextResult, ViewContributorResults, @@ -28,40 +33,47 @@ ) from evap.tools import unordered_groupby +if TYPE_CHECKING: + from typing import Any -def get_course_result_template_fragment_cache_key(course_id, language): + from evap.results.tools import PublishedRatingResult + + +def get_course_result_template_fragment_cache_key(course_id: int, language: str) -> str: return make_template_fragment_key("course_result_template_fragment", [course_id, language]) -def get_evaluation_result_template_fragment_cache_key(evaluation_id, language, links_to_results_page): +def get_evaluation_result_template_fragment_cache_key( + evaluation_id: int, language: str, links_to_results_page: bool +) -> str: return make_template_fragment_key( "evaluation_result_template_fragment", [evaluation_id, language, links_to_results_page] ) -def delete_template_cache(evaluation): +def delete_template_cache(evaluation: Evaluation) -> None: assert evaluation.state not in STATES_WITH_RESULT_TEMPLATE_CACHING _delete_template_cache_impl(evaluation) -def _delete_template_cache_impl(evaluation): +def _delete_template_cache_impl(evaluation: Evaluation) -> None: _delete_evaluation_template_cache_impl(evaluation) _delete_course_template_cache_impl(evaluation.course) -def _delete_evaluation_template_cache_impl(evaluation): +def _delete_evaluation_template_cache_impl(evaluation: Evaluation) -> None: caches["results"].delete(get_evaluation_result_template_fragment_cache_key(evaluation.id, "en", True)) caches["results"].delete(get_evaluation_result_template_fragment_cache_key(evaluation.id, "en", False)) caches["results"].delete(get_evaluation_result_template_fragment_cache_key(evaluation.id, "de", True)) caches["results"].delete(get_evaluation_result_template_fragment_cache_key(evaluation.id, "de", False)) -def _delete_course_template_cache_impl(course): +def _delete_course_template_cache_impl(course: Course) -> None: caches["results"].delete(get_course_result_template_fragment_cache_key(course.id, "en")) caches["results"].delete(get_course_result_template_fragment_cache_key(course.id, "de")) -def update_template_cache(evaluations): +def update_template_cache[T: (QuerySet[Evaluation], list[Evaluation])](evaluations: T) -> None: assert all(evaluation.state in STATES_WITH_RESULT_TEMPLATE_CACHING for evaluation in evaluations) evaluations = get_evaluations_with_course_result_attributes(get_evaluations_with_prefetched_data(evaluations)) @@ -100,7 +112,7 @@ def update_template_cache(evaluations): translation.activate(current_language) # reset to previously set language to prevent unwanted side effects -def update_template_cache_of_published_evaluations_in_course(course): +def update_template_cache_of_published_evaluations_in_course(course: Course) -> None: # Delete template caches for evaluations that no longer need to be cached (e.g. after unpublishing) _delete_course_template_cache_impl(course) @@ -108,8 +120,8 @@ def update_template_cache_of_published_evaluations_in_course(course): update_template_cache(course_evaluations) -def get_evaluations_with_prefetched_data(evaluations): - if isinstance(evaluations, QuerySet): # type: ignore[misc] +def get_evaluations_with_prefetched_data[T: (list[Evaluation], QuerySet[Evaluation])](evaluations: T) -> T: + if isinstance(evaluations, QuerySet): evaluations = evaluations.select_related("course__type").prefetch_related( "course__programs", "course__semester", @@ -123,11 +135,17 @@ def get_evaluations_with_prefetched_data(evaluations): @internal_required -def index(request): +def index(request: HttpRequest) -> HttpResponse: + assert isinstance(request.user, UserProfile) + semesters = Semester.get_all_with_published_unarchived_results() - evaluations = Evaluation.objects.filter(course__semester__in=semesters, state=Evaluation.State.PUBLISHED) - evaluations = evaluations.select_related("course", "course__semester") - evaluations = [evaluation for evaluation in evaluations if evaluation.can_be_seen_by(request.user)] + evaluations = [ + evaluation + for evaluation in Evaluation.objects.filter( + course__semester__in=semesters, state=Evaluation.State.PUBLISHED + ).select_related("course", "course__semester") + if evaluation.can_be_seen_by(request.user) + ] if request.user.is_reviewer: additional_evaluations = get_evaluations_with_prefetched_data( @@ -136,8 +154,7 @@ def index(request): state__in=[Evaluation.State.IN_EVALUATION, Evaluation.State.EVALUATED, Evaluation.State.REVIEWED], ) ) - additional_evaluations = get_evaluations_with_course_result_attributes(additional_evaluations) - evaluations += additional_evaluations + evaluations += get_evaluations_with_course_result_attributes(additional_evaluations) # put evaluations into a dict that maps from course to a list of evaluations. # this dict is sorted by course.pk (important for the zip below) @@ -152,7 +169,7 @@ def index(request): Course.objects.filter(pk__in=course_pks).annotate(num_evaluations=Count("evaluations")).order_by("pk").defer() ) for course, annotated_course in zip(courses_and_evaluations.keys(), annotated_courses, strict=True): - course.num_evaluations = annotated_course.num_evaluations + cast("Any", course).num_evaluations = annotated_course.num_evaluations programs = Program.objects.filter(courses__pk__in=course_pks).distinct() course_types = CourseType.objects.filter(courses__pk__in=course_pks).distinct() @@ -165,7 +182,7 @@ def index(request): return render(request, "results_index.html", template_data) -def evaluation_detail(request, semester_id, evaluation_id): +def evaluation_detail(request: HttpRequest, semester_id: int, evaluation_id: int) -> HttpResponse: # pylint: disable=too-many-locals semester = get_object_or_404(Semester, id=semester_id) evaluation = get_object_or_404(semester.evaluations, id=evaluation_id, course__semester=semester) @@ -261,8 +278,12 @@ def evaluation_detail(request, semester_id, evaluation_id): def remove_textanswers_that_the_user_must_not_see( - evaluation_result, user, represented_users, view_general_results, view_contributor_results -): + evaluation_result: EvaluationResult, + user: UserProfile, + represented_users: list[UserProfile], + view_general_results: ViewGeneralResults, + view_contributor_results: ViewContributorResults, +) -> None: for questionnaire_result in evaluation_result.questionnaire_results: for question_result in questionnaire_result.question_results: if isinstance(question_result, TextResult): @@ -282,7 +303,7 @@ def remove_textanswers_that_the_user_must_not_see( ) ] # remove empty TextResults - cleaned_results = [] + cleaned_results: list[TextResult | HeadingResult | RatingResult] = [] for result in questionnaire_result.question_results: if isinstance(result, TextResult): if result.answers: @@ -296,9 +317,9 @@ def remove_textanswers_that_the_user_must_not_see( questionnaire_result.question_results = cleaned_results -def filter_text_answers(evaluation_result): +def filter_text_answers(evaluation_result: EvaluationResult) -> None: for questionnaire_result in evaluation_result.questionnaire_results: - question_results = [] + question_results: list[RatingResult | HeadingResult | TextResult] = [] for result in questionnaire_result.question_results: if isinstance(result, TextResult): question_results.append(result) @@ -307,7 +328,7 @@ def filter_text_answers(evaluation_result): questionnaire_result.question_results = question_results -def exclude_empty_headings(evaluation_result): +def exclude_empty_headings(evaluation_result: EvaluationResult) -> None: for questionnaire_result in evaluation_result.questionnaire_results: filtered_question_results = [] for i, question_result in enumerate(questionnaire_result.question_results): @@ -321,7 +342,7 @@ def exclude_empty_headings(evaluation_result): questionnaire_result.question_results = filtered_question_results -def remove_empty_questionnaire_and_contribution_results(evaluation_result): +def remove_empty_questionnaire_and_contribution_results(evaluation_result: EvaluationResult) -> None: for contribution_result in evaluation_result.contribution_results: contribution_result.questionnaire_results = [ questionnaire_result @@ -335,7 +356,9 @@ def remove_empty_questionnaire_and_contribution_results(evaluation_result): ] -def split_evaluation_result_into_questionnaire_types(evaluation_result, view_as_user, view_contributor_results): +def split_evaluation_result_into_questionnaire_types( + evaluation_result: EvaluationResult, view_as_user: UserProfile, view_contributor_results: ViewContributorResults +) -> tuple[list[QuestionnaireResult], list[QuestionnaireResult], list[ContributionResult], list[QuestionnaireResult]]: top_results = [] bottom_results = [] contributor_results = [] @@ -364,7 +387,8 @@ def split_evaluation_result_into_questionnaire_types(evaluation_result, view_as_ return top_results, bottom_results, contributor_results, dropout_results -def get_evaluations_of_course(course, request): +def get_evaluations_of_course(course: Course, request: HttpRequest) -> list[Evaluation]: + assert isinstance(request.user, UserProfile) course_evaluations = [] if course.evaluations.count() > 1: @@ -383,7 +407,7 @@ def get_evaluations_of_course(course, request): return course_evaluations -def add_warnings(evaluation, evaluation_result): +def add_warnings(evaluation: Evaluation, evaluation_result: EvaluationResult) -> None: if not evaluation.can_publish_rating_results: return @@ -392,7 +416,7 @@ def add_warnings(evaluation, evaluation_result): for questionnaire_result in evaluation_result.questionnaire_results: max_answers = max( ( - question_result.count_sum + cast("PublishedRatingResult", question_result).count_sum for question_result in questionnaire_result.question_results if question_result.question.is_rating_question ), @@ -410,7 +434,7 @@ def add_warnings(evaluation, evaluation_result): if questionnaire_result.questionnaire.is_dropout: continue rating_results = [ - question_result + cast("PublishedRatingResult", question_result) for question_result in questionnaire_result.question_results if question_result.question.is_rating_question ] @@ -427,7 +451,10 @@ def add_warnings(evaluation, evaluation_result): ) -def evaluation_detail_parse_get_parameters(request, evaluation): +def evaluation_detail_parse_get_parameters( + request: HttpRequest, evaluation: Evaluation +) -> tuple[ViewGeneralResults, ViewContributorResults, UserProfile, list[UserProfile], int | None]: + assert isinstance(request.user, UserProfile) if not evaluation.can_results_page_be_seen_by(request.user): raise PermissionDenied @@ -451,7 +478,9 @@ def evaluation_detail_parse_get_parameters(request, evaluation): return view_general_results, view_contributor_results, view_as_user, represented_users, contributor_id -def extract_evaluation_answer_data(request, evaluation): +def extract_evaluation_answer_data( + request: HttpRequest, evaluation: Evaluation +) -> tuple[TextAnswerExporter.InputData, int | None]: # TextAnswerExporter wants a dict from Question to tuple of contributor_name and string list (of the answers) ( view_general_results, @@ -472,7 +501,7 @@ def extract_evaluation_answer_data(request, evaluation): return results, contributor_id -def evaluation_text_answers_export(request, evaluation_id): +def evaluation_text_answers_export(request: HttpRequest, evaluation_id: int) -> HttpResponse: evaluation = get_object_or_404(Evaluation, id=evaluation_id) results, contributor_id = extract_evaluation_answer_data(request, evaluation) diff --git a/evap/rewards/exporters.py b/evap/rewards/exporters.py index 83fc55347c..23c5a66f99 100644 --- a/evap/rewards/exporters.py +++ b/evap/rewards/exporters.py @@ -1,3 +1,5 @@ +from collections.abc import Iterable + from django.utils.translation import gettext as _ from evap.evaluation.tools import ExcelExporter @@ -6,7 +8,7 @@ class RewardsExporter(ExcelExporter): default_sheet_name = _("Redemptions") - def export_impl(self, users_with_redeemed_points): # pylint: disable=arguments-differ + def export_impl(self, users_with_redeemed_points: Iterable) -> None: # pylint: disable=arguments-differ self.write_row( [ _("Last name"), diff --git a/evap/rewards/forms.py b/evap/rewards/forms.py index ce54a4ebdc..88be0019e2 100644 --- a/evap/rewards/forms.py +++ b/evap/rewards/forms.py @@ -1,5 +1,7 @@ +from collections.abc import Generator from contextlib import contextmanager from datetime import date +from typing import Any, cast from django import forms from django.core.exceptions import ValidationError @@ -9,6 +11,7 @@ from evap.evaluation.models import UserProfile from evap.rewards.models import RewardPointRedemption, RewardPointRedemptionEvent from evap.rewards.tools import reward_points_of_user +from evap.tools import assert_not_none class RewardPointRedemptionEventForm(forms.ModelForm): @@ -39,7 +42,7 @@ def __init__(self, *args, **kwargs): help_text=help_text, ) - def clean_event(self): + def clean_event(self) -> RewardPointRedemptionEvent: event = self.cleaned_data["event"] if event.redeem_end_date < date.today(): raise ValidationError(_("Sorry, the deadline for this event expired already.")) @@ -52,16 +55,16 @@ def __init__(self, *args, user: UserProfile, **kwargs) -> None: self.user = user self.locked = False - def get_form_kwargs(self, index): + def get_form_kwargs(self, index: int | None) -> dict[str, Any]: kwargs = super().get_form_kwargs(index) if not self.initial: return kwargs - kwargs["initial"] = self.initial[index] + kwargs["initial"] = self.initial[assert_not_none(index)] kwargs["initial"]["total_points_available"] = reward_points_of_user(self.user) return kwargs @contextmanager - def lock(self): + def lock(self) -> Generator: with transaction.atomic(): # lock these rows to prevent race conditions list(self.user.reward_point_grantings.select_for_update()) @@ -73,7 +76,7 @@ def lock(self): finally: self.locked = False - def clean(self): + def clean(self) -> None: assert self.locked if any(self.errors): @@ -104,6 +107,6 @@ def save(self) -> list[RewardPointRedemption]: return created -RewardPointRedemptionFormSet = forms.formset_factory( +RewardPointRedemptionFormSet = cast("type[BaseRewardPointRedemptionFormSet]", forms.formset_factory( RewardPointRedemptionForm, formset=BaseRewardPointRedemptionFormSet, extra=0 -) +)) diff --git a/evap/rewards/models.py b/evap/rewards/models.py index 937358ee11..8562aa63e1 100644 --- a/evap/rewards/models.py +++ b/evap/rewards/models.py @@ -1,6 +1,6 @@ from django.core.validators import MinValueValidator from django.db import models -from django.db.models import Q, Sum +from django.db.models import Q, QuerySet, Sum from django.dispatch import Signal from django.utils.translation import gettext_lazy as _ @@ -20,10 +20,10 @@ class RewardPointRedemptionEvent(models.Model): ) @property - def can_delete(self): + def can_delete(self) -> bool: return not self.reward_point_redemptions.exists() - def users_with_redeemed_points(self): + def users_with_redeemed_points(self) -> QuerySet[UserProfile]: return UserProfile.objects.filter(reward_point_redemptions__event=self).annotate( points=Sum("reward_point_redemptions__value", default=0, filter=Q(reward_point_redemptions__event=self)) ) diff --git a/evap/rewards/views.py b/evap/rewards/views.py index 8a8a1be43c..afaaad3bc4 100644 --- a/evap/rewards/views.py +++ b/evap/rewards/views.py @@ -1,11 +1,12 @@ import csv from datetime import date, datetime +from typing import Any from django.contrib import messages from django.contrib.messages.views import SuccessMessageMixin from django.core.exceptions import BadRequest, SuspiciousOperation from django.db.models import OuterRef, Subquery, Sum -from django.http import HttpResponse +from django.http import HttpRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse_lazy from django.utils.translation import get_language, gettext_lazy @@ -28,13 +29,15 @@ @reward_user_required -def index(request): +def index(request: HttpRequest) -> HttpResponse: status = 200 + assert isinstance(request.user, UserProfile) + events = RewardPointRedemptionEvent.objects.filter(redeem_end_date__gte=date.today()).order_by("date") # pylint: disable=unexpected-keyword-arg - formset = RewardPointRedemptionFormSet( + formset: Any = RewardPointRedemptionFormSet( request.POST or None, initial=[{"event": e, "points": 0} for e in events], user=request.user, @@ -65,11 +68,11 @@ def index(request): reward_point_grantings = RewardPointGranting.objects.filter(user_profile=request.user) reward_point_redemptions = RewardPointRedemption.objects.filter(user_profile=request.user) - granted_point_actions = [ + granted_point_actions: list[tuple[datetime, str, str | int, str | int]] = [ (granting.granting_time, _("Reward for") + " " + granting.semester.name, granting.value, "") for granting in reward_point_grantings ] - redemption_point_actions = [ + redemption_point_actions: list[tuple[datetime, str, str | int, str | int]] = [ (redemption.redemption_time, redemption.event.name, "", redemption.value) for redemption in reward_point_redemptions ] @@ -90,7 +93,7 @@ def index(request): @manager_required -def reward_point_redemption_events(request): +def reward_point_redemption_events(request: HttpRequest) -> HttpResponse: upcoming_events = RewardPointRedemptionEvent.objects.filter(redeem_end_date__gte=datetime.now()).order_by("date") past_events = RewardPointRedemptionEvent.objects.filter(redeem_end_date__lt=datetime.now()).order_by("-date") total_points_granted = RewardPointGranting.objects.aggregate(Sum("value"))["value__sum"] or 0 @@ -126,7 +129,7 @@ class RewardPointRedemptionEventEditView(SuccessMessageMixin, UpdateView): @require_POST @manager_required -def reward_point_redemption_event_delete(request): +def reward_point_redemption_event_delete(request: HttpRequest) -> HttpResponse: event = get_object_from_dict_pk_entry_or_logged_40x(RewardPointRedemptionEvent, request.POST, "event_id") if not event.can_delete: @@ -136,7 +139,7 @@ def reward_point_redemption_event_delete(request): @manager_required -def reward_point_redemption_event_export(request, event_id): +def reward_point_redemption_event_export(request: HttpRequest, event_id: int) -> HttpResponse: event = get_object_or_404(RewardPointRedemptionEvent, id=event_id) filename = _("RewardPoints") + f"-{event.date}-{event.name}-{get_language()}.xls" @@ -148,7 +151,7 @@ def reward_point_redemption_event_export(request, event_id): @manager_required -def reward_points_export(request): +def reward_points_export(request: HttpRequest) -> HttpResponse: filename = _("RewardPoints") + f"-{get_language()}.csv" response = AttachmentResponse(filename, content_type="text/csv") @@ -183,7 +186,7 @@ def aggregated_sum(relation: str) -> Subquery: @require_POST @manager_required -def semester_activation_edit(request, semester_id): +def semester_activation_edit(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) status = request.POST.get("activation_status") if status == "on": diff --git a/evap/settings.py b/evap/settings.py index 954597ebad..28cee59963 100644 --- a/evap/settings.py +++ b/evap/settings.py @@ -17,6 +17,7 @@ from typing import Any from django.contrib.staticfiles.storage import ManifestStaticFilesStorage +from django.http import HttpRequest from evap.tools import MonthAndDay @@ -435,7 +436,7 @@ class ManifestStaticFilesStorageWithJsReplacement(ManifestStaticFilesStorage): ### Allowed chosen first names / display names -def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name +def CHARACTER_ALLOWED_IN_NAME(character: str) -> bool: # pylint: disable=invalid-name return any( ( ord(character) in range(32, 127), # printable ASCII / Basic Latin characters @@ -478,7 +479,7 @@ def CHARACTER_ALLOWED_IN_NAME(character): # pylint: disable=invalid-name # localsettings file may or may not exist (for example in CI) # the import can overwrite locals with a slightly different type (e.g. DATABASES), which is fine. - from evap.localsettings import * # type: ignore # noqa: F403,PGH003 + from evap.localsettings import * # noqa: F403,PGH003 except ImportError: pass @@ -531,7 +532,7 @@ def gen_string() -> str: INSTALLED_APPS += ["debug_toolbar"] MIDDLEWARE = ["debug_toolbar.middleware.DebugToolbarMiddleware"] + MIDDLEWARE - def show_toolbar(request): + def show_toolbar(request: HttpRequest) -> bool: return True DEBUG_TOOLBAR_CONFIG = { diff --git a/evap/staff/fixtures/excel_files_test_data.py b/evap/staff/fixtures/excel_files_test_data.py index c1d0fc5eb0..bc8b047af6 100644 --- a/evap/staff/fixtures/excel_files_test_data.py +++ b/evap/staff/fixtures/excel_files_test_data.py @@ -1,6 +1,7 @@ import csv import io -from typing import TextIO +from collections.abc import Iterable, Mapping +from typing import Any, TextIO import openpyxl @@ -220,7 +221,7 @@ # fmt: on -def create_memory_excel_file(data): +def create_memory_excel_file(data: Mapping[str, Iterable[Iterable[Any]]]) -> bytes: memory_excel_file = io.BytesIO() workbook = openpyxl.Workbook() for sheet_name, sheet_data in data.items(): @@ -233,7 +234,7 @@ def create_memory_excel_file(data): return memory_excel_file.getvalue() -def create_memory_csv_file(data) -> TextIO: +def create_memory_csv_file(data: Iterable[Iterable[Any]]) -> TextIO: memory_csv_file = io.StringIO() writer = csv.writer(memory_csv_file, delimiter=";", lineterminator="\n") writer.writerows(data) diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 7ba6104d52..57dd4741d4 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -1,5 +1,6 @@ import logging from collections.abc import Iterable +from typing import Any, Container, cast from django import forms from django.contrib.auth.models import Group @@ -7,8 +8,9 @@ from django.db import transaction from django.db.models import Max, Q from django.forms.models import BaseInlineFormSet -from django.forms.widgets import CheckboxSelectMultiple +from django.forms.widgets import CheckboxSelectMultiple, ChoiceWidget, Widget from django.http.request import QueryDict +from django.utils.safestring import SafeString from django.utils.text import normalize_newlines from django.utils.translation import gettext_lazy as _ @@ -37,11 +39,12 @@ from evap.results.views import update_template_cache, update_template_cache_of_published_evaluations_in_course from evap.staff.tools import remove_user_from_represented_and_ccing_users from evap.student.models import TextAnswerWarning +from evap.tools import assert_not_none logger = logging.getLogger(__name__) -def disable_all_fields(form): +def disable_all_fields(form: forms.ModelForm) -> None: for field in form.fields.values(): field.disabled = True @@ -53,25 +56,26 @@ class CharArrayField(forms.Field): "invalid_list": _("Enter a list of values."), } - def __init__(self, base_field, *, max_length=None, **kwargs): + def __init__(self, base_field: forms.Field, *, max_length: int | None = None, **kwargs) -> None: super().__init__(**kwargs) assert isinstance(base_field, forms.CharField) assert max_length is None - def to_python(self, value): + def to_python(self, value: Any) -> list[str]: if not value: return [] if not isinstance(value, Iterable): raise ValidationError(self.error_messages["invalid_list"], code="invalid_list") return [str(val) for val in value] - def get_bound_field(self, form, field_name): + def get_bound_field(self, form: forms.BaseForm, field_name: str) -> "BoundCharArrayField": return BoundCharArrayField(form, self, field_name) class BoundCharArrayField(forms.BoundField): - def as_widget(self, widget=None, attrs=None, only_initial=False): + def as_widget(self, widget: Widget | None = None, attrs: Any = None, only_initial: bool = False) -> SafeString: widget = widget or self.field.widget + assert isinstance(widget, ChoiceWidget) # Inject all current values as choices so they don’t get discarded if self.value(): widget.choices = [(value, value) for value in self.value()] @@ -83,11 +87,11 @@ class ModelWithImportNamesFormset(forms.BaseModelFormSet): A form set which validates that import names are not duplicated """ - def clean(self): + def clean(self) -> None: super().clean() import_names = set() for form in self.forms: - if self.can_delete and self._should_delete_form(form): + if self.can_delete and self._should_delete_form(form): # type: ignore[attr-defined] continue for import_name in form.cleaned_data.get("import_names", []): if import_name.lower() in import_names: @@ -126,13 +130,13 @@ class EvaluationParticipantCopyForm(forms.Form): Evaluation.objects.all(), empty_label="", required=False, label=_("Evaluation") ) - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self.evaluation_selection_required = False # Here we split the evaluations by semester and create supergroups for them. We also make sure to include an empty option. - choices = [("", "")] + choices: list[tuple[str, str | list[tuple[int, str]]]] = [("", "")] for semester in Semester.objects.all(): evaluation_choices = [ (evaluation.pk, evaluation.full_name) @@ -141,10 +145,10 @@ def __init__(self, *args, **kwargs): if evaluation_choices: choices += [(semester.name, evaluation_choices)] - self.fields["evaluation"].choices = choices + cast(forms.ModelChoiceField, self.fields["evaluation"]).choices = choices self.fields["evaluation"].widget.attrs["tomselect-no-sort"] = "" - def clean(self): + def clean(self) -> None: if self.evaluation_selection_required and self.cleaned_data.get("evaluation") is None: raise ValidationError(_("Please select an evaluation from the dropdown menu.")) @@ -161,11 +165,11 @@ class Meta: fields = ("name_de", "name_en", "short_name_de", "short_name_en", "cms_name", "default_course_end_date") localized_fields = ("default_course_end_date",) - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self.fields["default_course_end_date"].required = False - def save(self, commit=True): + def save(self, commit: bool = True) -> Semester: semester = super().save(commit) if "short_name_en" in self.changed_data or "short_name_de" in self.changed_data: update_template_cache(semester.evaluations.filter(state__in=STATES_WITH_RESULT_TEMPLATE_CACHING)) @@ -183,12 +187,12 @@ class Meta: "order": forms.HiddenInput(), } - def clean(self): + def clean(self) -> None: super().clean() if self.cleaned_data.get("DELETE") and not self.instance.can_be_deleted_by_manager: raise SuspiciousOperation("Deleting program not allowed") - def save(self, *args, **kwargs): + def save(self, *args, **kwargs) -> Program: program = super().save(*args, **kwargs) if "name_en" in self.changed_data or "name_de" in self.changed_data: update_template_cache( @@ -208,12 +212,12 @@ class Meta: "order": forms.HiddenInput(), } - def clean(self): + def clean(self) -> None: super().clean() if self.cleaned_data.get("DELETE") and not self.instance.can_be_deleted_by_manager: raise SuspiciousOperation("Deleting course type not allowed") - def save(self, *args, **kwargs): + def save(self, *args, **kwargs) -> CourseType: course_type = super().save(*args, **kwargs) if "name_en" in self.changed_data or "name_de" in self.changed_data: update_template_cache( @@ -233,12 +237,12 @@ class Meta: "order": forms.HiddenInput(), } - def clean(self): + def clean(self) -> None: super().clean() if self.cleaned_data.get("DELETE") and not self.instance.can_be_deleted_by_manager: raise SuspiciousOperation("Deleting exam type not allowed") - def save(self, *args, **kwargs): + def save(self, *args, **kwargs) -> ExamType: exam_type = super().save(*args, **kwargs) if "name_en" in self.changed_data or "name_de" in self.changed_data: update_template_cache( @@ -252,7 +256,7 @@ class ProgramMergeSelectionForm(forms.Form): main_instance = forms.ModelChoiceField(Program.objects.all(), label=_("Main program")) other_instance = forms.ModelChoiceField(Program.objects.all(), label=_("Other program")) - def clean(self): + def clean(self) -> None: super().clean() if self.cleaned_data.get("main_instance") == self.cleaned_data.get("other_instance"): raise ValidationError(_("You must select two different programs.")) @@ -262,7 +266,7 @@ class CourseTypeMergeSelectionForm(forms.Form): main_type = forms.ModelChoiceField(CourseType.objects.all(), label=_("Main type")) other_type = forms.ModelChoiceField(CourseType.objects.all(), label=_("Other type")) - def clean(self): + def clean(self) -> None: super().clean() if self.cleaned_data.get("main_type") == self.cleaned_data.get("other_type"): raise ValidationError(_("You must select two different course types.")) @@ -284,11 +288,11 @@ class Meta: "responsibles": UserModelMultipleChoiceField, } - def _set_responsibles_queryset(self, existing_course=None): + def _set_responsibles_queryset(self, existing_course: Course | None = None) -> None: queryset = UserProfile.objects.exclude(is_active=False) if existing_course: queryset = (queryset | existing_course.responsibles.all()).distinct() - self.fields["responsibles"].queryset = queryset + self.fields["responsibles"].queryset = queryset # type: ignore[attr-defined] def validate_unique(self): super().validate_unique() @@ -303,24 +307,24 @@ def validate_unique(self): self.add_error(name_field, e) -class CourseForm(CourseFormMixin, forms.ModelForm): # type: ignore[misc] +class CourseForm(CourseFormMixin, forms.ModelForm): semester = forms.ModelChoiceField(Semester.objects.all(), disabled=True, required=False, widget=forms.HiddenInput()) - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self._set_responsibles_queryset(self.instance if self.instance.pk else None) if not self.instance.can_be_edited_by_manager: disable_all_fields(self) -class CourseCopyForm(CourseFormMixin, forms.ModelForm): # type: ignore[misc] +class CourseCopyForm(CourseFormMixin, forms.ModelForm): semester = forms.ModelChoiceField(Semester.objects.all()) vote_start_datetime = forms.DateTimeField(label=_("Start of evaluations"), localize=True) vote_end_date = forms.DateField(label=_("Last day of evaluations"), localize=True) field_order = ["semester"] - def __init__(self, data=None, *, instance: Course): + def __init__(self, data: dict[str, Any] | None = None, *, instance: Course) -> None: self.old_course = instance opts = self._meta initial = forms.models.model_to_dict(instance, opts.fields, opts.exclude) @@ -375,7 +379,7 @@ def __init__(self, data=None, *, instance: Course): } @transaction.atomic() - def save(self, commit=True) -> Course: + def save(self, commit: bool = True) -> Course: new_course: Course = super().save() # we need to create copies of evaluations and their participation as well for old_evaluation in self.old_course.evaluations.all(): @@ -519,7 +523,7 @@ def clean_main_language(self): self.add_error("main_language", _("You have to set a main language for this evaluation.")) return main_language - def clean(self): + def clean(self) -> None: super().clean() vote_start_datetime = self.cleaned_data.get("vote_start_datetime") @@ -528,10 +532,10 @@ def clean(self): self.add_error("vote_start_datetime", "") self.add_error("vote_end_date", _("The first day of evaluation must be before the last one.")) - def save(self, *args, **kw): + def save(self, *args, **kw) -> Evaluation: evaluation = super().save(*args, **kw) - selected_questionnaires = self.cleaned_data.get("general_questionnaires") | self.cleaned_data.get( - "dropout_questionnaires" + selected_questionnaires = ( + self.cleaned_data["general_questionnaires"] | self.cleaned_data["dropout_questionnaires"] ) removed_questionnaires = set(self.instance.general_contribution.questionnaires.all()) - set( selected_questionnaires @@ -546,7 +550,7 @@ def save(self, *args, **kw): class EvaluationCopyForm(EvaluationForm): - def __init__(self, data=None, instance=None): + def __init__(self, data=None, instance=None) -> None: opts = self._meta initial = forms.models.model_to_dict(instance, opts.fields, opts.exclude) initial["general_questionnaires"] = instance.general_contribution.questionnaires.all() @@ -562,7 +566,7 @@ class ExamEvaluationForm(forms.Form): ) exam_type = forms.ModelChoiceField(ExamType.objects.all(), required=True, label=_("Exam type")) - def __init__(self, *args, evaluation=None, form_id=None, **kwargs): + def __init__(self, *args, evaluation=None, form_id=None, **kwargs) -> None: super().__init__(*args, **kwargs) if form_id is not None: for field in self.fields.values(): @@ -635,12 +639,12 @@ def __init__(self, *args, evaluation=None, **kwargs): # form is used as read-only evaluation view disable_all_fields(self) - def clean(self): + def clean(self) -> None: if not self.cleaned_data.get("does_not_contribute") and not self.cleaned_data.get("questionnaires"): self.add_error("does_not_contribute", _("Select either this option or at least one questionnaire!")) @property - def show_delete_button(self): + def show_delete_button(self) -> bool: if self.instance.pk is None: return True # not stored in the DB. Required so temporary instances in the formset can be deleted. @@ -677,11 +681,11 @@ class EvaluationEmailForm(forms.Form): plain_content = forms.CharField(widget=forms.Textarea(), label=_("Plain Text")) html_content = forms.CharField(widget=forms.Textarea(), label=_("HTML")) - def __init__(self, *args, evaluation, export=False, **kwargs): + def __init__(self, *args, evaluation: Evaluation, export: bool = False, **kwargs) -> None: super().__init__(*args, **kwargs) self.template = EmailTemplate() self.evaluation = evaluation - self.recipient_groups = None + self.recipient_groups: Container[EmailTemplate.Recipients] | None = None self.fields["subject"].required = not export self.fields["plain_content"].required = not export self.fields["html_content"].required = False @@ -694,9 +698,9 @@ def clean(self): return self.cleaned_data - def email_addresses(self): + def email_addresses(self) -> set[str]: recipients = self.template.recipient_list_for_evaluation( - self.evaluation, self.recipient_groups, filter_users_in_cc=False + self.evaluation, assert_not_none(self.recipient_groups), filter_users_in_cc=False ) return {user.email for user in recipients if user.email} @@ -777,7 +781,7 @@ def save(self, *args, commit=True, force_highest_order=False, **kwargs): class AtLeastOneFormset(BaseInlineFormSet): - def clean(self): + def clean(self) -> None: super().clean() count = 0 for form in self.forms: @@ -789,13 +793,13 @@ def clean(self): class ContributionFormset(BaseInlineFormSet): - def __init__(self, data=None, **kwargs): + def __init__(self, data=None, **kwargs) -> None: data = self.handle_moved_contributors(data, **kwargs) super().__init__(data, **kwargs) if self.instance.pk is not None: self.queryset = self.instance.contributions.exclude(contributor=None) - def handle_deleted_and_added_contributions(self): + def handle_deleted_and_added_contributions(self) -> None: """ If a contributor got removed and added in the same formset, django would usually complain when validating the added form, as it does not check whether the existing contribution was deleted. @@ -868,7 +872,7 @@ def handle_moved_contributors(data, **kwargs): break return data - def clean(self): + def clean(self) -> None: self.handle_deleted_and_added_contributions() found_contributor = set() for form in self.forms: @@ -891,7 +895,7 @@ def clean(self): class ContributionCopyFormset(ContributionFormset): - def __init__(self, data, instance, new_instance): + def __init__(self, data, instance: Contribution, new_instance: Evaluation) -> None: # First, pass the old evaluation instance to create a ContributionCopyForm for each contribution super().__init__(data, instance=instance, form_kwargs={"evaluation": new_instance}) # Then, use the new evaluation instance as target for validation and saving purposes @@ -914,7 +918,7 @@ class Meta: "text_en": forms.Textarea(attrs={"rows": 2}), } - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) if self.instance.pk and self.instance.type in [QuestionType.TEXT, QuestionType.HEADING] and not self.data: self.fields["allows_additional_textanswers"].disabled = True # disable only for frontend; validate in clean @@ -941,8 +945,8 @@ class Meta: "order": forms.HiddenInput(), } - def __init__(self, *args, instance=None, **kwargs): - super().__init__(*args, instance=instance, **kwargs) + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) if hasattr(self.instance, "question"): self.question_form = QuestionForm(*args, instance=self.instance.question, **kwargs) else: @@ -963,7 +967,7 @@ def save(self, *args, **kwargs): class QuestionnairesAssignForm(forms.Form): - def __init__(self, *args, course_types, exam_types, **kwargs): + def __init__(self, *args, course_types: Iterable[CourseType], exam_types: Iterable[ExamType], **kwargs) -> None: super().__init__(*args, **kwargs) contributor_questionnaires = Questionnaire.objects.contributor_questionnaires().exclude( @@ -1022,12 +1026,14 @@ class Meta: "cc_users": UserModelMultipleChoiceField, } - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) - self.user_with_same_email = None + self.user_with_same_email: UserProfile | None = None evaluations_in_active_semester = Evaluation.objects.filter(course__semester=Semester.active_semester()) - self.fields["evaluations_participating_in"].queryset = evaluations_in_active_semester - self.remove_messages = [] + cast( + forms.ModelMultipleChoiceField, self.fields["evaluations_participating_in"] + ).queryset = evaluations_in_active_semester + self.remove_messages: list[str] = [] if self.instance.pk: self.fields["evaluations_participating_in"].initial = evaluations_in_active_semester.filter( participants=self.instance @@ -1053,7 +1059,7 @@ def clean_evaluations_participating_in(self): return evaluations_participating_in - def clean_email(self): + def clean_email(self) -> str | None: email = clean_email(self.cleaned_data.get("email")) if email is None: return None diff --git a/evap/staff/importers/base.py b/evap/staff/importers/base.py index 141e0c955b..632b6f7e5b 100644 --- a/evap/staff/importers/base.py +++ b/evap/staff/importers/base.py @@ -10,6 +10,7 @@ import openpyxl from django.conf import settings from django.contrib import messages +from django.http import HttpRequest from django.utils.translation import gettext as _ from django.utils.translation import gettext_lazy, ngettext @@ -78,7 +79,7 @@ def _messages_with_level_by_category( grouped_messages = itertools.groupby(sorted_messages, lambda msg: msg.category) return {category: list(messages) for category, messages in grouped_messages} - def add_message(self, message: ImporterLogEntry): + def add_message(self, message: ImporterLogEntry) -> None: self.messages.append(message) def has_errors(self) -> bool: @@ -97,7 +98,7 @@ def warnings_by_category(self) -> dict[ImporterLogEntry.Category, list[ImporterL def errors_by_category(self) -> dict[ImporterLogEntry.Category, list[ImporterLogEntry]]: return self._messages_with_level_by_category(ImporterLogEntry.Level.ERROR) - def forward_messages_to_django(self, request) -> None: + def forward_messages_to_django(self, request: HttpRequest) -> None: method_by_level = { ImporterLogEntry.Level.SUCCESS: messages.success, ImporterLogEntry.Level.WARNING: messages.warning, @@ -107,13 +108,19 @@ def forward_messages_to_django(self, request) -> None: for message in self.messages: method_by_level[message.level](request, message.message) - def add_error(self, message_text, *, category=ImporterLogEntry.Category.GENERAL): + def add_error( + self, message_text: str, *, category: ImporterLogEntry.Category = ImporterLogEntry.Category.GENERAL + ) -> None: return self.add_message(ImporterLogEntry(ImporterLogEntry.Level.ERROR, category, message_text)) - def add_warning(self, message_text, *, category=ImporterLogEntry.Category.GENERAL): + def add_warning( + self, message_text: str, *, category: ImporterLogEntry.Category = ImporterLogEntry.Category.GENERAL + ) -> None: return self.add_message(ImporterLogEntry(ImporterLogEntry.Level.WARNING, category, message_text)) - def add_success(self, message_text, *, category=ImporterLogEntry.Category.GENERAL): + def add_success( + self, message_text: str, *, category: ImporterLogEntry.Category = ImporterLogEntry.Category.GENERAL + ) -> None: return self.add_message(ImporterLogEntry(ImporterLogEntry.Level.SUCCESS, category, message_text)) @@ -141,7 +148,7 @@ class ConvertExceptionsToMessages: def __init__(self, importer_log: ImporterLog): self.importer_log = importer_log - def __enter__(self): + def __enter__(self) -> None: pass def __exit__(self, exc_type, exc_value, traceback) -> bool: @@ -184,7 +191,7 @@ def __init__(self, location: ExcelFileLocation, *cells: Iterable[str]): pass @classmethod - def from_cells(cls, location: ExcelFileLocation, cells: Iterable[str]): + def from_cells(cls, location: ExcelFileLocation, cells: Iterable[str]) -> "InputRow": return cls(location, *cells) @@ -198,7 +205,7 @@ def __init__(self, skip_first_n_rows: int, row_cls: type[InputRow], importer_log self.row_cls = row_cls self.importer_log = importer_log - def map(self, file_content: bytes): + def map(self, file_content: bytes) -> list: try: book = openpyxl.load_workbook(BytesIO(file_content)) except Exception as e: # noqa: BLE001 @@ -257,7 +264,7 @@ def __init__(self, *args, **kwargs) -> None: self.first_location_by_key: dict[Any, ExcelFileLocation] = {} self.location_count_by_key: Counter = Counter() - def add_location_for_key(self, location: ExcelFileLocation, key: Any): + def add_location_for_key(self, location: ExcelFileLocation, key: Any) -> None: self.first_location_by_key.setdefault(key, location) self.location_count_by_key.update([key]) diff --git a/evap/staff/importers/enrollment.py b/evap/staff/importers/enrollment.py index de1d391cce..a36cb5f595 100644 --- a/evap/staff/importers/enrollment.py +++ b/evap/staff/importers/enrollment.py @@ -555,7 +555,7 @@ def finalize(self) -> None: seen_evaluation_names = self.participant_emails_per_course_name_en.keys() existing_participation_pairs = [ - (participation.evaluation.course.name_en, participation.userprofile.email) # type: ignore[misc] + (participation.evaluation.course.name_en, participation.userprofile.email) for participation in Evaluation.participants.through._default_manager.filter( evaluation__course__name_en__in=seen_evaluation_names, userprofile__email__in=seen_user_emails ).prefetch_related("userprofile", "evaluation__course") diff --git a/evap/staff/importers/user.py b/evap/staff/importers/user.py index 45549314ea..51f82e994d 100644 --- a/evap/staff/importers/user.py +++ b/evap/staff/importers/user.py @@ -7,6 +7,7 @@ from django.db import transaction from django.db.models import Q from django.utils.html import escape, format_html +from django.utils.safestring import SafeString from django.utils.translation import gettext as _ from django.utils.translation import ngettext @@ -38,7 +39,7 @@ class UserData: email: str @staticmethod - def bulk_update_fields(): + def bulk_update_fields() -> list[str]: """Fields passed to bulk_update when updating existing users with new UserData""" return ["first_name_given", "last_name", "title", "email", "is_active"] @@ -49,7 +50,7 @@ def __init__(self, first_name: str, last_name: str, title: str, email: str): object.__setattr__(self, "title", title.strip()) object.__setattr__(self, "email", clean_email(email)) - def apply_to_and_make_active(self, user_profile: UserProfile): + def apply_to_and_make_active(self, user_profile: UserProfile) -> None: """Intended to update existing UserProfile entries from the database. email is not touched""" user_profile.first_name_given = self.first_name user_profile.last_name = self.last_name @@ -109,7 +110,7 @@ def as_parsed_row(self) -> UserParsedRow: class UserDataEmptyFieldsChecker(Checker): """Assert email, first name and last name are not empty""" - def check_userdata(self, user_data: UserData, location: ExcelFileLocation): + def check_userdata(self, user_data: UserData, location: ExcelFileLocation) -> None: if user_data.email == "": self.importer_log.add_error( _("{location}: Email address is missing.").format(location=location), @@ -139,7 +140,7 @@ def __init__(self, *args, **kwargs) -> None: self.in_file_mismatch_tracker = FirstLocationAndCountTracker() - def check_userdata(self, user_data: UserData, location: ExcelFileLocation): + def check_userdata(self, user_data: UserData, location: ExcelFileLocation) -> None: if user_data.email == "": # UserDataEmptyFieldsChecker will give an error for these, no need to spam additional errors return @@ -197,7 +198,7 @@ def finalize(self) -> None: if existing_db_users: self._add_user_name_collision_warning(user_data, existing_db_users) - def _add_user_data_mismatch_warning(self, user: UserProfile, user_data: UserData): + def _add_user_data_mismatch_warning(self, user: UserProfile, user_data: UserData) -> None: if self.test_run: msg = escape(_("The existing user would be overwritten with the following data:")) else: @@ -211,7 +212,7 @@ def _add_user_data_mismatch_warning(self, user: UserProfile, user_data: UserData self.importer_log.add_warning(msg, category=ImporterLogEntry.Category.NAME) - def _add_user_inactive_warning(self, user: UserProfile): + def _add_user_inactive_warning(self, user: UserProfile) -> None: user_string = self._create_user_string(user) if self.test_run: msg = format_html( @@ -226,7 +227,9 @@ def _add_user_inactive_warning(self, user: UserProfile): self.importer_log.add_warning(msg, category=ImporterLogEntry.Category.INACTIVE) - def _add_user_name_collision_warning(self, user_data: UserData, users_with_same_names: Iterable[UserProfile]): + def _add_user_name_collision_warning( + self, user_data: UserData, users_with_same_names: Iterable[UserProfile] + ) -> None: msg = escape(_("A user in the import file has the same first and last name as an existing user:")) for user in users_with_same_names: msg += format_html("
- {} ({})", self._create_user_string(user), _("existing")) @@ -235,7 +238,7 @@ def _add_user_name_collision_warning(self, user_data: UserData, users_with_same_ self.importer_log.add_warning(msg, category=ImporterLogEntry.Category.DUPL) @staticmethod - def _create_user_string(user: UserProfile | UserData): + def _create_user_string(user: UserProfile | UserData) -> SafeString: if isinstance(user, UserProfile): return format_html( "{} {} {}, {} [{}]", @@ -258,7 +261,7 @@ def __init__(self, *args, **kwargs) -> None: # Only give one error per unique user_data. self.already_checked: set[UserData] = set() - def check_userdata(self, user_data: UserData, _location: ExcelFileLocation): + def check_userdata(self, user_data: UserData, _location: ExcelFileLocation) -> None: if user_data.email == "": # Should trigger another checker. We cannot meaningfully give an error message. return @@ -285,7 +288,7 @@ def __init__(self, *args, **kwargs) -> None: self.tracker = FirstLocationAndCountTracker() - def check_userdata(self, user_data: UserData, location: ExcelFileLocation): + def check_userdata(self, user_data: UserData, location: ExcelFileLocation) -> None: if user_data not in self.first_location_by_user_data: self.first_location_by_user_data[user_data] = location return @@ -307,10 +310,10 @@ def finalize(self) -> None: class UserDataAdapter(RowCheckerMixin): """Adapter to use Checkers for UserData with UserParsedRow""" - def __init__(self, user_data_checker): + def __init__(self, user_data_checker) -> None: self.user_data_checker = user_data_checker - def check_row(self, row: UserParsedRow): + def check_row(self, row: UserParsedRow) -> None: self.user_data_checker.check_userdata(row.user_data, row.location) def finalize(self) -> None: @@ -390,7 +393,7 @@ def get_user_profile_objects(users: Iterable[UserData]) -> tuple[list[UserProfil def update_existing_and_create_new_user_profiles( existing_user_profiles: Iterable[UserProfile], new_user_profiles: Iterable[UserProfile], -): +) -> None: for user_profile in existing_user_profiles: user_profile.save() diff --git a/evap/staff/staff_mode.py b/evap/staff/staff_mode.py index 797c207205..ea82e2e741 100644 --- a/evap/staff/staff_mode.py +++ b/evap/staff/staff_mode.py @@ -1,11 +1,17 @@ import time +from collections.abc import Callable from django.conf import settings from django.contrib import messages +from django.http import HttpRequest, HttpResponseBase from django.utils.translation import gettext as _ +from evap.evaluation.models import UserProfile -def staff_mode_middleware(get_response): + +def staff_mode_middleware( + get_response: Callable[[HttpRequest], HttpResponseBase], +) -> Callable[[HttpRequest], HttpResponseBase]: """ Middleware handling the staff mode. @@ -13,7 +19,7 @@ def staff_mode_middleware(get_response): Otherwise, the last request time will be updated. """ - def middleware(request): + def middleware(request: HttpRequest) -> HttpResponseBase: if is_in_staff_mode(request): current_time = time.time() if current_time <= request.session.get("staff_mode_start_time", 0) + settings.STAFF_MODE_TIMEOUT: @@ -31,6 +37,7 @@ def middleware(request): messages.info(request, _("Your staff mode timed out.")) if is_in_staff_mode(request): + assert isinstance(request.user, UserProfile) assert request.user.has_staff_permission request.user.is_participant = False request.user.is_student = False @@ -41,19 +48,20 @@ def middleware(request): request.user.is_responsible_or_contributor_or_delegate = False else: request.user.is_staff = False - request.user.is_manager = False - request.user.is_reviewer = False + request.user.is_manager = False # type: ignore[union-attr] + request.user.is_reviewer = False # type: ignore[union-attr] return get_response(request) return middleware -def is_in_staff_mode(request): +def is_in_staff_mode(request: HttpRequest) -> bool: return "staff_mode_start_time" in request.session -def update_staff_mode(request): +def update_staff_mode(request: HttpRequest) -> None: + assert isinstance(request.user, UserProfile) if not request.user.has_staff_permission: exit_staff_mode(request) return @@ -62,11 +70,11 @@ def update_staff_mode(request): request.session.modified = True -def enter_staff_mode(request): +def enter_staff_mode(request: HttpRequest) -> None: update_staff_mode(request) -def exit_staff_mode(request): +def exit_staff_mode(request: HttpRequest) -> None: if is_in_staff_mode(request): del request.session["staff_mode_start_time"] request.session.modified = True diff --git a/evap/staff/tools.py b/evap/staff/tools.py index 83ed4fa765..63a8c97ed4 100644 --- a/evap/staff/tools.py +++ b/evap/staff/tools.py @@ -10,13 +10,14 @@ from django.core.exceptions import SuspiciousOperation from django.db import transaction from django.db.models import Count, Max, Model +from django.http import HttpRequest from django.urls import reverse from django.utils.html import escape, format_html, format_html_join from django.utils.safestring import SafeString +from django.utils.translation import gettext, ngettext from django.utils.translation import gettext_lazy as _ -from django.utils.translation import ngettext -from evap.evaluation.models import Contribution, Course, Evaluation, TextAnswer, UserProfile +from evap.evaluation.models import Contribution, Course, Evaluation, Semester, TextAnswer, UserProfile from evap.evaluation.models_logging import LogEntry from evap.evaluation.tools import StrOrPromise, clean_email, is_external_email from evap.grades.models import GradeDocument @@ -34,11 +35,11 @@ class ImportType(Enum): USER_BULK_UPDATE = "user_bulk_update" -def generate_import_path(user_id, import_type) -> Path: +def generate_import_path(user_id: int, import_type: ImportType) -> Path: return settings.MEDIA_ROOT / "temp_import_files" / f"{user_id}.{import_type.value}.xls" -def save_import_file(excel_file, user_id, import_type): +def save_import_file(excel_file, user_id: int, import_type: ImportType) -> None: path = generate_import_path(user_id, import_type) path.parent.mkdir(parents=True, exist_ok=True) with open(path, "wb") as file: @@ -47,17 +48,17 @@ def save_import_file(excel_file, user_id, import_type): excel_file.seek(0) -def delete_import_file(user_id, import_type): +def delete_import_file(user_id: int, import_type: ImportType) -> None: path = generate_import_path(user_id, import_type) path.unlink(missing_ok=True) -def import_file_exists(user_id, import_type): +def import_file_exists(user_id: int, import_type: ImportType) -> bool: path = generate_import_path(user_id, import_type) return path.is_file() -def get_import_file_content_or_raise(user_id, import_type): +def get_import_file_content_or_raise(user_id: int, import_type: ImportType) -> bytes: path = generate_import_path(user_id, import_type) if not path.is_file(): raise SuspiciousOperation("No test run performed previously.") @@ -85,11 +86,11 @@ def conditional_escape(s: str) -> SafeString: return escape(s) -def find_matching_internal_user_for_email(request, email): +def find_matching_internal_user_for_email(request: HttpRequest, email: str) -> UserProfile | None: # for internal users only the part before the @ must be the same to match a user to an email matching_users = [ user - for user in UserProfile.objects.filter(email__startswith=email.split("@")[0] + "@").order_by("id") + for user in UserProfile.objects.filter(email__startswith=email.split("@", maxsplit=1)[0] + "@").order_by("id") if not user.is_external ] @@ -102,7 +103,7 @@ def find_matching_internal_user_for_email(request, email): return matching_users[0] -def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 +def bulk_update_users(request: HttpRequest, user_file_content: bytes, test_run: bool) -> bool: # noqa: PLR0912 # pylint: disable=too-many-locals # user_file must have one user per line in the format "{username},{email}" imported_emails = {clean_email(line.decode().split(",")[1]) for line in user_file_content.splitlines()} @@ -121,7 +122,7 @@ def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 messages.error( request, format_html( - _("Multiple users match the email {}:{}"), + gettext("Multiple users match the email {}:{}"), imported_email, create_user_list_html_string_for_message(e.args[0]), ), @@ -161,7 +162,7 @@ def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 messages.info( request, format_html( - _("Users to be updated are:{}"), + gettext("Users to be updated are:{}"), format_html_join( "", "
{} {} ({} > {})", @@ -172,13 +173,15 @@ def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 if deletable_users: messages.info( request, - format_html(_("Users to be deleted are:{}"), create_user_list_html_string_for_message(deletable_users)), + format_html( + gettext("Users to be deleted are:{}"), create_user_list_html_string_for_message(deletable_users) + ), ) if users_to_mark_inactive: messages.info( request, format_html( - _("Users to be marked inactive are:{}"), + gettext("Users to be marked inactive are:{}"), create_user_list_html_string_for_message(users_to_mark_inactive), ), ) @@ -186,12 +189,13 @@ def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 messages.info( request, format_html( - _("Users to be created are:{}"), + gettext("Users to be created are:{}"), format_html_join("", "
{}", ((email,) for email in emails_of_users_to_be_created)), ), ) with transaction.atomic(): + message: StrOrPromise for user in deletable_users + users_to_mark_inactive: for message in remove_user_from_represented_and_ccing_users( user, deletable_users + users_to_mark_inactive, test_run @@ -222,11 +226,11 @@ def bulk_update_users(request, user_file_content, test_run): # noqa: PLR0912 @transaction.atomic def merge_users( # noqa: PLR0915 # This is much stuff to do. However, splitting it up into subtasks doesn't make much sense. - main_user, other_user, preview=False + main_user: UserProfile, other_user: UserProfile, preview: bool = False ): """Merges other_user into main_user""" - merged_user = {} + merged_user: dict[str, Any] = {} merged_user["is_active"] = main_user.is_active or other_user.is_active merged_user["title"] = main_user.title or other_user.title or "" merged_user["first_name_chosen"] = main_user.first_name_chosen or other_user.first_name_chosen or "" @@ -334,7 +338,7 @@ def merge_users( # noqa: PLR0915 # This is much stuff to do. However, splittin return merged_user, errors, warnings -def find_unreviewed_evaluations(semester, excluded): +def find_unreviewed_evaluations(semester: Semester, excluded: Iterable[int]) -> list[Evaluation]: # as evaluations are open for an offset of hours after vote_end_datetime, the evaluations ending yesterday are also excluded during offset exclude_date = date.today() if datetime.now().hour < settings.EVALUATION_END_OFFSET_HOURS: @@ -353,11 +357,13 @@ def find_unreviewed_evaluations(semester, excluded): ) .annotate(num_unreviewed_textanswers=Count("contributions__textanswer_set")) ), - key=lambda e: (-e.grading_process_is_finished, e.vote_end_date, -e.num_unreviewed_textanswers), + key=lambda e: (-e.grading_process_is_finished, e.vote_end_date, -e.num_unreviewed_textanswers), # type: ignore[attr-defined] ) -def remove_user_from_represented_and_ccing_users(user, ignored_users=None, test_run=False): +def remove_user_from_represented_and_ccing_users( + user: UserProfile, ignored_users: Iterable[UserProfile] | None = None, test_run: bool = False +) -> list[str]: remove_messages = [] ignored_users = ignored_users or [] for represented_user in user.represented_users.exclude(id__in=[user.id for user in ignored_users]): @@ -381,7 +387,7 @@ def remove_user_from_represented_and_ccing_users(user, ignored_users=None, test_ return remove_messages -def remove_participations_if_inactive(user: UserProfile, test_run=False) -> list[StrOrPromise]: +def remove_participations_if_inactive(user: UserProfile, test_run: bool = False) -> list[StrOrPromise]: if user.is_active and not user.can_be_marked_inactive_by_manager: return [] last_participation = user.evaluations_participating_in.aggregate(Max("vote_end_date"))["vote_end_date__max"] @@ -410,7 +416,7 @@ def remove_participations_if_inactive(user: UserProfile, test_run=False) -> list ] -def user_edit_link(user_id): +def user_edit_link(user_id: int) -> SafeString: return format_html( ' {}', reverse("staff:user_edit", kwargs={"user_id": user_id}), @@ -420,7 +426,7 @@ def user_edit_link(user_id): def update_or_create_with_changes[M: Model]( model: type[M], - defaults=None, + defaults: dict | None = None, **kwargs, ) -> tuple[M, bool, dict[str, tuple[Any, Any]]]: """Do update_or_create and track changed values.""" diff --git a/evap/staff/views.py b/evap/staff/views.py index d5d7b675b4..f436086126 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -7,6 +7,7 @@ from datetime import date, datetime from enum import Enum from typing import Any, Final, Literal, assert_never, cast +from uuid import UUID import openpyxl from django.conf import settings @@ -24,6 +25,7 @@ OuterRef, Prefetch, Q, + QuerySet, Sum, When, ) @@ -137,13 +139,13 @@ from evap.student.forms import QuestionnaireVotingForm from evap.student.models import TextAnswerWarning from evap.student.views import render_vote_page -from evap.tools import unordered_groupby +from evap.tools import assert_not_none, unordered_groupby logger = logging.getLogger(__name__) @manager_required -def index(request): +def index(request: HttpRequest) -> HttpResponse: template_data = { "semesters": Semester.objects.all(), "templates": EmailTemplate.objects.all().order_by("id"), @@ -153,7 +155,7 @@ def index(request): return render(request, "staff_index.html", template_data) -def annotate_evaluations_with_grade_document_counts(evaluations): +def annotate_evaluations_with_grade_document_counts(evaluations: QuerySet[Evaluation]) -> QuerySet[Evaluation]: return evaluations.annotate( midterm_grade_documents_count=Count( "course__grade_documents", @@ -168,7 +170,7 @@ def annotate_evaluations_with_grade_document_counts(evaluations): ) -def get_evaluations_with_prefetched_data(semester): +def get_evaluations_with_prefetched_data(semester) -> QuerySet[Evaluation]: evaluations = ( semester.evaluations.select_related("course__type") .prefetch_related( @@ -200,14 +202,13 @@ def get_evaluations_with_prefetched_data(semester): @reviewer_required -def semester_view(request, semester_id) -> HttpResponse: +def semester_view(request, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) if semester.results_are_archived and not request.user.is_manager: raise PermissionDenied rewards_active = is_semester_activated(semester) - evaluations = get_evaluations_with_prefetched_data(semester) - evaluations = sorted(evaluations, key=lambda cr: cr.full_name) + evaluations = sorted(get_evaluations_with_prefetched_data(semester), key=lambda cr: cr.full_name) courses = Course.objects.filter(semester=semester).prefetch_related( "type", "programs", "responsibles", "evaluations" ) @@ -274,22 +275,22 @@ class EvaluationOperation: confirmation_message: StrOrPromise | None = None @staticmethod - def applicable_to(evaluation): + def applicable_to(evaluation: Evaluation) -> bool: raise NotImplementedError @staticmethod - def warning_for_inapplicables(amount): + def warning_for_inapplicables(amount: int) -> str: raise NotImplementedError @staticmethod def apply( - request, - evaluations, - email_template=None, - email_template_contributor=None, - email_template_participant=None, - delete_previous_answers=None, - ): + request: HttpRequest, + evaluations: Collection[Evaluation], + email_template: EmailTemplate | None = None, + email_template_contributor: EmailTemplate | None = None, + email_template_participant: EmailTemplate | None = None, + delete_previous_answers: bool | None = None, + ) -> None: raise NotImplementedError @@ -297,11 +298,11 @@ class ResetToNewOperation(EvaluationOperation): confirmation_message = gettext_lazy("Do you want to reset the following evaluations to preparation?") @staticmethod - def applicable_to(evaluation: Evaluation): + def applicable_to(evaluation: Evaluation) -> bool: return evaluation.can_reset_to_new @staticmethod - def warning_for_inapplicables(amount: int): + def warning_for_inapplicables(amount: int) -> str: return ngettext( "{} evaluation cannot be reset, because it is already in preparation or published. It was removed from the selection.", "{} evaluations cannot be reset, because they were already in preparation or published. They were removed from the selection.", @@ -310,13 +311,13 @@ def warning_for_inapplicables(amount: int): @staticmethod def apply( - request, - evaluations, - email_template=None, - email_template_contributor=None, - email_template_participant=None, - delete_previous_answers=None, - ): + request: HttpRequest, + evaluations: Collection[Evaluation], + email_template: EmailTemplate | None = None, + email_template_contributor: EmailTemplate | None = None, + email_template_participant: EmailTemplate | None = None, + delete_previous_answers: bool | None = None, + ) -> None: assert email_template_contributor is None assert email_template_participant is None @@ -338,11 +339,11 @@ class ReadyForEditorsOperation(EvaluationOperation): confirmation_message = gettext_lazy("Do you want to send the following evaluations to editor review?") @staticmethod - def applicable_to(evaluation): + def applicable_to(evaluation: Evaluation) -> bool: return evaluation.state in [Evaluation.State.NEW, Evaluation.State.EDITOR_APPROVED] @staticmethod - def warning_for_inapplicables(amount): + def warning_for_inapplicables(amount: int) -> str: return ngettext( "{} evaluation cannot be reverted, because it was already approved. It was removed from the selection.", "{} evaluations cannot be reverted, because they were already approved. They were removed from the selection.", @@ -351,12 +352,12 @@ def warning_for_inapplicables(amount): @staticmethod def apply( - request, - evaluations, - email_template=None, - email_template_contributor=None, - email_template_participant=None, - delete_previous_answers=None, + request: HttpRequest, + evaluations: Collection[Evaluation], + email_template: EmailTemplate | None = None, + email_template_contributor: EmailTemplate | None = None, + email_template_participant: EmailTemplate | None = None, + delete_previous_answers: bool | None = None, ): assert email_template_contributor is None assert email_template_participant is None @@ -402,11 +403,11 @@ class BeginEvaluationOperation(EvaluationOperation): confirmation_message = gettext_lazy("Do you want to immediately start the following evaluations?") @staticmethod - def applicable_to(evaluation): + def applicable_to(evaluation: Evaluation) -> bool: return evaluation.state == Evaluation.State.APPROVED and evaluation.vote_end_date >= date.today() @staticmethod - def warning_for_inapplicables(amount): + def warning_for_inapplicables(amount: int) -> str: return ngettext( "{} evaluation cannot be started, because it was not approved, was already evaluated or its evaluation end date lies in the past. It was removed from the selection.", "{} evaluations cannot be started, because they were not approved, were already evaluated or their evaluation end dates lie in the past. They were removed from the selection.", @@ -415,12 +416,12 @@ def warning_for_inapplicables(amount): @staticmethod def apply( - request, - evaluations, - email_template=None, - email_template_contributor=None, - email_template_participant=None, - delete_previous_answers=None, + request: HttpRequest, + evaluations: Collection[Evaluation], + email_template: EmailTemplate | None = None, + email_template_contributor: EmailTemplate | None = None, + email_template_participant: EmailTemplate | None = None, + delete_previous_answers: bool | None = None, ): assert email_template_contributor is None assert email_template_participant is None @@ -446,11 +447,11 @@ class UnpublishOperation(EvaluationOperation): confirmation_message = gettext_lazy("Do you want to unpublish the following evaluations?") @staticmethod - def applicable_to(evaluation): + def applicable_to(evaluation: Evaluation) -> bool: return evaluation.state == Evaluation.State.PUBLISHED @staticmethod - def warning_for_inapplicables(amount): + def warning_for_inapplicables(amount: int) -> str: return ngettext( "{} evaluation cannot be unpublished, because it's results have not been published. It was removed from the selection.", "{} evaluations cannot be unpublished because their results have not been published. They were removed from the selection.", @@ -459,12 +460,12 @@ def warning_for_inapplicables(amount): @staticmethod def apply( - request, - evaluations, - email_template=None, - email_template_contributor=None, - email_template_participant=None, - delete_previous_answers=None, + request: HttpRequest, + evaluations: Collection[Evaluation], + email_template: EmailTemplate | None = None, + email_template_contributor: EmailTemplate | None = None, + email_template_participant: EmailTemplate | None = None, + delete_previous_answers: bool | None = None, ): assert email_template_contributor is None assert email_template_participant is None @@ -487,11 +488,11 @@ class PublishOperation(EvaluationOperation): confirmation_message = gettext_lazy("Do you want to publish the following evaluations?") @staticmethod - def applicable_to(evaluation): + def applicable_to(evaluation: Evaluation) -> bool: return evaluation.state == Evaluation.State.REVIEWED @staticmethod - def warning_for_inapplicables(amount): + def warning_for_inapplicables(amount: int) -> str: return ngettext( "{} evaluation cannot be published, because it's not finished or not all of its text answers have been reviewed. It was removed from the selection.", "{} evaluations cannot be published, because they are not finished or not all of their text answers have been reviewed. They were removed from the selection.", @@ -500,12 +501,12 @@ def warning_for_inapplicables(amount): @staticmethod def apply( - request, - evaluations, - email_template=None, - email_template_contributor=None, - email_template_participant=None, - delete_previous_answers=None, + request: HttpRequest, + evaluations: Collection[Evaluation], + email_template: EmailTemplate | None = None, + email_template_contributor: EmailTemplate | None = None, + email_template_participant: EmailTemplate | None = None, + delete_previous_answers: bool | None = None, ): assert email_template is None assert delete_previous_answers is None @@ -548,7 +549,7 @@ def target_state_and_operation_from_str(target_state_str: str) -> tuple[Evaluati @manager_required -def evaluation_operation(request, semester_id): +def evaluation_operation(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) if semester.participations_are_archived: raise PermissionDenied @@ -664,7 +665,7 @@ def get_success_url(self) -> str: @require_POST @manager_required @transaction.atomic -def semester_make_active(request): +def semester_make_active(request: HttpRequest) -> HttpResponse: semester = get_object_from_dict_pk_entry_or_logged_40x(Semester, request.POST, "semester_id") Semester.objects.update(is_active=None) @@ -676,7 +677,7 @@ def semester_make_active(request): @require_POST @manager_required -def semester_delete(request): +def semester_delete(request: HttpRequest) -> HttpResponse: semester = get_object_from_dict_pk_entry_or_logged_40x(Semester, request.POST, "semester_id") if not semester.can_be_deleted_by_manager: @@ -695,7 +696,7 @@ def semester_delete(request): @manager_required -def semester_import(request, semester_id): +def semester_import(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) if semester.participations_are_archived: raise PermissionDenied @@ -704,6 +705,7 @@ def semester_import(request, semester_id): import_type = ImportType.SEMESTER importer_log = None + user_id = assert_not_none(request.user.id) if request.method == "POST": operation = request.POST.get("operation") @@ -711,7 +713,7 @@ def semester_import(request, semester_id): raise SuspiciousOperation("Invalid POST operation") if operation == "test": - delete_import_file(request.user.id, import_type) # remove old files if still exist + delete_import_file(user_id, import_type) # remove old files if still exist excel_form.fields["excel_file"].required = True if excel_form.is_valid(): excel_file = excel_form.cleaned_data["excel_file"] @@ -720,10 +722,10 @@ def semester_import(request, semester_id): file_content, semester, vote_start_datetime=None, vote_end_date=None, test_run=True ) if not importer_log.has_errors(): - save_import_file(excel_file, request.user.id, import_type) + save_import_file(excel_file, user_id, import_type) elif operation == "import": - file_content = get_import_file_content_or_raise(request.user.id, import_type) + file_content = get_import_file_content_or_raise(user_id, import_type) excel_form.fields["vote_start_datetime"].required = True excel_form.fields["vote_end_date"].required = True if excel_form.is_valid(): @@ -733,10 +735,10 @@ def semester_import(request, semester_id): file_content, semester, vote_start_datetime, vote_end_date, test_run=False ) importer_log.forward_messages_to_django(request) - delete_import_file(request.user.id, import_type) + delete_import_file(user_id, import_type) return redirect("staff:semester_view", semester_id) - test_passed = import_file_exists(request.user.id, import_type) + test_passed = import_file_exists(user_id, import_type) # casting warnings to a normal dict is necessary for the template to iterate over it. return render( request, @@ -751,7 +753,7 @@ def semester_import(request, semester_id): @manager_required -def semester_export(request, semester_id): +def semester_export(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) ExportSheetFormset = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) @@ -774,7 +776,7 @@ def semester_export(request, semester_id): @manager_required -def semester_raw_export(_request, semester_id): +def semester_raw_export(_request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) filename = f"Evaluation-{semester.name}-{get_language()}_raw.csv" @@ -817,7 +819,7 @@ def semester_raw_export(_request, semester_id): @manager_required -def semester_participation_export(_request, semester_id): +def semester_participation_export(_request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) participants = ( UserProfile.objects.filter(evaluations_participating_in__course__semester=semester).distinct().order_by("email") @@ -867,7 +869,7 @@ def semester_participation_export(_request, semester_id): @manager_required -def vote_timestamps_export(_request, semester_id): +def vote_timestamps_export(_request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) timestamps = VoteTimestamp.objects.filter(evaluation__course__semester=semester).prefetch_related( "evaluation__course__programs" @@ -902,7 +904,7 @@ def vote_timestamps_export(_request, semester_id): @manager_required -def semester_questionnaire_assign(request, semester_id): +def semester_questionnaire_assign(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) if semester.participations_are_archived: raise PermissionDenied @@ -978,8 +980,8 @@ def semester_preparation_reminder(request: HttpRequest, semester_id: int) -> Htt if request.method == "POST": template = EmailTemplate.objects.get(name=EmailTemplate.EDITOR_REVIEW_REMINDER) - for responsible, evaluations, __ in responsible_list: - body_params = {"user": responsible, "evaluations": evaluations} + for responsible, responsible_evaluations, __ in responsible_list: + body_params = {"user": responsible, "evaluations": responsible_evaluations} template.send_to_user(responsible, subject_params={}, body_params=body_params, use_cc=True, request=request) messages.success(request, _("Successfully sent reminders to everyone.")) return HttpResponse() @@ -991,7 +993,7 @@ def semester_preparation_reminder(request: HttpRequest, semester_id: int) -> Htt @manager_required -def semester_grade_reminder(request, semester_id: int) -> HttpResponse: +def semester_grade_reminder(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) courses = sorted( @@ -1011,7 +1013,7 @@ def semester_grade_reminder(request, semester_id: int) -> HttpResponse: @manager_required -def send_reminder(request, semester_id, responsible_id): +def send_reminder(request: HttpRequest, semester_id: int, responsible_id: int) -> HttpResponse: responsible = get_object_or_404(UserProfile, id=responsible_id) semester = get_object_or_404(Semester, id=semester_id) @@ -1031,7 +1033,7 @@ def send_reminder(request, semester_id, responsible_id): @require_POST @manager_required -def semester_archive_participations(request): +def semester_archive_participations(request: HttpRequest) -> HttpResponse: semester = get_object_from_dict_pk_entry_or_logged_40x(Semester, request.POST, "semester_id") if not semester.participations_can_be_archived: @@ -1042,7 +1044,7 @@ def semester_archive_participations(request): @require_POST @manager_required -def semester_delete_grade_documents(request): +def semester_delete_grade_documents(request: HttpRequest) -> HttpResponse: semester = get_object_from_dict_pk_entry_or_logged_40x(Semester, request.POST, "semester_id") if not semester.grade_documents_can_be_deleted: @@ -1053,7 +1055,7 @@ def semester_delete_grade_documents(request): @require_POST @manager_required -def semester_archive_results(request): +def semester_archive_results(request: HttpRequest) -> HttpResponse: semester_id = request.POST.get("semester_id") semester = get_object_or_404(Semester, id=semester_id) @@ -1064,7 +1066,7 @@ def semester_archive_results(request): @manager_required -def course_create(request, semester_id): +def course_create(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) if semester.participations_are_archived: raise PermissionDenied @@ -1091,7 +1093,7 @@ def course_create(request, semester_id): @manager_required -def course_copy(request, course_id): +def course_copy(request: HttpRequest, course_id: int) -> HttpResponse: course = get_object_or_404(Course, id=course_id) course_form = CourseCopyForm(request.POST or None, instance=course) @@ -1190,7 +1192,7 @@ def get_success_url(self) -> str: @require_POST @manager_required -def course_delete(request): +def course_delete(request: HttpRequest) -> HttpResponse: course = get_object_from_dict_pk_entry_or_logged_40x(Course, request.POST, "course_id") if not course.can_be_deleted_by_manager: raise SuspiciousOperation("Deleting course not allowed") @@ -1198,7 +1200,7 @@ def course_delete(request): return HttpResponse() # 200 OK -def evaluation_create_impl(request, semester: Semester, course: Course | None): +def evaluation_create_impl(request: HttpRequest, semester: Semester, course: Course | None) -> HttpResponse: if course is not None: assert course.semester == semester if semester.participations_are_archived: @@ -1239,19 +1241,19 @@ def evaluation_create_impl(request, semester: Semester, course: Course | None): @manager_required -def evaluation_create_for_semester(request, semester_id): +def evaluation_create_for_semester(request: HttpRequest, semester_id: int) -> HttpResponse: semester = get_object_or_404(Semester, id=semester_id) return evaluation_create_impl(request, semester, None) @manager_required -def evaluation_create_for_course(request, course_id): +def evaluation_create_for_course(request: HttpRequest, course_id: int) -> HttpResponse: course = get_object_or_404(Course, id=course_id) return evaluation_create_impl(request, course.semester, course) @manager_required -def evaluation_copy(request, evaluation_id): +def evaluation_copy(request: HttpRequest, evaluation_id: int) -> HttpResponse: evaluation = get_object_or_404(Evaluation, id=evaluation_id) form = EvaluationCopyForm(request.POST or None, evaluation) @@ -1286,7 +1288,7 @@ def evaluation_copy(request, evaluation_id): @manager_required -def evaluation_edit(request, evaluation_id): +def evaluation_edit(request: HttpRequest, evaluation_id: int) -> HttpResponse: evaluation = get_object_or_404(Evaluation, id=evaluation_id) if request.method == "POST" and not evaluation.can_be_edited_by_manager: @@ -1296,7 +1298,7 @@ def evaluation_edit(request, evaluation_id): @manager_required -def helper_evaluation_edit(request, evaluation): +def helper_evaluation_edit(request: HttpRequest, evaluation: Evaluation) -> HttpResponse: editable = evaluation.can_be_edited_by_manager InlineContributionFormset = inlineformset_factory( Evaluation, Contribution, formset=ContributionFormset, form=ContributionForm, extra=1 if editable else 0 @@ -1361,7 +1363,7 @@ def notify_reward_points(grantings, **_kwargs): contributor_questionnaire_pairs = [ (answer.contribution.contributor, answer.assignment.questionnaire) for answer_cls in [TextAnswer, RatingAnswerCounter] - for answer in answer_cls.objects.filter(contribution__evaluation=evaluation).select_related( + for answer in answer_cls.objects.filter(contribution__evaluation=evaluation).select_related( # type: ignore[attr-defined] "assignment__questionnaire", "contribution__contributor" ) ] @@ -1390,7 +1392,7 @@ def notify_reward_points(grantings, **_kwargs): @require_POST @manager_required @transaction.atomic -def evaluation_delete(request): +def evaluation_delete(request: HttpRequest) -> HttpResponse: evaluation = get_object_from_dict_pk_entry_or_logged_40x(Evaluation, request.POST, "evaluation_id") if not evaluation.can_be_deleted_by_manager: @@ -1419,7 +1421,7 @@ def notify_reward_points(grantings, **_kwargs): @manager_required -def evaluation_email(request, evaluation_id): +def evaluation_email(request: HttpRequest, evaluation_id: int) -> HttpResponse: evaluation = get_object_or_404(Evaluation, id=evaluation_id) export = "export" in request.POST form = EvaluationEmailForm(request.POST or None, evaluation=evaluation, export=export) @@ -1506,7 +1508,7 @@ def import_or_copy_participants( @manager_required @transaction.atomic -def evaluation_person_management(request, evaluation_id: int): +def evaluation_person_management(request: HttpRequest, evaluation_id: int) -> HttpResponse: # This view indeed handles 4 tasks. However, they are tightly coupled, splitting them up # would lead to more code duplication. Thus, we decided to leave it as is for now # pylint: disable=too-many-locals @@ -1522,6 +1524,8 @@ def evaluation_person_management(request, evaluation_id: int): importer_log = None + user_id = assert_not_none(request.user.id) + if request.method == "POST": operation = request.POST.get("operation") if operation not in ( @@ -1544,7 +1548,7 @@ def evaluation_person_management(request, evaluation_id: int): copy_form = participant_copy_form if "participants" in operation else contributor_copy_form if import_action == ImportAction.TEST: - delete_import_file(request.user.id, import_type) # remove old files if still exist + delete_import_file(user_id, import_type) # remove old files if still exist excel_form.fields["excel_file"].required = True if excel_form.is_valid(): excel_file = excel_form.cleaned_data["excel_file"] @@ -1553,7 +1557,7 @@ def evaluation_person_management(request, evaluation_id: int): import_type, evaluation, test_run=True, file_content=file_content ) if not importer_log.has_errors(): - save_import_file(excel_file, request.user.id, import_type) + save_import_file(excel_file, user_id, import_type) else: successfully_processed = import_or_copy_participants( request, "-replace-" in operation, import_action, import_type, evaluation, copy_form @@ -1561,8 +1565,8 @@ def evaluation_person_management(request, evaluation_id: int): if successfully_processed: return redirect("staff:semester_view", evaluation.course.semester.pk) - participant_test_passed = import_file_exists(request.user.id, ImportType.PARTICIPANT) - contributor_test_passed = import_file_exists(request.user.id, ImportType.CONTRIBUTOR) + participant_test_passed = import_file_exists(user_id, ImportType.PARTICIPANT) + contributor_test_passed = import_file_exists(user_id, ImportType.CONTRIBUTOR) return render( request, "staff_evaluation_person_management.html", @@ -1581,7 +1585,7 @@ def evaluation_person_management(request, evaluation_id: int): @manager_required -def evaluation_login_key_export(_request, evaluation_id): +def evaluation_login_key_export(_request: HttpRequest, evaluation_id: int) -> HttpResponse: evaluation = get_object_or_404(Evaluation, id=evaluation_id) filename = f"Login_keys-{evaluation.full_name}-{evaluation.course.semester.short_name}.csv" @@ -1709,7 +1713,7 @@ def semester_flagged_textanswers(request: HttpRequest, semester_id: int) -> Http @reviewer_required -def evaluation_textanswers_skip(request): +def evaluation_textanswers_skip(request: HttpRequest) -> HttpResponse: evaluation = get_object_from_dict_pk_entry_or_logged_40x(Evaluation, request.POST, "evaluation_id") visited = request.session.get("review-skipped", set()) visited.add(evaluation.pk) @@ -1728,7 +1732,7 @@ def assert_textanswer_review_permissions(evaluation: Evaluation) -> None: @require_POST @reviewer_required -def evaluation_textanswers_update_publish(request): +def evaluation_textanswers_update_publish(request: HttpRequest) -> HttpResponse: answer = get_object_from_dict_pk_entry_or_logged_40x(TextAnswer, request.POST, "answer_id") evaluation = answer.contribution.evaluation action = request.POST.get("action", None) @@ -1763,7 +1767,7 @@ def evaluation_textanswers_update_publish(request): @require_POST @reviewer_required -def evaluation_textanswers_update_flag(request): +def evaluation_textanswers_update_flag(request: HttpRequest) -> HttpResponse: answer = get_object_from_dict_pk_entry_or_logged_40x(TextAnswer, request.POST, "answer_id") assert_textanswer_review_permissions(answer.contribution.evaluation) @@ -1778,7 +1782,7 @@ def evaluation_textanswers_update_flag(request): @manager_required -def evaluation_textanswer_edit(request, textanswer_id): +def evaluation_textanswer_edit(request: HttpRequest, textanswer_id: UUID) -> HttpResponse: textanswer = get_object_or_404(TextAnswer, id=textanswer_id) evaluation = textanswer.contribution.evaluation assert_textanswer_review_permissions(evaluation) @@ -1806,8 +1810,9 @@ def evaluation_textanswer_edit(request, textanswer_id): @reviewer_required -def evaluation_preview(request, evaluation_id): +def evaluation_preview(request: HttpRequest, evaluation_id: int) -> HttpResponse: evaluation = get_object_or_404(Evaluation, id=evaluation_id) + assert isinstance(request.user, UserProfile) if evaluation.course.semester.results_are_archived and not request.user.is_manager: raise PermissionDenied @@ -1815,7 +1820,7 @@ def evaluation_preview(request, evaluation_id): @manager_required -def questionnaire_index(request): +def questionnaire_index(request: HttpRequest) -> HttpResponse: filter_questionnaires = get_parameter_from_url_or_session(request, "filter_questionnaires") prefetch_list = ("questions", "contributions__evaluation") @@ -1845,7 +1850,8 @@ def questionnaire_index(request): @manager_required -def questionnaire_view(request, questionnaire_id): +def questionnaire_view(request: HttpRequest, questionnaire_id: int) -> HttpResponse: + assert isinstance(request.user, UserProfile) language = request.GET.get("language", request.user.language) questionnaire = get_object_or_404(Questionnaire, id=questionnaire_id) @@ -1867,7 +1873,7 @@ def questionnaire_view(request, questionnaire_id): @manager_required -def questionnaire_create(request): +def questionnaire_create(request: HttpRequest) -> HttpResponse: questionnaire = Questionnaire() InlineQuestionAssignmentFormset = inlineformset_factory( Questionnaire, @@ -1898,7 +1904,9 @@ def disable_all_except_named(fields: dict[str, Any], names_of_editable: Collecti field.disabled = True -def make_questionnaire_edit_forms(request, questionnaire, editable): +def make_questionnaire_edit_forms( + request: HttpRequest, questionnaire: Questionnaire, editable: bool +) -> tuple[QuestionnaireForm, Any]: if editable: formset_kwargs = {"extra": 1} else: @@ -1917,7 +1925,7 @@ def make_questionnaire_edit_forms(request, questionnaire, editable): formset=AtLeastOneFormset, form=QuestionAssignmentForm, exclude=("questionnaire",), - **formset_kwargs, + **formset_kwargs, # type: ignore[arg-type] ) form = QuestionnaireForm(request.POST or None, instance=questionnaire) @@ -1934,15 +1942,15 @@ def make_questionnaire_edit_forms(request, questionnaire, editable): # disallow type changed from and to contributor or dropout single_types = [Questionnaire.Type.CONTRIBUTOR, Questionnaire.Type.DROPOUT] if questionnaire.type in single_types: - form.fields["type"].choices = filter(lambda c: c[0] == questionnaire.type, form.fields["type"].choices) + form.fields["type"].choices = filter(lambda c: c[0] == questionnaire.type, form.fields["type"].choices) # type: ignore[attr-defined,arg-type] else: - form.fields["type"].choices = filter(lambda c: c[0] not in single_types, form.fields["type"].choices) + form.fields["type"].choices = filter(lambda c: c[0] not in single_types, form.fields["type"].choices) # type: ignore[attr-defined,arg-type] return form, formset @manager_required -def questionnaire_edit(request, questionnaire_id): +def questionnaire_edit(request: HttpRequest, questionnaire_id: int) -> HttpResponse: questionnaire = get_object_or_404(Questionnaire, id=questionnaire_id) editable = questionnaire.can_be_edited_by_manager @@ -1960,7 +1968,7 @@ def questionnaire_edit(request, questionnaire_id): return render(request, "staff_questionnaire_form.html", template_data) -def get_identical_form_and_formset(questionnaire): +def get_identical_form_and_formset(questionnaire: Questionnaire) -> tuple[QuestionnaireForm, Any]: """ Generates a Questionnaire creation form and formset filled out like the already exisiting Questionnaire specified in questionnaire_id. Used for copying and creating of new versions. @@ -1981,7 +1989,7 @@ def get_identical_form_and_formset(questionnaire): @manager_required -def questionnaire_copy(request, questionnaire_id): +def questionnaire_copy(request: HttpRequest, questionnaire_id: int) -> HttpResponse: copied_questionnaire = get_object_or_404(Questionnaire, id=questionnaire_id) if request.method == "POST": @@ -2011,7 +2019,7 @@ def questionnaire_copy(request, questionnaire_id): @manager_required -def questionnaire_new_version(request, questionnaire_id): +def questionnaire_new_version(request: HttpRequest, questionnaire_id: int) -> HttpResponse: old_questionnaire = get_object_or_404(Questionnaire, id=questionnaire_id) # Check if we can use the old name with the current time stamp. @@ -2065,7 +2073,7 @@ def questionnaire_new_version(request, questionnaire_id): @require_POST @manager_required -def questionnaire_delete(request): +def questionnaire_delete(request: HttpRequest) -> HttpResponse: questionnaire = get_object_from_dict_pk_entry_or_logged_40x(Questionnaire, request.POST, "questionnaire_id") if not questionnaire.can_be_deleted_by_manager: @@ -2076,9 +2084,9 @@ def questionnaire_delete(request): @require_POST @manager_required -def questionnaire_update_indices(request): +def questionnaire_update_indices(request: HttpRequest) -> HttpResponse: try: - order_by_questionnaire = {int(key): int(value) for key, value in request.POST.items()} + order_by_questionnaire = {int(key): int(value) for key, value in request.POST.items() if isinstance(value, str)} except (TypeError, ValueError) as e: raise SuspiciousOperation from e @@ -2095,7 +2103,7 @@ def questionnaire_update_indices(request): @require_POST @manager_required -def questionnaire_visibility(request): +def questionnaire_visibility(request: HttpRequest) -> HttpResponse: questionnaire = get_object_from_dict_pk_entry_or_logged_40x(Questionnaire, request.POST, "questionnaire_id") try: visibility = int(request.POST["visibility"]) @@ -2112,7 +2120,7 @@ def questionnaire_visibility(request): @require_POST @manager_required -def questionnaire_set_locked(request): +def questionnaire_set_locked(request: HttpRequest) -> HttpResponse: questionnaire = get_object_from_dict_pk_entry_or_logged_40x(Questionnaire, request.POST, "questionnaire_id") try: is_locked = bool(int(request.POST["is_locked"])) @@ -2140,7 +2148,7 @@ class ProgramIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): @manager_required -def program_merge_selection(request): +def program_merge_selection(request: HttpRequest) -> HttpResponse: form = ProgramMergeSelectionForm(request.POST or None) if form.is_valid(): @@ -2152,7 +2160,7 @@ def program_merge_selection(request): @manager_required -def program_merge(request, main_id, other_id): +def program_merge(request: HttpRequest, main_id: int, other_id: int) -> HttpResponse: main_instance = get_object_or_404(Program, id=main_id) other_instance = get_object_or_404(Program, id=other_id) assert main_instance != other_instance @@ -2202,7 +2210,7 @@ class CourseTypeIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): @manager_required -def course_type_merge_selection(request): +def course_type_merge_selection(request: HttpRequest) -> HttpResponse: form = CourseTypeMergeSelectionForm(request.POST or None) if form.is_valid(): @@ -2214,7 +2222,7 @@ def course_type_merge_selection(request): @manager_required -def course_type_merge(request, main_type_id, other_type_id): +def course_type_merge(request: HttpRequest, main_type_id: int, other_type_id: int) -> HttpResponse: main_type = get_object_or_404(CourseType, id=main_type_id) other_type = get_object_or_404(CourseType, id=other_type_id) assert main_type != other_type @@ -2251,7 +2259,7 @@ class ExamTypeIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): @manager_required -def text_answer_warnings_index(request): +def text_answer_warnings_index(request: HttpRequest) -> HttpResponse: text_answer_warnings = TextAnswerWarning.objects.all() TextAnswerWarningFormset = modelformset_factory( @@ -2275,7 +2283,7 @@ def text_answer_warnings_index(request): @manager_required -def user_index(request): +def user_index(request: HttpRequest) -> HttpResponse: form = UserEditSelectionForm(request.POST or None) if form.is_valid(): @@ -2286,7 +2294,7 @@ def user_index(request): @manager_required -def user_list(request): +def user_list(request: HttpRequest) -> HttpResponse: filter_users = get_parameter_from_url_or_session(request, "filter_users") users = UserProfile.objects.all() @@ -2294,7 +2302,7 @@ def user_list(request): users = users.exclude(is_active=False) users = ( - users + users # type: ignore[no-redef,misc] # the following six annotations basically add three bools indicating whether each user is part of a group or not. .annotate(manager_group_count=Sum(Case(When(groups__name="Manager", then=1), output_field=IntegerField()))) .annotate(is_manager=ExpressionWrapper(Q(manager_group_count__exact=1), output_field=BooleanField())) @@ -2323,7 +2331,7 @@ def user_list(request): @manager_required -def user_export(request): +def user_export(request: HttpRequest) -> HttpResponse: response = AttachmentResponse("exported_users.csv") writer = csv.writer(response, delimiter=";", lineterminator="\n") header_row = (_("Title"), _("Last name"), _("First name"), _("Email")) @@ -2344,11 +2352,12 @@ class UserCreateView(SuccessMessageMixin, CreateView): @manager_required -def user_import(request): +def user_import(request: HttpRequest) -> HttpResponse: excel_form = UserImportForm(request.POST or None, request.FILES or None) import_type = ImportType.USER importer_log = None + user_id = assert_not_none(request.user.id) if request.method == "POST": operation = request.POST.get("operation") @@ -2356,23 +2365,23 @@ def user_import(request): raise SuspiciousOperation("Invalid POST operation") if operation == "test": - delete_import_file(request.user.id, import_type) # remove old files if still exist + delete_import_file(user_id, import_type) # remove old files if still exist excel_form.fields["excel_file"].required = True if excel_form.is_valid(): excel_file = excel_form.cleaned_data["excel_file"] file_content = excel_file.read() __, importer_log = import_users(file_content, test_run=True) if not importer_log.has_errors(): - save_import_file(excel_file, request.user.id, import_type) + save_import_file(excel_file, user_id, import_type) elif operation == "import": - file_content = get_import_file_content_or_raise(request.user.id, import_type) + file_content = get_import_file_content_or_raise(user_id, import_type) __, importer_log = import_users(file_content, test_run=False) importer_log.forward_messages_to_django(request) - delete_import_file(request.user.id, import_type) + delete_import_file(user_id, import_type) return redirect("staff:user_index") - test_passed = import_file_exists(request.user.id, import_type) + test_passed = import_file_exists(user_id, import_type) # casting warnings to a normal dict is necessary for the template to iterate over it. return render( request, @@ -2386,7 +2395,7 @@ def user_import(request): @manager_required -def user_edit(request, user_id): +def user_edit(request: HttpRequest, user_id: int) -> HttpResponse: user = get_object_or_404(UserProfile, id=user_id) form = UserForm(request.POST or None, request.FILES or None, instance=user) @@ -2433,7 +2442,7 @@ def notify_reward_points(grantings, **_kwargs): @require_POST @manager_required -def user_delete(request): +def user_delete(request: HttpRequest) -> HttpResponse: user = get_object_from_dict_pk_entry_or_logged_40x(UserProfile, request.POST, "user_id") if not user.can_be_deleted_by_manager: @@ -2445,7 +2454,7 @@ def user_delete(request): @require_POST @manager_required -def user_resend_email(request): +def user_resend_email(request: HttpRequest) -> HttpResponse: user = get_object_from_dict_pk_entry_or_logged_40x(UserProfile, request.POST, "user_id") template = EmailTemplate.objects.get(name=EmailTemplate.EVALUATION_STARTED) @@ -2461,18 +2470,19 @@ def user_resend_email(request): @manager_required -def user_bulk_update(request): +def user_bulk_update(request: HttpRequest) -> HttpResponse: form = UserBulkUpdateForm(request.POST or None, request.FILES or None) operation = request.POST.get("operation") test_run = operation == "test" import_type = ImportType.USER_BULK_UPDATE + user_id = assert_not_none(request.user.id) if request.POST: if operation not in ("test", "bulk_update"): raise SuspiciousOperation("Invalid POST operation") if test_run: - delete_import_file(request.user.id, import_type) # remove old files if still exist + delete_import_file(user_id, import_type) # remove old files if still exist form.fields["user_file"].required = True if form.is_valid(): user_file = form.cleaned_data["user_file"] @@ -2489,14 +2499,14 @@ def user_bulk_update(request): ) if success: - save_import_file(user_file, request.user.id, import_type) + save_import_file(user_file, user_id, import_type) else: - file_content = get_import_file_content_or_raise(request.user.id, import_type) + file_content = get_import_file_content_or_raise(user_id, import_type) bulk_update_users(request, file_content, test_run) - delete_import_file(request.user.id, import_type) + delete_import_file(user_id, import_type) return redirect("staff:user_index") - test_passed = import_file_exists(request.user.id, import_type) + test_passed = import_file_exists(user_id, import_type) return render(request, "staff_user_bulk_update.html", {"form": form, "test_passed": test_passed}) @@ -2543,7 +2553,7 @@ def form_valid(self, form: UserMergeSelectionForm) -> HttpResponse: @manager_required -def user_merge(request, main_user_id, other_user_id): +def user_merge(request: HttpRequest, main_user_id: int, other_user_id: int) -> HttpResponse: main_user = get_object_or_404(UserProfile, id=main_user_id) other_user = get_object_or_404(UserProfile, id=other_user_id) @@ -2628,7 +2638,7 @@ def get_context_data(self, **kwargs) -> dict[str, Any]: @manager_required -def faq_section(request, section_id): +def faq_section(request: HttpRequest, section_id: int) -> HttpResponse: section = get_object_or_404(FaqSection, id=section_id) questions = FaqQuestion.objects.filter(section=section) @@ -2655,14 +2665,14 @@ class InfotextsView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): @manager_required -def download_sample_file(_request, filename): +def download_sample_file(_request: HttpRequest, filename: str) -> HttpResponse: email_placeholder = "institution.com" if filename not in ["sample.xlsx", "sample_user.xlsx"]: raise SuspiciousOperation("Invalid file name.") book = openpyxl.load_workbook(filename=settings.STATICFILES_DIRS[0] / filename) - for sheet in book: + for sheet in book: # type: ignore[attr-defined] for row in sheet: for cell in row: if cell.value is not None: @@ -2671,25 +2681,25 @@ def download_sample_file(_request, filename): response = AttachmentResponse( filename, content_type="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" ) - book.save(response) + book.save(response) # type: ignore[arg-type] return response @manager_required -def export_contributor_results_view(request, contributor_id): +def export_contributor_results_view(request: HttpRequest, contributor_id: int) -> HttpResponse: contributor = get_object_or_404(UserProfile, id=contributor_id) return export_contributor_results(contributor) @require_POST @staff_permission_required -def enter_staff_mode(request): +def enter_staff_mode(request: HttpRequest) -> HttpResponse: staff_mode.enter_staff_mode(request) return redirect("evaluation:index") @require_POST @staff_permission_required -def exit_staff_mode(request): +def exit_staff_mode(request: HttpRequest) -> HttpResponse: staff_mode.exit_staff_mode(request) return redirect("evaluation:index") diff --git a/evap/student/forms.py b/evap/student/forms.py index 44b51e9eda..c2061fad85 100644 --- a/evap/student/forms.py +++ b/evap/student/forms.py @@ -1,6 +1,6 @@ from django import forms -from evap.evaluation.models import CHOICES, Question +from evap.evaluation.models import CHOICES, Contribution, Question, Questionnaire from evap.student.tools import answer_field_id @@ -8,28 +8,28 @@ class HeadingField(forms.Field): """Pseudo field used to store and display headings inside a QuestionnaireVotingForm. Does not handle any kind of input.""" - def __init__(self, label): + def __init__(self, label) -> None: super().__init__(label=label, required=False) @classmethod - def from_question(cls, question: Question): + def from_question(cls, question: Question) -> "HeadingField": return cls(label=question.text) class TextAnswerField(forms.CharField): - def __init__(self, *args, related_answer_field_id=None, **kwargs): + def __init__(self, *args, related_answer_field_id: str | None = None, **kwargs) -> None: self.related_answer_field_id = related_answer_field_id kwargs["required"] = False kwargs["widget"] = forms.Textarea(attrs={"related_answer_field_id": self.related_answer_field_id}) super().__init__(*args, **kwargs) @classmethod - def from_question(cls, question: Question): + def from_question(cls, question: Question) -> "TextAnswerField": return cls(label=question.text) class RatingAnswerField(forms.TypedChoiceField): - def __init__(self, widget_choices, *args, allows_textanswer=False, **kwargs): + def __init__(self, widget_choices, *args, allows_textanswer: bool = False, **kwargs) -> None: self.allows_textanswer = allows_textanswer kwargs["coerce"] = int kwargs["widget"] = forms.RadioSelect( @@ -41,7 +41,7 @@ def __init__(self, widget_choices, *args, allows_textanswer=False, **kwargs): super().__init__(*args, **kwargs) @classmethod - def from_question(cls, question: Question): + def from_question(cls, question: Question) -> "RatingAnswerField": return cls( widget_choices=CHOICES[question.type], choices=zip(CHOICES[question.type].values, CHOICES[question.type].names, strict=True), @@ -55,11 +55,12 @@ class QuestionnaireVotingForm(forms.Form): See http://jacobian.org/writing/dynamic-form-generation/""" - def __init__(self, *args, contribution, questionnaire, **kwargs): + def __init__(self, *args, contribution: Contribution, questionnaire: Questionnaire, **kwargs) -> None: super().__init__(*args, **kwargs) self.questionnaire = questionnaire for question in self.questionnaire.questions.all(): + field: TextAnswerField | RatingAnswerField | HeadingField if question.is_text_question: field = TextAnswerField.from_question(question) elif question.is_rating_question: diff --git a/evap/student/templatetags/student_filters.py b/evap/student/templatetags/student_filters.py index 11b2e25814..c689e1c2c6 100644 --- a/evap/student/templatetags/student_filters.py +++ b/evap/student/templatetags/student_filters.py @@ -1,8 +1,12 @@ +from collections.abc import Iterable + from django.template import Library +from evap.student.models import TextAnswerWarning + register = Library() @register.filter -def text_answer_warning_trigger_strings(text_answer_warnings): +def text_answer_warning_trigger_strings(text_answer_warnings: Iterable[TextAnswerWarning]) -> list[list[str]]: return [text_answer_warning.trigger_strings for text_answer_warning in text_answer_warnings] diff --git a/evap/student/tests/test_views.py b/evap/student/tests/test_views.py index a618eb1814..e034c86804 100644 --- a/evap/student/tests/test_views.py +++ b/evap/student/tests/test_views.py @@ -642,6 +642,8 @@ def test_main_language_does_not_use_gettext_lazy(self): class TestDropoutView(WebTest): + evaluation: Evaluation + @classmethod def setUpTestData(cls) -> None: cls.user = baker.make(UserProfile, email="student@institution.example.com") diff --git a/evap/student/views.py b/evap/student/views.py index a6f983535e..dd8cc55772 100644 --- a/evap/student/views.py +++ b/evap/student/views.py @@ -4,6 +4,7 @@ from collections.abc import Iterable from dataclasses import dataclass from fractions import Fraction +from typing import TYPE_CHECKING, cast from django.conf import settings from django.contrib import messages @@ -38,6 +39,10 @@ from evap.student.models import TextAnswerWarning from evap.student.tools import answer_field_id +if TYPE_CHECKING: + from evap.evaluation.models import UserProfile + + SUCCESS_MAGIC_STRING = "vote submitted successfully" @@ -76,14 +81,13 @@ def from_settings() -> "GlobalEvaluationProgress | None": if not settings.GLOBAL_EVALUATION_PROGRESS_REWARDS: return None - if not Semester.active_semester(): + if not (active_semester := Semester.active_semester()): return None language = get_language() evaluations = ( - Semester.active_semester() - .evaluations.exclude(state__lt=Evaluation.State.APPROVED) + active_semester.evaluations.exclude(state__lt=Evaluation.State.APPROVED) .exclude(is_rewarded=False) .exclude(id__in=settings.GLOBAL_EVALUATION_PROGRESS_EXCLUDED_EVALUATION_IDS) .exclude(course__type__id__in=settings.GLOBAL_EVALUATION_PROGRESS_EXCLUDED_COURSE_TYPE_IDS) @@ -345,7 +349,7 @@ def vote(request: HttpRequest, evaluation_id: int, dropout: bool = False) -> Htt if dropout and not evaluation.is_dropout_allowed: raise SuspiciousOperation("Dropping out is not allowed") - if not evaluation.can_be_voted_for_by(request.user): + if not evaluation.can_be_voted_for_by(cast("UserProfile", request.user)): raise PermissionDenied form_groups = get_vote_page_form_groups(request, evaluation, preview=False, dropout=dropout) @@ -377,6 +381,7 @@ def vote(request: HttpRequest, evaluation_id: int, dropout: bool = False) -> Htt contribution=contribution, assignment=assignment, answer=value ) else: + assert isinstance(question.answer_class, RatingAnswerCounter) if value != NO_ANSWER: answer_counter, __ = question.answer_class.objects.get_or_create( contribution=contribution, assignment=assignment, answer=value diff --git a/pyproject.toml b/pyproject.toml index 91de1c1d79..b6d1fe0767 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -181,6 +181,12 @@ exclude_also = [ packages = ["evap", "tools"] plugins = ["mypy_django_plugin.main"] +# list from https://mypy.readthedocs.io/en/stable/existing_code.html#introduce-stricter-options +warn_unused_configs = true +warn_redundant_casts = true +warn_unused_ignores = true +strict_equality = true + [tool.django-stubs] django_settings_module = "evap.settings" diff --git a/tools/check_dist.py b/tools/check_dist.py index 8c9d40b55d..dd21dab815 100755 --- a/tools/check_dist.py +++ b/tools/check_dist.py @@ -3,13 +3,15 @@ import argparse import sys import tomllib +from collections.abc import Iterable from pathlib import Path +from typing import Any from zipfile import ZipFile from pathspec.patterns.gitwildmatch import GitWildMatchPattern -def ensure_all_artifacts_included(pyproject, wheel_paths): +def ensure_all_artifacts_included(pyproject: Any, wheel_paths: Iterable[str]) -> int: try: artifacts = pyproject["tool"]["hatch"]["build"]["artifacts"] except KeyError: @@ -27,7 +29,7 @@ def ensure_all_artifacts_included(pyproject, wheel_paths): return status -def main(argv): +def main(argv: list[str]) -> int: parser = argparse.ArgumentParser("check_dist") parser.add_argument("pyproject", type=Path) parser.add_argument("wheels", type=Path, nargs="*") diff --git a/tools/enrollment_preprocessor.py b/tools/enrollment_preprocessor.py index c810655184..42fb1431ee 100755 --- a/tools/enrollment_preprocessor.py +++ b/tools/enrollment_preprocessor.py @@ -44,7 +44,7 @@ def _clean(self) -> Iterator[str]: field.value = str(field.value).strip() yield field.value - def clean_user(self): + def clean_user(self) -> User: return User(*self._clean()) def update_from(self, user: User) -> bool: @@ -56,7 +56,7 @@ def update_from(self, user: User) -> bool: return changed -def user_from_row(row: tuple[Cell, ...]): +def user_from_row(row: tuple[Cell, ...]) -> list[User]: return [ UserCells(None, *row[:3]).clean_user(), UserCells(*row[7:]).clean_user(),