From ba37efd29148331fe82b050700d2d94a94916ab7 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 29 May 2023 18:32:03 +0200 Subject: [PATCH 01/39] work --- evap/evaluation/auth.py | 83 +++++++++++++---------------------------- evap/grades/urls.py | 1 + evap/grades/views.py | 13 +++++++ 3 files changed, 40 insertions(+), 57 deletions(-) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index 085e8b45af..a2e2c65e69 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -1,5 +1,8 @@ from functools import wraps +from typing import Callable +import inspect +from django.utils.decorators import method_decorator from django.contrib.auth.backends import ModelBackend from django.core.exceptions import PermissionDenied from mozilla_django_oidc.auth import OIDCAuthenticationBackend @@ -45,78 +48,44 @@ def authenticate(self, request, email=None, password=None): # pylint: disable=a return None -def user_passes_test(test_func): - """ - Decorator for views that checks whether a user passes a given test - (raising 403 if not). The test should be a callable that takes the - user object and returns True if the user passes. - """ - - def decorator(view_func): - @wraps(view_func) - def _wrapped_view(request, *args, **kwargs): +def class_or_function_check_decorator(test_func: Callable[[UserProfile], bool]): + def function_decorator(func): + @wraps(func) + def wrapped(request, *args, **kwargs): if not test_func(request.user): - raise PermissionDenied() - return view_func(request, *args, **kwargs) + raise PermissionDenied + return func(request, *args, **kwargs) - return _wrapped_view - - return decorator + return wrapped + def decorator(class_or_function): + if inspect.isclass(class_or_function): + return method_decorator(function_decorator, name="dispatch")(class_or_function) -def internal_required(view_func): - """ - Decorator for views that checks that the user is logged in and not an external user - """ - - def check_user(user): - return not user.is_external - - return user_passes_test(check_user)(view_func) - - -def staff_permission_required(view_func): - """ - Decorator for views that checks that the user is logged in and staff (regardless of staff mode!) - """ - - def check_user(user): - return user.has_staff_permission + assert inspect.isfunction(class_or_function) + return function_decorator(class_or_function) - return user_passes_test(check_user)(view_func) - - -def manager_required(view_func): - """ - Decorator for views that checks that the user is logged in and a manager - """ + return decorator - def check_user(user): - return user.is_manager - return user_passes_test(check_user)(view_func) +def internal_required(user): + return not user.is_external -def reviewer_required(view_func): - """ - Decorator for views that checks that the user is logged in and a reviewer - """ +def staff_permission_required(user): + return user.has_staff_permission - def check_user(user): - return user.is_reviewer - return user_passes_test(check_user)(view_func) +def manager_required(user): + return user.is_manager -def grade_publisher_required(view_func): - """ - Decorator for views that checks that the user is logged in and a grade publisher - """ +def reviewer_required(user): + return user.is_reviewer - def check_user(user): - return user.is_grade_publisher - return user_passes_test(check_user)(view_func) +def grade_publisher_required(user): + return user.is_grade_publisher def grade_publisher_or_manager_required(view_func): diff --git a/evap/grades/urls.py b/evap/grades/urls.py index c932577a57..6b082602c0 100644 --- a/evap/grades/urls.py +++ b/evap/grades/urls.py @@ -6,6 +6,7 @@ urlpatterns = [ path("", views.index, name="index"), + path("newindex", views.IndexView.as_view(), name="newindex"), path("download/", views.download_grades, name="download_grades"), path("semester/", views.semester_view, name="semester_view"), diff --git a/evap/grades/views.py b/evap/grades/views.py index 2e9556fe64..0ab46dcdd5 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -6,6 +6,7 @@ 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 +from django.views.generic.base import TemplateView from evap.evaluation.auth import ( grade_downloader_required, @@ -18,6 +19,18 @@ from evap.grades.models import GradeDocument +@grade_publisher_required +class IndexView(TemplateView): + template_name = "grades_index.html" + + def get_context_data(self, **kwargs): + return { + **super().get_context_data(**kwargs), + "semesters": Semester.objects.filter(grade_documents_are_deleted=False), + "disable_breadcrumb_grades": True, + } + + @grade_publisher_required def index(request): template_data = { From 09f3f49b36ff527fa2f99cb9f3030b0c3ed4f0ad Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 5 Jun 2023 22:48:39 +0200 Subject: [PATCH 02/39] more work --- evap/evaluation/auth.py | 68 +++++++++++++-------------------- evap/grades/urls.py | 8 ++-- evap/grades/views.py | 83 ++++++++++++++++++++--------------------- 3 files changed, 70 insertions(+), 89 deletions(-) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index a2e2c65e69..f413613b7d 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -68,106 +68,90 @@ def decorator(class_or_function): return decorator +@class_or_function_check_decorator def internal_required(user): return not user.is_external +@class_or_function_check_decorator def staff_permission_required(user): return user.has_staff_permission +@class_or_function_check_decorator def manager_required(user): return user.is_manager +@class_or_function_check_decorator def reviewer_required(user): return user.is_reviewer +@class_or_function_check_decorator def grade_publisher_required(user): return user.is_grade_publisher -def grade_publisher_or_manager_required(view_func): +@class_or_function_check_decorator +def grade_publisher_or_manager_required(user): """ Decorator for views that checks that the user is logged in and a grade publisher or a manager """ + return user.is_grade_publisher or user.is_manager - def check_user(user): - return user.is_grade_publisher or user.is_manager - return user_passes_test(check_user)(view_func) - - -def grade_downloader_required(view_func): +@class_or_function_check_decorator +def grade_downloader_required(user): """ Decorator for views that checks that the user is logged in and can download grades """ - - def check_user(user): - return user.can_download_grades - - return user_passes_test(check_user)(view_func) + return user.can_download_grades -def responsible_or_contributor_or_delegate_required(view_func): +@class_or_function_check_decorator +def responsible_or_contributor_or_delegate_required(user): """ Decorator for views that checks that the user is logged in, is responsible for a course, or is a contributor, or is a delegate. """ + return user.is_responsible_or_contributor_or_delegate - def check_user(user): - return user.is_responsible_or_contributor_or_delegate - return user_passes_test(check_user)(view_func) - - -def editor_or_delegate_required(view_func): +@class_or_function_check_decorator +def editor_or_delegate_required(user): """ Decorator for views that checks that the user is logged in, has edit rights for at least one evaluation or is a delegate for such a person. """ + return user.is_editor_or_delegate - def check_user(user): - return user.is_editor_or_delegate - - return user_passes_test(check_user)(view_func) - -def editor_required(view_func): +@class_or_function_check_decorator +def editor_required(user): """ Decorator for views that checks that the user is logged in and has edit right for at least one evaluation. """ + return user.is_editor - def check_user(user): - return user.is_editor - - return user_passes_test(check_user)(view_func) - -def participant_required(view_func): +@class_or_function_check_decorator +def participant_required(user): """ Decorator for views that checks that the user is logged in and participates in at least one evaluation. """ + return user.is_participant - def check_user(user): - return user.is_participant - - return user_passes_test(check_user)(view_func) - -def reward_user_required(view_func): +@class_or_function_check_decorator +def reward_user_required(user): """ Decorator for views that checks that the user is logged in and can use reward points. """ - - def check_user(user): - return can_reward_points_be_used_by(user) - - return user_passes_test(check_user)(view_func) + return can_reward_points_be_used_by(user) # see https://mozilla-django-oidc.readthedocs.io/en/stable/ diff --git a/evap/grades/urls.py b/evap/grades/urls.py index 6b082602c0..541d26e2cb 100644 --- a/evap/grades/urls.py +++ b/evap/grades/urls.py @@ -5,12 +5,10 @@ app_name = "grades" urlpatterns = [ - path("", views.index, name="index"), - path("newindex", views.IndexView.as_view(), name="newindex"), - + path("", views.IndexView.as_view(), name="index"), path("download/", views.download_grades, name="download_grades"), - path("semester/", views.semester_view, name="semester_view"), - path("course/", views.course_view, name="course_view"), + path("semester/", views.SemesterView.as_view(), name="semester_view"), + path("course/", views.CourseView.as_view(), name="course_view"), path("course//upload", views.upload_grades, name="upload_grades"), path("grade_document//edit", views.edit_grades, name="edit_grades"), diff --git a/evap/grades/views.py b/evap/grades/views.py index 0ab46dcdd5..b95f6f84f8 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -6,7 +6,7 @@ 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 -from django.views.generic.base import TemplateView +from django.views.generic import DetailView, TemplateView from evap.evaluation.auth import ( grade_downloader_required, @@ -24,22 +24,12 @@ class IndexView(TemplateView): template_name = "grades_index.html" def get_context_data(self, **kwargs): - return { - **super().get_context_data(**kwargs), + return super().get_context_data(**kwargs) | { "semesters": Semester.objects.filter(grade_documents_are_deleted=False), "disable_breadcrumb_grades": True, } -@grade_publisher_required -def index(request): - template_data = { - "semesters": Semester.objects.filter(grade_documents_are_deleted=False), - "disable_breadcrumb_grades": True, - } - return render(request, "grades_index.html", template_data) - - def course_grade_document_count_tuples(courses: QuerySet[Course]) -> list[tuple[Course, int, int]]: courses = courses.prefetch_related("degrees", "responsibles", "evaluations", "grade_documents") @@ -54,42 +44,51 @@ def course_grade_document_count_tuples(courses: QuerySet[Course]) -> list[tuple[ @grade_publisher_required -def semester_view(request, semester_id): - semester = get_object_or_404(Semester, id=semester_id) - if semester.grade_documents_are_deleted: - raise PermissionDenied +class SemesterView(DetailView): + template_name = "grades_semester_view.html" + model = Semester + pk_url_kwarg = "semester_id" - courses = ( - semester.courses.filter(evaluations__wait_for_grade_upload_before_publishing=True) - .exclude(evaluations__state=Evaluation.State.NEW) - .distinct() - ) - courses = course_grade_document_count_tuples(courses) + def get_object(self, *args, **kwargs): + semester = super().get_object(*args, **kwargs) + if semester.grade_documents_are_deleted: + raise PermissionDenied + return semester - template_data = { - "semester": semester, - "courses": courses, - "disable_if_archived": "disabled" if semester.grade_documents_are_deleted else "", - "disable_breadcrumb_semester": True, - } - return render(request, "grades_semester_view.html", template_data) + def get_context_data(self, **kwargs): + courses = ( + self.object.courses.filter(evaluations__wait_for_grade_upload_before_publishing=True) + .exclude(evaluations__state=Evaluation.State.NEW) + .distinct() + ) + courses = course_grade_document_count_tuples(courses) + + return super().get_context_data(**kwargs) | { + "courses": courses, + "disable_if_archived": "disabled", + "disable_breadcrumb_semester": True, + } @grade_publisher_or_manager_required -def course_view(request, course_id): - course = get_object_or_404(Course, id=course_id) - semester = course.semester - if semester.grade_documents_are_deleted: - raise PermissionDenied +class CourseView(DetailView): + template_name = "grades_course_view.html" + model = Course + pk_url_kwarg = "course_id" - template_data = { - "semester": semester, - "course": course, - "grade_documents": course.grade_documents.all(), - "disable_if_archived": "disabled" if semester.grade_documents_are_deleted else "", - "disable_breadcrumb_course": True, - } - return render(request, "grades_course_view.html", template_data) + def get_object(self, *args, **kwargs): + course = super().get_object(*args, **kwargs) + if course.semester.grade_documents_are_deleted: + raise PermissionDenied + return course + + def get_context_data(self, **kwargs): + return super().get_context_data(**kwargs) | { + "semester": self.object.semester, + "grade_documents": self.object.grade_documents.all(), + "disable_if_archived": "disabled", + "disable_breadcrumb_course": True, + } def on_grading_process_finished(course): From 7ca0ad7ad727262cf5de69197a5023be27732b27 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 19 Jun 2023 21:02:27 +0200 Subject: [PATCH 03/39] this one is controversial --- evap/grades/urls.py | 2 +- evap/grades/views.py | 76 +++++++++++++++++++++++++------------------- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/evap/grades/urls.py b/evap/grades/urls.py index 541d26e2cb..4e88c4507c 100644 --- a/evap/grades/urls.py +++ b/evap/grades/urls.py @@ -9,7 +9,7 @@ path("download/", views.download_grades, name="download_grades"), path("semester/", views.SemesterView.as_view(), name="semester_view"), path("course/", views.CourseView.as_view(), name="course_view"), - path("course//upload", views.upload_grades, name="upload_grades"), + path("course//upload", views.UploadGradesView.as_view(), name="upload_grades"), path("grade_document//edit", views.edit_grades, name="edit_grades"), path("delete_grades", views.delete_grades, name="delete_grades"), diff --git a/evap/grades/views.py b/evap/grades/views.py index b95f6f84f8..792ede5373 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -2,11 +2,12 @@ from django.contrib import messages from django.core.exceptions import PermissionDenied from django.db.models.query import QuerySet -from django.http import FileResponse, HttpResponse +from django.http import FileResponse, HttpResponse, HttpResponseRedirect 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 -from django.views.generic import DetailView, TemplateView +from django.views.generic import DetailView, TemplateView, UpdateView +from django.urls import reverse from evap.evaluation.auth import ( grade_downloader_required, @@ -105,43 +106,54 @@ def on_grading_process_finished(course): @grade_publisher_required -def upload_grades(request, course_id): - course = get_object_or_404(Course, id=course_id) - semester = course.semester - if semester.grade_documents_are_deleted: - raise PermissionDenied +class UploadGradesView(UpdateView): + model = GradeDocument + form_class = GradeDocumentForm + template_name = "grades_upload_form.html" + + def dispatch(self, request, course_id): + self.course = get_object_or_404(Course, id=course_id) + if self.course.semester.grade_documents_are_deleted: + raise PermissionDenied + self.final_grades = request.GET.get("final") == "true" # if parameter is not given, assume midterm grades - final_grades = request.GET.get("final") == "true" # if parameter is not given, assume midterm grades + return super().dispatch(request) - grade_document = GradeDocument(course=course) - if final_grades: - grade_document.type = GradeDocument.Type.FINAL_GRADES - grade_document.description_en = settings.DEFAULT_FINAL_GRADES_DESCRIPTION_EN - grade_document.description_de = settings.DEFAULT_FINAL_GRADES_DESCRIPTION_DE - else: - grade_document.type = GradeDocument.Type.MIDTERM_GRADES - grade_document.description_en = settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_EN - grade_document.description_de = settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_DE + def get_object(self): + if self.final_grades: + type_specific_kwargs = { + "type": GradeDocument.Type.FINAL_GRADES, + "description_en": settings.DEFAULT_FINAL_GRADES_DESCRIPTION_EN, + "description_de": settings.DEFAULT_FINAL_GRADES_DESCRIPTION_DE, + } + else: + type_specific_kwargs = { + "type": GradeDocument.Type.MIDTERM_GRADES, + "description_en": settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_EN, + "description_de": settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_DE, + } - form = GradeDocumentForm(request.POST or None, request.FILES or None, instance=grade_document) + return GradeDocument(course=self.course, **type_specific_kwargs) - if form.is_valid(): - form.save(modifying_user=request.user) + def get_success_url(self): + return reverse("grades:course_view", args=[self.course.id]) - if final_grades: - on_grading_process_finished(course) + def get_context_data(self, **kwargs): + return super().get_context_data(**kwargs) | { + "semester": self.course.semester, + "course": self.course, + "final_grades": self.final_grades, + "show_automated_publishing_info": self.final_grades, + } - messages.success(request, _("Successfully uploaded grades.")) - return redirect("grades:course_view", course.id) + def form_valid(self, form): + # Don't call super().form_valid() to avoid saving again + self.object = form.save(modifying_user=self.request.user) + if self.final_grades: + on_grading_process_finished(self.course) - template_data = { - "semester": semester, - "course": course, - "form": form, - "final_grades": final_grades, - "show_automated_publishing_info": final_grades, - } - return render(request, "grades_upload_form.html", template_data) + messages.success(self.request, _("Successfully uploaded grades.")) + return HttpResponseRedirect(self.get_success_url()) @require_POST From c6f94977a7b0edb7bcc783aeb4923429955535d6 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 26 Jun 2023 18:16:49 +0200 Subject: [PATCH 04/39] some nice generic views --- evap/rewards/urls.py | 4 ++-- evap/rewards/views.py | 40 ++++++++++++++++++---------------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/evap/rewards/urls.py b/evap/rewards/urls.py index bc4c135a19..a30b734328 100644 --- a/evap/rewards/urls.py +++ b/evap/rewards/urls.py @@ -8,8 +8,8 @@ path("", views.index, name="index"), path("reward_point_redemption_events/", views.reward_point_redemption_events, name="reward_point_redemption_events"), - path("reward_point_redemption_event/create", views.reward_point_redemption_event_create, name="reward_point_redemption_event_create"), - path("reward_point_redemption_event//edit", views.reward_point_redemption_event_edit, name="reward_point_redemption_event_edit"), + path("reward_point_redemption_event/create", views.RewardPointRedemptionEventCreateView.as_view(), name="reward_point_redemption_event_create"), + path("reward_point_redemption_event//edit", views.RewardPointRedemptionEventEditView.as_view(), name="reward_point_redemption_event_edit"), path("reward_point_redemption_event//export", views.reward_point_redemption_event_export, name="reward_point_redemption_event_export"), path("reward_point_redemption_event/delete", views.reward_point_redemption_event_delete, name="reward_point_redemption_event_delete"), diff --git a/evap/rewards/views.py b/evap/rewards/views.py index 0dc58eee4f..7b38b201e1 100644 --- a/evap/rewards/views.py +++ b/evap/rewards/views.py @@ -1,13 +1,16 @@ from datetime import datetime +from django.contrib.messages.views import SuccessMessageMixin from django.contrib import messages from django.core.exceptions import BadRequest, SuspiciousOperation from django.db.models import Sum from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect, render from django.utils.translation import get_language -from django.utils.translation import gettext as _ +from django.views.generic import CreateView, UpdateView +from django.utils.translation import gettext as _, gettext_lazy from django.views.decorators.http import require_POST +from django.urls import reverse_lazy from evap.evaluation.auth import manager_required, reward_user_required from evap.evaluation.models import Semester @@ -104,30 +107,23 @@ def reward_point_redemption_events(request): @manager_required -def reward_point_redemption_event_create(request): - event = RewardPointRedemptionEvent() - form = RewardPointRedemptionEventForm(request.POST or None, instance=event) - - if form.is_valid(): - form.save() - messages.success(request, _("Successfully created event.")) - return redirect("rewards:reward_point_redemption_events") - - return render(request, "rewards_reward_point_redemption_event_form.html", {"form": form}) +class RewardPointRedemptionEventCreateView(SuccessMessageMixin, CreateView): + model = RewardPointRedemptionEvent + form_class = RewardPointRedemptionEventForm + template_name = "rewards_reward_point_redemption_event_form.html" + success_url = reverse_lazy("rewards:reward_point_redemption_events") + success_message = gettext_lazy("Successfully created event.") @manager_required -def reward_point_redemption_event_edit(request, event_id): - event = get_object_or_404(RewardPointRedemptionEvent, id=event_id) - form = RewardPointRedemptionEventForm(request.POST or None, instance=event) - - if form.is_valid(): - event = form.save() - - messages.success(request, _("Successfully updated event.")) - return redirect("rewards:reward_point_redemption_events") - - return render(request, "rewards_reward_point_redemption_event_form.html", {"event": event, "form": form}) +class RewardPointRedemptionEventEditView(SuccessMessageMixin, UpdateView): + model = RewardPointRedemptionEvent + form_class = RewardPointRedemptionEventForm + template_name = "rewards_reward_point_redemption_event_form.html" + success_url = reverse_lazy("rewards:reward_point_redemption_events") + success_message = gettext_lazy("Successfully updated event.") + pk_url_kwarg = "event_id" + context_object_name = "event" @require_POST From e7b6fa530047e0740b8bf8aa4ae600fe9c1adb7e Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 26 Jun 2023 20:12:43 +0200 Subject: [PATCH 05/39] more views --- evap/staff/urls.py | 4 ++-- evap/staff/views.py | 40 ++++++++++++++++++++-------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 97603cb9ba..5267b676eb 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -9,9 +9,9 @@ path("", views.index, name="index"), path("semester/", RedirectView.as_view(url='/staff/', permanent=True)), - path("semester/create", views.semester_create, name="semester_create"), + path("semester/create", views.SemesterCreateView.as_view(), name="semester_create"), path("semester/", views.semester_view, name="semester_view"), - path("semester//edit", views.semester_edit, name="semester_edit"), + path("semester//edit", views.SemesterEditView.as_view(), name="semester_edit"), path("semester/make_active", views.semester_make_active, name="semester_make_active"), path("semester/delete", views.semester_delete, name="semester_delete"), path("semester//import", views.semester_import, name="semester_import"), diff --git a/evap/staff/views.py b/evap/staff/views.py index d6f143a7b6..bfff989ba5 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -12,6 +12,8 @@ from django.core.exceptions import PermissionDenied, SuspiciousOperation from django.db import IntegrityError, transaction from django.db.models import BooleanField, Case, Count, ExpressionWrapper, IntegerField, Prefetch, Q, Sum, When +from django.contrib.messages.views import SuccessMessageMixin +from django.views.generic import CreateView, UpdateView from django.dispatch import receiver from django.forms import formset_factory from django.forms.models import inlineformset_factory, modelformset_factory @@ -571,15 +573,26 @@ def evaluation_operation(request, semester_id): @manager_required -def semester_create(request): - form = SemesterForm(request.POST or None) +class SemesterCreateView(SuccessMessageMixin, CreateView): + template_name = "staff_semester_form.html" + model = Semester + form_class = SemesterForm + success_message = gettext_lazy("Successfully created semester.") - if form.is_valid(): - semester = form.save() - messages.success(request, _("Successfully created semester.")) - return redirect("staff:semester_view", semester.id) + def get_success_url(self): + return reverse("staff:semester_view", args=[self.object.id]) - return render(request, "staff_semester_form.html", {"form": form}) + +@manager_required +class SemesterEditView(SuccessMessageMixin, UpdateView): + template_name = "staff_semester_form.html" + model = Semester + form_class = SemesterForm + pk_url_kwarg = "semester_id" + success_message = gettext_lazy("Successfully updated semester.") + + def get_success_url(self): + return reverse("staff:semester_view", args=[self.object.id]) @require_POST @@ -595,19 +608,6 @@ def semester_make_active(request): return HttpResponse() -@manager_required -def semester_edit(request, semester_id): - semester = get_object_or_404(Semester, id=semester_id) - form = SemesterForm(request.POST or None, instance=semester) - - if form.is_valid(): - semester = form.save() - messages.success(request, _("Successfully updated semester.")) - return redirect("staff:semester_view", semester.id) - - return render(request, "staff_semester_form.html", {"semester": semester, "form": form}) - - @require_POST @manager_required def semester_delete(request): From 614d96d018ea10e1879d7456552caa8b171dd6ea Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 26 Jun 2023 22:49:01 +0200 Subject: [PATCH 06/39] yet another view --- evap/staff/tests/test_views.py | 14 +++---- evap/staff/urls.py | 2 +- evap/staff/views.py | 71 ++++++++++++++++++---------------- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/evap/staff/tests/test_views.py b/evap/staff/tests/test_views.py index a906402546..9c9aed8522 100644 --- a/evap/staff/tests/test_views.py +++ b/evap/staff/tests/test_views.py @@ -1946,20 +1946,20 @@ def test_edit_course(self): self.course = Course.objects.get(pk=self.course.pk) self.assertEqual(self.course.name_en, "A different name") - @patch("evap.staff.views.redirect") - def test_operation_redirects(self, mock_redirect): - mock_redirect.side_effect = lambda *_args: HttpResponse() + @patch("evap.staff.views.reverse") + def test_operation_redirects(self, mock_reverse): + mock_reverse.return_value = "very_legit_url" self.prepare_form("a").submit("operation", value="save") - self.assertEqual(mock_redirect.call_args.args[0], "staff:semester_view") + self.assertEqual(mock_reverse.call_args.args[0], "staff:semester_view") self.prepare_form("b").submit("operation", value="save_create_evaluation") - self.assertEqual(mock_redirect.call_args.args[0], "staff:evaluation_create_for_course") + self.assertEqual(mock_reverse.call_args.args[0], "staff:evaluation_create_for_course") self.prepare_form("c").submit("operation", value="save_create_single_result") - self.assertEqual(mock_redirect.call_args.args[0], "staff:single_result_create_for_course") + self.assertEqual(mock_reverse.call_args.args[0], "staff:single_result_create_for_course") - self.assertEqual(mock_redirect.call_count, 3) + self.assertEqual(mock_reverse.call_count, 3) class TestCourseDeleteView(DeleteViewTestMixin, WebTestStaffMode): diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 5267b676eb..55c4554319 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -40,7 +40,7 @@ path("semester//course/create", views.course_create, name="course_create"), path("course/delete", views.course_delete, name="course_delete"), - path("course//edit", views.course_edit, name="course_edit"), + path("course//edit", views.CourseEditView.as_view(), name="course_edit"), path("course//copy", views.course_copy, name="course_copy"), path("semester//singleresult/create", views.single_result_create_for_semester, name="single_result_create_for_semester"), diff --git a/evap/staff/views.py b/evap/staff/views.py index bfff989ba5..3f71ec9c39 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -1039,41 +1039,46 @@ def course_copy(request, course_id): @manager_required -def course_edit(request, course_id): - course = get_object_or_404(Course, id=course_id) - - course_form = CourseForm(request.POST or None, instance=course) - editable = course.can_be_edited_by_manager - - if request.method == "POST" and not editable: - raise SuspiciousOperation("Modifying this course is not allowed.") - - operation = request.POST.get("operation") - - if course_form.is_valid(): - if operation not in ("save", "save_create_evaluation", "save_create_single_result"): - raise SuspiciousOperation("Invalid POST operation") - - if course_form.has_changed(): - course = course_form.save() - update_template_cache_of_published_evaluations_in_course(course) - - messages.success(request, _("Successfully updated course.")) - if operation == "save_create_evaluation": - return redirect("staff:evaluation_create_for_course", course.id) - if operation == "save_create_single_result": - return redirect("staff:single_result_create_for_course", course.id) +class CourseEditView(SuccessMessageMixin, UpdateView): + model = Course + pk_url_kwarg = "course_id" + form_class = CourseForm + template_name = "staff_course_form.html" + success_message = gettext_lazy("Successfully updated course.") + + object: Course + + def get_object(self, *args, **kwargs): + course = super().get_object(*args, **kwargs) + if self.request.method == "POST" and not course.can_be_edited_by_manager: + raise SuspiciousOperation("Modifying this course is not allowed.") + return course + + def get_context_data(self, **kwargs): + context_data = super().get_context_data(**kwargs) | { + "semester": self.object.semester, + "editable": self.object.can_be_edited_by_manager, + "disable_breadcrumb_course": True, + } + context_data["course_form"] = context_data.pop("form") + return context_data - return redirect("staff:semester_view", course.semester.id) + def form_valid(self, form): + response = super().form_valid(form) + if form.has_changed(): + update_template_cache_of_published_evaluations_in_course(self.object) + return response - template_data = { - "course": course, - "semester": course.semester, - "course_form": course_form, - "editable": editable, - "disable_breadcrumb_course": True, - } - return render(request, "staff_course_form.html", template_data) + def get_success_url(self): + match self.request.POST.get("operation"): + case "save": + return reverse("staff:semester_view", args=[self.object.semester.id]) + case "save_create_evaluation": + return reverse("staff:evaluation_create_for_course", args=[self.object.id]) + case "save_create_single_result": + return reverse("staff:single_result_create_for_course", args=[self.object.id]) + case _: + raise SuspiciousOperation("Invalid POST operation") @require_POST From d4e55367c506965aec5ee807d0b70fade0f19652 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 3 Jul 2023 17:44:56 +0200 Subject: [PATCH 07/39] make something atomic --- evap/staff/views.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/evap/staff/views.py b/evap/staff/views.py index 3f71ec9c39..9a045701ed 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -615,12 +615,13 @@ def semester_delete(request): if not semester.can_be_deleted_by_manager: raise SuspiciousOperation("Deleting semester not allowed") - RatingAnswerCounter.objects.filter(contribution__evaluation__course__semester=semester).delete() - TextAnswer.objects.filter(contribution__evaluation__course__semester=semester).delete() - Contribution.objects.filter(evaluation__course__semester=semester).delete() - Evaluation.objects.filter(course__semester=semester).delete() - Course.objects.filter(semester=semester).delete() - semester.delete() + with transaction.atomic(): + RatingAnswerCounter.objects.filter(contribution__evaluation__course__semester=semester).delete() + TextAnswer.objects.filter(contribution__evaluation__course__semester=semester).delete() + Contribution.objects.filter(evaluation__course__semester=semester).delete() + Evaluation.objects.filter(course__semester=semester).delete() + Course.objects.filter(semester=semester).delete() + semester.delete() return redirect("staff:index") From 729316b7acfe127fbfbac33bdff6b8591e4cf003 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 3 Jul 2023 18:11:57 +0200 Subject: [PATCH 08/39] classic formview --- evap/staff/urls.py | 2 +- evap/staff/views.py | 46 ++++++++++++++++++++++++++++++--------------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 55c4554319..23dd7841ef 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -15,7 +15,7 @@ path("semester/make_active", views.semester_make_active, name="semester_make_active"), path("semester/delete", views.semester_delete, name="semester_delete"), path("semester//import", views.semester_import, name="semester_import"), - path("semester//export", views.semester_export, name="semester_export"), + path("semester//export", views.SemesterExportView.as_view(), name="semester_export"), path("semester//raw_export", views.semester_raw_export, name="semester_raw_export"), path("semester//participation_export", views.semester_participation_export, name="semester_participation_export"), path("semester//vote_timestamps_export", views.vote_timestamps_export, name="vote_timestamps_export"), diff --git a/evap/staff/views.py b/evap/staff/views.py index 9a045701ed..987273af97 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -13,7 +13,8 @@ from django.db import IntegrityError, transaction from django.db.models import BooleanField, Case, Count, ExpressionWrapper, IntegerField, Prefetch, Q, Sum, When from django.contrib.messages.views import SuccessMessageMixin -from django.views.generic import CreateView, UpdateView +from django.views.generic import CreateView, UpdateView, FormView +from django.views.generic.detail import SingleObjectMixin from django.dispatch import receiver from django.forms import formset_factory from django.forms.models import inlineformset_factory, modelformset_factory @@ -682,27 +683,42 @@ def semester_import(request, semester_id): @manager_required -def semester_export(request, semester_id): - semester = get_object_or_404(Semester, id=semester_id) +class SemesterExportView(SingleObjectMixin, FormView): + model = Semester + pk_url_kwarg = "semester_id" + form_class = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) + template_name = "staff_semester_export.html" - ExportSheetFormset = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) - formset = ExportSheetFormset(request.POST or None, form_kwargs={"semester": semester}) + def dispatch(self, *args, **kwargs): + self.object = self.semester = self.get_object() + return super().dispatch(*args, **kwargs) - if formset.is_valid(): - include_not_enough_voters = request.POST.get("include_not_enough_voters") == "on" - include_unpublished = request.POST.get("include_unpublished") == "on" - selection_list = [] - for form in formset: - selection_list.append((form.cleaned_data["selected_degrees"], form.cleaned_data["selected_course_types"])) + def get_form_kwargs(self): + return super().get_form_kwargs() | {"form_kwargs": {"semester": self.semester}} - filename = f"Evaluation-{semester.name}-{get_language()}.xls" + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["formset"] = context.pop("form") + return context + + def form_valid(self, formset): + include_not_enough_voters = self.request.POST.get("include_not_enough_voters") == "on" + include_unpublished = self.request.POST.get("include_unpublished") == "on" + selection_list = [ + (form.cleaned_data["selected_degrees"], form.cleaned_data["selected_course_types"]) for form in formset + ] + filename = f"Evaluation-{self.semester.name}-{get_language()}.xls" response = AttachmentResponse(filename, content_type="application/vnd.ms-excel") - ResultsExporter().export(response, [semester], selection_list, include_not_enough_voters, include_unpublished) + ResultsExporter().export( + response, + [self.semester], + selection_list, + include_not_enough_voters, + include_unpublished, + ) return response - return render(request, "staff_semester_export.html", {"semester": semester, "formset": formset}) - @manager_required def semester_raw_export(_request, semester_id): From fe893221ddacedebab0ec2134a4a03984e81d6c1 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 3 Jul 2023 21:36:29 +0200 Subject: [PATCH 09/39] staff degree and course type index --- evap/staff/urls.py | 4 +-- evap/staff/views.py | 60 ++++++++++++++++++++++++++++----------------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 23dd7841ef..3149472099 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -64,9 +64,9 @@ path("questionnaire/questionnaire_visibility", views.questionnaire_visibility, name="questionnaire_visibility"), path("questionnaire/questionnaire_set_locked", views.questionnaire_set_locked, name="questionnaire_set_locked"), - path("degrees/", views.degree_index, name="degree_index"), + path("degrees/", views.DegreeIndexView.as_view(), name="degree_index"), - path("course_types/", views.course_type_index, name="course_type_index"), + path("course_types/", views.CourseTypeIndexView.as_view(), name="course_type_index"), path("course_types/merge", views.course_type_merge_selection, name="course_type_merge_selection"), path("course_types//merge/", views.course_type_merge, name="course_type_merge"), diff --git a/evap/staff/views.py b/evap/staff/views.py index 987273af97..2ccc8b9226 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -20,7 +20,7 @@ from django.forms.models import inlineformset_factory, modelformset_factory from django.http import Http404, HttpRequest, HttpResponse, HttpResponseBadRequest, HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect, render -from django.urls import reverse +from django.urls import reverse, reverse_lazy from django.utils.html import format_html from django.utils.translation import get_language from django.utils.translation import gettext as _ @@ -2000,37 +2000,51 @@ def questionnaire_set_locked(request): @manager_required -def degree_index(request): - degrees = Degree.objects.all() - - DegreeFormset = modelformset_factory( - Degree, form=DegreeForm, formset=ModelWithImportNamesFormset, can_delete=True, extra=1 +class DegreeIndexView(SuccessMessageMixin, FormView): + model = Degree + form_class = modelformset_factory( + Degree, + form=DegreeForm, + formset=ModelWithImportNamesFormset, + can_delete=True, + extra=1, ) - formset = DegreeFormset(request.POST or None, queryset=degrees) + template_name = "staff_degree_index.html" + success_url = reverse_lazy("staff:degree_index") + success_message = gettext_lazy("Successfully updated the degrees.") - if formset.is_valid(): - formset.save() - messages.success(request, _("Successfully updated the degrees.")) - return redirect("staff:degree_index") + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["formset"] = context.pop("form") + return context - return render(request, "staff_degree_index.html", {"formset": formset}) + def form_valid(self, form): + form.save() + return super().form_valid(form) @manager_required -def course_type_index(request): - course_types = CourseType.objects.all() - - CourseTypeFormset = modelformset_factory( - CourseType, form=CourseTypeForm, formset=ModelWithImportNamesFormset, can_delete=True, extra=1 +class CourseTypeIndexView(SuccessMessageMixin, FormView): + model = CourseType + form_class = modelformset_factory( + CourseType, + form=CourseTypeForm, + formset=ModelWithImportNamesFormset, + can_delete=True, + extra=1, ) - formset = CourseTypeFormset(request.POST or None, queryset=course_types) + template_name = "staff_course_type_index.html" + success_url = reverse_lazy("staff:course_type_index") + success_message = gettext_lazy("Successfully updated the course types.") - if formset.is_valid(): - formset.save() - messages.success(request, _("Successfully updated the course types.")) - return redirect("staff:course_type_index") + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["formset"] = context.pop("form") + return context - return render(request, "staff_course_type_index.html", {"formset": formset}) + def form_valid(self, form): + form.save() + return super().form_valid(form) @manager_required From 8af1a6b2254f58838fe848493dee2c677ab96cac Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 3 Jul 2023 21:49:20 +0200 Subject: [PATCH 10/39] user create view --- evap/staff/forms.py | 1 + evap/staff/urls.py | 2 +- evap/staff/views.py | 15 ++++++--------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/evap/staff/forms.py b/evap/staff/forms.py index e06898b3d0..6d2746e81f 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -1046,6 +1046,7 @@ def save(self, *args, **kw): cache_results(evaluation) self.instance.save() + return self.instance class UserMergeSelectionForm(forms.Form): diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 3149472099..ada5b3e012 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -71,7 +71,7 @@ path("course_types//merge/", views.course_type_merge, name="course_type_merge"), path("user/", views.user_index, name="user_index"), - path("user/create", views.user_create, name="user_create"), + path("user/create", views.UserCreateView.as_view(), name="user_create"), path("user/import", views.user_import, name="user_import"), path("user//edit", views.user_edit, name="user_edit"), path("user/list", views.user_list, name="user_list"), diff --git a/evap/staff/views.py b/evap/staff/views.py index 2ccc8b9226..795e45e3d2 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -2153,15 +2153,12 @@ def user_list(request): @manager_required -def user_create(request): - form = UserForm(request.POST or None, instance=UserProfile()) - - if form.is_valid(): - form.save() - messages.success(request, _("Successfully created user.")) - return redirect("staff:user_index") - - return render(request, "staff_user_form.html", {"form": form}) +class UserCreateView(SuccessMessageMixin, CreateView): + model = UserProfile + form_class = UserForm + template_name = "staff_user_form.html" + success_url = reverse_lazy("staff:user_index") + success_message = gettext_lazy("Successfully created user.") @manager_required From 1ebde2f46a5029d7093b6c322297118e08768cfa Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 3 Jul 2023 21:54:46 +0200 Subject: [PATCH 11/39] faq index --- evap/staff/urls.py | 2 +- evap/staff/views.py | 24 ++++++++++++++---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/evap/staff/urls.py b/evap/staff/urls.py index ada5b3e012..6cc09661a0 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -86,7 +86,7 @@ path("text_answer_warnings/", views.text_answer_warnings_index, name="text_answer_warnings"), - path("faq/", views.faq_index, name="faq_index"), + path("faq/", views.FaqIndexView.as_view(), name="faq_index"), path("faq/", views.faq_section, name="faq_section"), path("infotexts/", views.infotexts, name="infotexts"), diff --git a/evap/staff/views.py b/evap/staff/views.py index 795e45e3d2..49d38e14dc 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -2399,18 +2399,22 @@ def template_edit(request, template_id): @manager_required -def faq_index(request): - sections = FaqSection.objects.all() +class FaqIndexView(SuccessMessageMixin, FormView): + model = FaqSection + form_class = modelformset_factory(FaqSection, form=FaqSectionForm, can_delete=True, extra=1) + template_name = "staff_faq_index.html" + success_url = reverse_lazy("staff:faq_index") + success_message = gettext_lazy("Successfully updated the FAQ sections.") - SectionFormset = modelformset_factory(FaqSection, form=FaqSectionForm, can_delete=True, extra=1) - formset = SectionFormset(request.POST or None, queryset=sections) - - if formset.is_valid(): - formset.save() - messages.success(request, _("Successfully updated the FAQ sections.")) - return redirect("staff:faq_index") + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["formset"] = context.pop("form") + context["sections"] = FaqSection.objects.all() + return context - return render(request, "staff_faq_index.html", {"formset": formset, "sections": sections}) + def form_valid(self, form): + form.save() + return super().form_valid(form) @manager_required From c0a2b20daf332aed277e26b493b8af99a8e525ed Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 3 Jul 2023 21:59:44 +0200 Subject: [PATCH 12/39] infotexts --- evap/staff/urls.py | 2 +- evap/staff/views.py | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 6cc09661a0..385eb77424 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -89,7 +89,7 @@ path("faq/", views.FaqIndexView.as_view(), name="faq_index"), path("faq/", views.faq_section, name="faq_section"), - path("infotexts/", views.infotexts, name="infotexts"), + path("infotexts/", views.InfotextsView.as_view(), name="infotexts"), path("download_sample_file/", views.download_sample_file, name="download_sample_file"), diff --git a/evap/staff/views.py b/evap/staff/views.py index 49d38e14dc..09ee611dc1 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -2437,20 +2437,20 @@ def faq_section(request, section_id): @manager_required -def infotexts(request): - InfotextFormSet = modelformset_factory(Infotext, form=InfotextForm, edit_only=True, extra=0) - formset = InfotextFormSet(request.POST or None) +class InfotextsView(SuccessMessageMixin, FormView): + form_class = modelformset_factory(Infotext, form=InfotextForm, edit_only=True, extra=0) + template_name = "staff_infotexts.html" + success_url = reverse_lazy("staff:infotexts") + success_message = gettext_lazy("Successfully updated the infotext entries.") - if formset.is_valid(): - formset.save() - messages.success(request, _("Successfully updated the infotext entries.")) - return redirect("staff:infotexts") + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["formset"] = context.pop("form") + return context - return render( - request, - "staff_infotexts.html", - {"formset": formset}, - ) + def form_valid(self, form): + form.save() + return super().form_valid(form) @manager_required From c2021862cdb182259a507304f1221dccf77f3c08 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 10 Jul 2023 17:25:37 +0200 Subject: [PATCH 13/39] format --- evap/evaluation/auth.py | 4 ++-- evap/grades/views.py | 2 +- evap/rewards/views.py | 9 +++++---- evap/staff/views.py | 6 +++--- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index f413613b7d..0121396577 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -1,10 +1,10 @@ +import inspect from functools import wraps from typing import Callable -import inspect -from django.utils.decorators import method_decorator from django.contrib.auth.backends import ModelBackend from django.core.exceptions import PermissionDenied +from django.utils.decorators import method_decorator from mozilla_django_oidc.auth import OIDCAuthenticationBackend from evap.evaluation.models import UserProfile diff --git a/evap/grades/views.py b/evap/grades/views.py index 792ede5373..b2ffa6dc13 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -4,10 +4,10 @@ from django.db.models.query import QuerySet from django.http import FileResponse, HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect, render +from django.urls import reverse from django.utils.translation import gettext as _ from django.views.decorators.http import require_GET, require_POST from django.views.generic import DetailView, TemplateView, UpdateView -from django.urls import reverse from evap.evaluation.auth import ( grade_downloader_required, diff --git a/evap/rewards/views.py b/evap/rewards/views.py index 7b38b201e1..027d2b75ef 100644 --- a/evap/rewards/views.py +++ b/evap/rewards/views.py @@ -1,16 +1,17 @@ from datetime import datetime -from django.contrib.messages.views import SuccessMessageMixin from django.contrib import messages +from django.contrib.messages.views import SuccessMessageMixin from django.core.exceptions import BadRequest, SuspiciousOperation from django.db.models import Sum from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect, render +from django.urls import reverse_lazy from django.utils.translation import get_language -from django.views.generic import CreateView, UpdateView -from django.utils.translation import gettext as _, gettext_lazy +from django.utils.translation import gettext as _ +from django.utils.translation import gettext_lazy from django.views.decorators.http import require_POST -from django.urls import reverse_lazy +from django.views.generic import CreateView, UpdateView from evap.evaluation.auth import manager_required, reward_user_required from evap.evaluation.models import Semester diff --git a/evap/staff/views.py b/evap/staff/views.py index 09ee611dc1..645da1897f 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -9,12 +9,10 @@ import openpyxl from django.conf import settings from django.contrib import messages +from django.contrib.messages.views import SuccessMessageMixin from django.core.exceptions import PermissionDenied, SuspiciousOperation from django.db import IntegrityError, transaction from django.db.models import BooleanField, Case, Count, ExpressionWrapper, IntegerField, Prefetch, Q, Sum, When -from django.contrib.messages.views import SuccessMessageMixin -from django.views.generic import CreateView, UpdateView, FormView -from django.views.generic.detail import SingleObjectMixin from django.dispatch import receiver from django.forms import formset_factory from django.forms.models import inlineformset_factory, modelformset_factory @@ -26,6 +24,8 @@ from django.utils.translation import gettext as _ from django.utils.translation import gettext_lazy, ngettext from django.views.decorators.http import require_POST +from django.views.generic import CreateView, FormView, UpdateView +from django.views.generic.detail import SingleObjectMixin from django_stubs_ext import StrOrPromise from evap.contributor.views import export_contributor_results From 117c289f0cdbda724dcbcda9a32cc91dd226ca8e Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 10 Jul 2023 18:21:06 +0200 Subject: [PATCH 14/39] create mixins for common stuff --- evap/evaluation/tools.py | 14 +++++++++++ evap/staff/views.py | 53 ++++++---------------------------------- 2 files changed, 22 insertions(+), 45 deletions(-) diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index dac303a995..1678ec0594 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -12,6 +12,7 @@ from django.http import HttpResponse from django.shortcuts import get_object_or_404 from django.utils.translation import get_language +from django.views.generic import FormView M = TypeVar("M", bound=Model) T = TypeVar("T") @@ -138,6 +139,19 @@ def assert_not_none(value: T | None) -> T: return value +class FormsetView(FormView): + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["formset"] = context.pop("form") + return context + + +class SaveValidFormMixin: + def form_valid(self, form): + form.save() + return super().form_valid(form) + + class AttachmentResponse(HttpResponse): """ Helper class that sets the correct Content-Disposition header for a given diff --git a/evap/staff/views.py b/evap/staff/views.py index 645da1897f..7e87621bf4 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -51,7 +51,9 @@ ) from evap.evaluation.tools import ( AttachmentResponse, + FormsetView, HttpResponseNoContent, + SaveValidFormMixin, get_object_from_dict_pk_entry_or_logged_40x, get_parameter_from_url_or_session, sort_formset, @@ -683,7 +685,7 @@ def semester_import(request, semester_id): @manager_required -class SemesterExportView(SingleObjectMixin, FormView): +class SemesterExportView(SingleObjectMixin, FormsetView): model = Semester pk_url_kwarg = "semester_id" form_class = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) @@ -696,11 +698,6 @@ def dispatch(self, *args, **kwargs): def get_form_kwargs(self): return super().get_form_kwargs() | {"form_kwargs": {"semester": self.semester}} - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["formset"] = context.pop("form") - return context - def form_valid(self, formset): include_not_enough_voters = self.request.POST.get("include_not_enough_voters") == "on" include_unpublished = self.request.POST.get("include_unpublished") == "on" @@ -2000,7 +1997,7 @@ def questionnaire_set_locked(request): @manager_required -class DegreeIndexView(SuccessMessageMixin, FormView): +class DegreeIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): model = Degree form_class = modelformset_factory( Degree, @@ -2013,18 +2010,9 @@ class DegreeIndexView(SuccessMessageMixin, FormView): success_url = reverse_lazy("staff:degree_index") success_message = gettext_lazy("Successfully updated the degrees.") - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["formset"] = context.pop("form") - return context - - def form_valid(self, form): - form.save() - return super().form_valid(form) - @manager_required -class CourseTypeIndexView(SuccessMessageMixin, FormView): +class CourseTypeIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): model = CourseType form_class = modelformset_factory( CourseType, @@ -2037,15 +2025,6 @@ class CourseTypeIndexView(SuccessMessageMixin, FormView): success_url = reverse_lazy("staff:course_type_index") success_message = gettext_lazy("Successfully updated the course types.") - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["formset"] = context.pop("form") - return context - - def form_valid(self, form): - form.save() - return super().form_valid(form) - @manager_required def course_type_merge_selection(request): @@ -2399,7 +2378,7 @@ def template_edit(request, template_id): @manager_required -class FaqIndexView(SuccessMessageMixin, FormView): +class FaqIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): model = FaqSection form_class = modelformset_factory(FaqSection, form=FaqSectionForm, can_delete=True, extra=1) template_name = "staff_faq_index.html" @@ -2407,14 +2386,7 @@ class FaqIndexView(SuccessMessageMixin, FormView): success_message = gettext_lazy("Successfully updated the FAQ sections.") def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["formset"] = context.pop("form") - context["sections"] = FaqSection.objects.all() - return context - - def form_valid(self, form): - form.save() - return super().form_valid(form) + return super().get_context_data(**kwargs) | {"sections": FaqSection.objects.all()} @manager_required @@ -2437,21 +2409,12 @@ def faq_section(request, section_id): @manager_required -class InfotextsView(SuccessMessageMixin, FormView): +class InfotextsView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): form_class = modelformset_factory(Infotext, form=InfotextForm, edit_only=True, extra=0) template_name = "staff_infotexts.html" success_url = reverse_lazy("staff:infotexts") success_message = gettext_lazy("Successfully updated the infotext entries.") - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["formset"] = context.pop("form") - return context - - def form_valid(self, form): - form.save() - return super().form_valid(form) - @manager_required def download_sample_file(_request, filename): From 7d5e744ef34ed8461bce2061bbbc814e99544f0f Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 10 Jul 2023 18:36:07 +0200 Subject: [PATCH 15/39] user merge selection --- evap/staff/urls.py | 2 +- evap/staff/views.py | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 385eb77424..6b684088cf 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -78,7 +78,7 @@ path("user/delete", views.user_delete, name="user_delete"), path("user/resend_email", views.user_resend_email, name="user_resend_email"), path("user/bulk_update", views.user_bulk_update, name="user_bulk_update"), - path("user/merge", views.user_merge_selection, name="user_merge_selection"), + path("user/merge", views.UserMergeSelectionView.as_view(), name="user_merge_selection"), path("user//merge/", views.user_merge, name="user_merge"), path("template/", RedirectView.as_view(url='/staff/', permanent=True)), diff --git a/evap/staff/views.py b/evap/staff/views.py index 7e87621bf4..416ed88fcf 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -2296,15 +2296,16 @@ def user_bulk_update(request): @manager_required -def user_merge_selection(request): - form = UserMergeSelectionForm(request.POST or None) +class UserMergeSelectionView(FormView): + form_class = UserMergeSelectionForm + template_name = "staff_user_merge_selection.html" - if form.is_valid(): - main_user = form.cleaned_data["main_user"] - other_user = form.cleaned_data["other_user"] - return redirect("staff:user_merge", main_user.id, other_user.id) - - return render(request, "staff_user_merge_selection.html", {"form": form}) + def get_success_url(self): + return redirect( + "staff:user_merge", + self.form.cleaned_data["main_user"].id, + self.form.cleaned_data["other_user"].id, + ) @manager_required From 62411e5806745e684fe4e0b4bbb9b7ced9d05505 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 10 Jul 2023 18:56:04 +0200 Subject: [PATCH 16/39] template edit --- evap/staff/forms.py | 6 ---- evap/staff/urls.py | 2 +- evap/staff/views.py | 76 ++++++++++++++++++++++----------------------- 3 files changed, 39 insertions(+), 45 deletions(-) diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 6d2746e81f..33454971ff 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -1058,12 +1058,6 @@ class UserEditSelectionForm(forms.Form): user = UserModelChoiceField(UserProfile.objects.all()) -class EmailTemplateForm(forms.ModelForm): - class Meta: - model = EmailTemplate - fields = ("subject", "plain_content", "html_content") - - class FaqSectionForm(forms.ModelForm): class Meta: model = FaqSection diff --git a/evap/staff/urls.py b/evap/staff/urls.py index 6b684088cf..c4296a3f8f 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -82,7 +82,7 @@ path("user//merge/", views.user_merge, name="user_merge"), path("template/", RedirectView.as_view(url='/staff/', permanent=True)), - path("template/", views.template_edit, name="template_edit"), + path("template/", views.TemplateEditView.as_view(), name="template_edit"), path("text_answer_warnings/", views.text_answer_warnings_index, name="text_answer_warnings"), diff --git a/evap/staff/views.py b/evap/staff/views.py index 416ed88fcf..f2c913e996 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -76,7 +76,6 @@ CourseTypeForm, CourseTypeMergeSelectionForm, DegreeForm, - EmailTemplateForm, EvaluationCopyForm, EvaluationEmailForm, EvaluationForm, @@ -2336,46 +2335,47 @@ def user_merge(request, main_user_id, other_user_id): @manager_required -def template_edit(request, template_id): - template = get_object_or_404(EmailTemplate, id=template_id) - form = EmailTemplateForm(request.POST or None, request.FILES or None, instance=template) +class TemplateEditView(SuccessMessageMixin, UpdateView): + model = EmailTemplate + pk_url_kwarg = "template_id" + fields = ("subject", "plain_content", "html_content") + success_message = gettext_lazy("Successfully updated template.") + success_url = reverse_lazy("staff:index") + template_name = "staff_template_form.html" - if form.is_valid(): - form.save() - messages.success(request, _("Successfully updated template.")) - return redirect("staff:index") - - available_variables = [ - "contact_email", - "page_url", - "login_url", # only if they need it - "user", - ] + def get_context_data(self, **kwargs) -> dict: + context = super().get_context_data(**kwargs) + template = context["template"] = context.pop("emailtemplate") - if template.name == EmailTemplate.STUDENT_REMINDER: - available_variables += ["first_due_in_days", "due_evaluations"] - elif template.name in [ - EmailTemplate.EDITOR_REVIEW_NOTICE, - EmailTemplate.EDITOR_REVIEW_REMINDER, - EmailTemplate.PUBLISHING_NOTICE_CONTRIBUTOR, - EmailTemplate.PUBLISHING_NOTICE_PARTICIPANT, - ]: - available_variables += ["evaluations"] - elif template.name == EmailTemplate.TEXT_ANSWER_REVIEW_REMINDER: - available_variables += ["evaluation_url_tuples"] - elif template.name == EmailTemplate.EVALUATION_STARTED: - available_variables += ["evaluations", "due_evaluations"] - elif template.name == EmailTemplate.DIRECT_DELEGATION: - available_variables += ["evaluation", "delegate_user"] - - available_variables = ["{{ " + variable + " }}" for variable in available_variables] - available_variables.sort() + available_variables = [ + "contact_email", + "page_url", + "login_url", # only if they need it + "user", + ] - return render( - request, - "staff_template_form.html", - {"form": form, "template": template, "available_variables": available_variables}, - ) + if template.name == EmailTemplate.STUDENT_REMINDER: + available_variables += ["first_due_in_days", "due_evaluations"] + elif template.name in [ + EmailTemplate.EDITOR_REVIEW_NOTICE, + EmailTemplate.EDITOR_REVIEW_REMINDER, + EmailTemplate.PUBLISHING_NOTICE_CONTRIBUTOR, + EmailTemplate.PUBLISHING_NOTICE_PARTICIPANT, + ]: + available_variables += ["evaluations"] + elif template.name == EmailTemplate.TEXT_ANSWER_REVIEW_REMINDER: + available_variables += ["evaluation_url_tuples"] + elif template.name == EmailTemplate.EVALUATION_STARTED: + available_variables += ["evaluations", "due_evaluations"] + elif template.name == EmailTemplate.DIRECT_DELEGATION: + available_variables += ["evaluation", "delegate_user"] + + available_variables = ["{{ " + variable + " }}" for variable in available_variables] + available_variables.sort() + + context["available_variables"] = available_variables + + return context @manager_required From 8849da74a35fad823d7bc1d8b08399b63e327f7e Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 24 Jul 2023 19:53:22 +0200 Subject: [PATCH 17/39] Remove unused `editor_required` --- evap/evaluation/auth.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index 0121396577..92c24b2e16 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -127,15 +127,6 @@ def editor_or_delegate_required(user): return user.is_editor_or_delegate -@class_or_function_check_decorator -def editor_required(user): - """ - Decorator for views that checks that the user is logged in and has edit - right for at least one evaluation. - """ - return user.is_editor - - @class_or_function_check_decorator def participant_required(user): """ From 08b78283a7cfacfa82a2019f7a9c5812f26c7c80 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 24 Jul 2023 19:59:24 +0200 Subject: [PATCH 18/39] wrong way around --- evap/grades/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/grades/views.py b/evap/grades/views.py index b2ffa6dc13..9670e8973a 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -87,7 +87,7 @@ def get_context_data(self, **kwargs): return super().get_context_data(**kwargs) | { "semester": self.object.semester, "grade_documents": self.object.grade_documents.all(), - "disable_if_archived": "disabled", + "disable_if_archived": "", "disable_breadcrumb_course": True, } From 9c80711e246b4c9b1bd9ba0330484d5bd6430ae3 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 24 Jul 2023 20:04:20 +0200 Subject: [PATCH 19/39] use `self.object` in `SemesterExportView` --- evap/staff/views.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/evap/staff/views.py b/evap/staff/views.py index f2c913e996..4695fa6921 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -690,12 +690,14 @@ class SemesterExportView(SingleObjectMixin, FormsetView): form_class = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) template_name = "staff_semester_export.html" + object: Semester + def dispatch(self, *args, **kwargs): - self.object = self.semester = self.get_object() + self.object = self.get_object() return super().dispatch(*args, **kwargs) def get_form_kwargs(self): - return super().get_form_kwargs() | {"form_kwargs": {"semester": self.semester}} + return super().get_form_kwargs() | {"form_kwargs": {"semester": self.object}} def form_valid(self, formset): include_not_enough_voters = self.request.POST.get("include_not_enough_voters") == "on" @@ -703,12 +705,12 @@ def form_valid(self, formset): selection_list = [ (form.cleaned_data["selected_degrees"], form.cleaned_data["selected_course_types"]) for form in formset ] - filename = f"Evaluation-{self.semester.name}-{get_language()}.xls" + filename = f"Evaluation-{self.object.name}-{get_language()}.xls" response = AttachmentResponse(filename, content_type="application/vnd.ms-excel") ResultsExporter().export( response, - [self.semester], + [self.object], selection_list, include_not_enough_voters, include_unpublished, From 8e89e0c1e507cac9ba6825ca963769fb86d70614 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 31 Jul 2023 17:58:11 +0200 Subject: [PATCH 20/39] Add renaming functionality to `FormsetView` --- evap/evaluation/tools.py | 16 ++++++++++++++++ evap/staff/views.py | 16 ++++++++-------- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index 1678ec0594..c77b59fc23 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -140,6 +140,22 @@ def assert_not_none(value: T | None) -> T: class FormsetView(FormView): + @property + def form_class(self): + return self.formset_class + + def get_form_kwargs(self): + return self.get_formset_kwargs() + + def get_formset_kwargs(self): + return super().get_form_kwargs() + + def form_valid(self, formset): + return self.formset_valid(formset) + + def formset_valid(self, formset): + return super().form_valid(formset) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context["formset"] = context.pop("form") diff --git a/evap/staff/views.py b/evap/staff/views.py index 4695fa6921..bcc4c570aa 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -687,7 +687,7 @@ def semester_import(request, semester_id): class SemesterExportView(SingleObjectMixin, FormsetView): model = Semester pk_url_kwarg = "semester_id" - form_class = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) + formset_class = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) template_name = "staff_semester_export.html" object: Semester @@ -696,10 +696,10 @@ def dispatch(self, *args, **kwargs): self.object = self.get_object() return super().dispatch(*args, **kwargs) - def get_form_kwargs(self): - return super().get_form_kwargs() | {"form_kwargs": {"semester": self.object}} + def get_formset_kwargs(self): + return super().get_formset_kwargs() | {"form_kwargs": {"semester": self.object}} - def form_valid(self, formset): + def formset_valid(self, formset): include_not_enough_voters = self.request.POST.get("include_not_enough_voters") == "on" include_unpublished = self.request.POST.get("include_unpublished") == "on" selection_list = [ @@ -2000,7 +2000,7 @@ def questionnaire_set_locked(request): @manager_required class DegreeIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): model = Degree - form_class = modelformset_factory( + formset_class = modelformset_factory( Degree, form=DegreeForm, formset=ModelWithImportNamesFormset, @@ -2015,7 +2015,7 @@ class DegreeIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): @manager_required class CourseTypeIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): model = CourseType - form_class = modelformset_factory( + formset_class = modelformset_factory( CourseType, form=CourseTypeForm, formset=ModelWithImportNamesFormset, @@ -2383,7 +2383,7 @@ def get_context_data(self, **kwargs) -> dict: @manager_required class FaqIndexView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): model = FaqSection - form_class = modelformset_factory(FaqSection, form=FaqSectionForm, can_delete=True, extra=1) + formset_class = modelformset_factory(FaqSection, form=FaqSectionForm, can_delete=True, extra=1) template_name = "staff_faq_index.html" success_url = reverse_lazy("staff:faq_index") success_message = gettext_lazy("Successfully updated the FAQ sections.") @@ -2413,7 +2413,7 @@ def faq_section(request, section_id): @manager_required class InfotextsView(SuccessMessageMixin, SaveValidFormMixin, FormsetView): - form_class = modelformset_factory(Infotext, form=InfotextForm, edit_only=True, extra=0) + formset_class = modelformset_factory(Infotext, form=InfotextForm, edit_only=True, extra=0) template_name = "staff_infotexts.html" success_url = reverse_lazy("staff:infotexts") success_message = gettext_lazy("Successfully updated the infotext entries.") From 69028e6e61f43df3733cfc1a2537da6d90737e29 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 31 Jul 2023 19:23:19 +0200 Subject: [PATCH 21/39] Check that forms redirect --- evap/staff/tests/test_views.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/evap/staff/tests/test_views.py b/evap/staff/tests/test_views.py index 9c9aed8522..e762e8d6a4 100644 --- a/evap/staff/tests/test_views.py +++ b/evap/staff/tests/test_views.py @@ -1948,16 +1948,19 @@ def test_edit_course(self): @patch("evap.staff.views.reverse") def test_operation_redirects(self, mock_reverse): - mock_reverse.return_value = "very_legit_url" + mock_reverse.return_value = "/very_legit_url" - self.prepare_form("a").submit("operation", value="save") + response = self.prepare_form("a").submit("operation", value="save") self.assertEqual(mock_reverse.call_args.args[0], "staff:semester_view") + self.assertRedirects(response, "/very_legit_url", fetch_redirect_response=False) - self.prepare_form("b").submit("operation", value="save_create_evaluation") + response = self.prepare_form("b").submit("operation", value="save_create_evaluation") self.assertEqual(mock_reverse.call_args.args[0], "staff:evaluation_create_for_course") + self.assertRedirects(response, "/very_legit_url", fetch_redirect_response=False) - self.prepare_form("c").submit("operation", value="save_create_single_result") + response = self.prepare_form("c").submit("operation", value="save_create_single_result") self.assertEqual(mock_reverse.call_args.args[0], "staff:single_result_create_for_course") + self.assertRedirects(response, "/very_legit_url", fetch_redirect_response=False) self.assertEqual(mock_reverse.call_count, 3) From e7abaf214fdec85326c08c196e21ffd0487aff5a Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 31 Jul 2023 19:30:42 +0200 Subject: [PATCH 22/39] Revert `UploadGradesView` --- evap/grades/urls.py | 2 +- evap/grades/views.py | 71 +++++++++++++++++++------------------------- 2 files changed, 31 insertions(+), 42 deletions(-) diff --git a/evap/grades/urls.py b/evap/grades/urls.py index 4e88c4507c..541d26e2cb 100644 --- a/evap/grades/urls.py +++ b/evap/grades/urls.py @@ -9,7 +9,7 @@ path("download/", views.download_grades, name="download_grades"), path("semester/", views.SemesterView.as_view(), name="semester_view"), path("course/", views.CourseView.as_view(), name="course_view"), - path("course//upload", views.UploadGradesView.as_view(), name="upload_grades"), + path("course//upload", views.upload_grades, name="upload_grades"), path("grade_document//edit", views.edit_grades, name="edit_grades"), path("delete_grades", views.delete_grades, name="delete_grades"), diff --git a/evap/grades/views.py b/evap/grades/views.py index 9670e8973a..608c87917f 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -106,54 +106,43 @@ def on_grading_process_finished(course): @grade_publisher_required -class UploadGradesView(UpdateView): - model = GradeDocument - form_class = GradeDocumentForm - template_name = "grades_upload_form.html" - - def dispatch(self, request, course_id): - self.course = get_object_or_404(Course, id=course_id) - if self.course.semester.grade_documents_are_deleted: - raise PermissionDenied - self.final_grades = request.GET.get("final") == "true" # if parameter is not given, assume midterm grades +def upload_grades(request, course_id): + course = get_object_or_404(Course, id=course_id) + semester = course.semester + if semester.grade_documents_are_deleted: + raise PermissionDenied - return super().dispatch(request) + final_grades = request.GET.get("final") == "true" # if parameter is not given, assume midterm grades - def get_object(self): - if self.final_grades: - type_specific_kwargs = { - "type": GradeDocument.Type.FINAL_GRADES, - "description_en": settings.DEFAULT_FINAL_GRADES_DESCRIPTION_EN, - "description_de": settings.DEFAULT_FINAL_GRADES_DESCRIPTION_DE, - } - else: - type_specific_kwargs = { - "type": GradeDocument.Type.MIDTERM_GRADES, - "description_en": settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_EN, - "description_de": settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_DE, - } + grade_document = GradeDocument(course=course) + if final_grades: + grade_document.type = GradeDocument.Type.FINAL_GRADES + grade_document.description_en = settings.DEFAULT_FINAL_GRADES_DESCRIPTION_EN + grade_document.description_de = settings.DEFAULT_FINAL_GRADES_DESCRIPTION_DE + else: + grade_document.type = GradeDocument.Type.MIDTERM_GRADES + grade_document.description_en = settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_EN + grade_document.description_de = settings.DEFAULT_MIDTERM_GRADES_DESCRIPTION_DE - return GradeDocument(course=self.course, **type_specific_kwargs) + form = GradeDocumentForm(request.POST or None, request.FILES or None, instance=grade_document) - def get_success_url(self): - return reverse("grades:course_view", args=[self.course.id]) + if form.is_valid(): + form.save(modifying_user=request.user) - def get_context_data(self, **kwargs): - return super().get_context_data(**kwargs) | { - "semester": self.course.semester, - "course": self.course, - "final_grades": self.final_grades, - "show_automated_publishing_info": self.final_grades, - } + if final_grades: + on_grading_process_finished(course) - def form_valid(self, form): - # Don't call super().form_valid() to avoid saving again - self.object = form.save(modifying_user=self.request.user) - if self.final_grades: - on_grading_process_finished(self.course) + messages.success(request, _("Successfully uploaded grades.")) + return redirect("grades:course_view", course.id) - messages.success(self.request, _("Successfully uploaded grades.")) - return HttpResponseRedirect(self.get_success_url()) + template_data = { + "semester": semester, + "course": course, + "form": form, + "final_grades": final_grades, + "show_automated_publishing_info": final_grades, + } + return render(request, "grades_upload_form.html", template_data) @require_POST From 6c73bace9da16146d57c71429aba57fba0546277 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 31 Jul 2023 19:31:02 +0200 Subject: [PATCH 23/39] Missed a rename --- evap/evaluation/tools.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index c77b59fc23..ecda5a97eb 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -150,8 +150,8 @@ def get_form_kwargs(self): def get_formset_kwargs(self): return super().get_form_kwargs() - def form_valid(self, formset): - return self.formset_valid(formset) + def form_valid(self, form): + return self.formset_valid(form) def formset_valid(self, formset): return super().form_valid(formset) From bf73430f0597fcf1d14ce6cae8fa951b68210755 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 31 Jul 2023 19:32:33 +0200 Subject: [PATCH 24/39] unused imports --- evap/grades/views.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/evap/grades/views.py b/evap/grades/views.py index 608c87917f..576cb61bbe 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -2,12 +2,11 @@ from django.contrib import messages from django.core.exceptions import PermissionDenied from django.db.models.query import QuerySet -from django.http import FileResponse, HttpResponse, HttpResponseRedirect +from django.http import FileResponse, HttpResponse from django.shortcuts import get_object_or_404, redirect, render -from django.urls import reverse from django.utils.translation import gettext as _ from django.views.decorators.http import require_GET, require_POST -from django.views.generic import DetailView, TemplateView, UpdateView +from django.views.generic import DetailView, TemplateView from evap.evaluation.auth import ( grade_downloader_required, From 560c598153db206945f2b9093555fe54b6733fe7 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 31 Jul 2023 20:39:23 +0200 Subject: [PATCH 25/39] Don't save form if operation is invalid --- evap/staff/views.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/evap/staff/views.py b/evap/staff/views.py index bcc4c570aa..81b6476b1d 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -1079,21 +1079,22 @@ def get_context_data(self, **kwargs): return context_data def form_valid(self, form): + if self.request.POST.get("operation") not in ("save", "save_create_evaluation", "save_create_single_result"): + raise SuspiciousOperation("Invalid POST operation") + response = super().form_valid(form) if form.has_changed(): update_template_cache_of_published_evaluations_in_course(self.object) return response def get_success_url(self): - match self.request.POST.get("operation"): + match self.request.POST["operation"]: case "save": return reverse("staff:semester_view", args=[self.object.semester.id]) case "save_create_evaluation": return reverse("staff:evaluation_create_for_course", args=[self.object.id]) case "save_create_single_result": return reverse("staff:single_result_create_for_course", args=[self.object.id]) - case _: - raise SuspiciousOperation("Invalid POST operation") @require_POST From afe079651493f3a417a175c083e6865ee610c015 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 21 Aug 2023 17:41:08 +0200 Subject: [PATCH 26/39] empty commit From 823e1dfacf567c00f66dbd76ec24315a2aadde35 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 28 Aug 2023 19:59:45 +0200 Subject: [PATCH 27/39] Add tests for `class_or_function_check_decorator` --- evap/evaluation/tests/test_auth.py | 60 +++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 2 deletions(-) diff --git a/evap/evaluation/tests/test_auth.py b/evap/evaluation/tests/test_auth.py index a3cf03d4cc..65916da4fe 100644 --- a/evap/evaluation/tests/test_auth.py +++ b/evap/evaluation/tests/test_auth.py @@ -4,11 +4,15 @@ from django.conf import settings from django.contrib.auth.models import Group from django.core import mail -from django.test import override_settings -from django.urls import reverse +from django.core.exceptions import PermissionDenied +from django.http import HttpRequest, HttpResponse +from django.test import TestCase, override_settings +from django.urls import path, reverse +from django.views import View from model_bakery import baker from evap.evaluation import auth +from evap.evaluation.auth import class_or_function_check_decorator from evap.evaluation.models import Contribution, Evaluation, UserProfile from evap.evaluation.tests.tools import WebTest @@ -176,3 +180,55 @@ def test_entering_staff_mode_after_logout_and_login(self): page = page.forms["enter-staff-mode-form"].submit().follow().follow() self.assertTrue("staff_mode_start_time" in self.app.session) self.assertContains(page, "Users") + + +class TestAuthDecorators(WebTest): + @classmethod + def setUpTestData(cls): + @class_or_function_check_decorator + def check_decorator(user: UserProfile) -> bool: + return user.some_condition() # mocked later + + @check_decorator + def function_based_view(_request): + return HttpResponse() + + @check_decorator + class ClassBasedView(View): + def get(self, _request): + return HttpResponse() + + cls.user = baker.make(UserProfile, email="testuser@institution.example.com") + cls.function_based_view = function_based_view + cls.class_based_view = ClassBasedView.as_view() + + @classmethod + def make_request(cls): + request = HttpRequest() + request.method = "GET" + request.user = cls.user + return request + + @patch("evap.evaluation.models.UserProfile.some_condition", return_value=True, create=True) + def test_passing_user_function_based(self, some_condition_mock): + response = self.function_based_view(self.make_request()) + some_condition_mock.assert_called_once() + self.assertEqual(response.status_code, 200) + + @patch("evap.evaluation.models.UserProfile.some_condition", return_value=True, create=True) + def test_passing_user_class_based(self, some_condition_mock): + response = self.class_based_view(self.make_request()) + some_condition_mock.assert_called_once() + self.assertEqual(response.status_code, 200) + + @patch("evap.evaluation.models.UserProfile.some_condition", return_value=False, create=True) + def test_failing_user_function_based(self, some_condition_mock): + with self.assertRaises(PermissionDenied): + self.function_based_view(self.make_request()) + some_condition_mock.assert_called_once() + + @patch("evap.evaluation.models.UserProfile.some_condition", return_value=False, create=True) + def test_failing_user_class_based(self, some_condition_mock): + with self.assertRaises(PermissionDenied): + self.class_based_view(self.make_request()) + some_condition_mock.assert_called_once() From 2ade9d5306c0c070d26e549704f8d224374a4538 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 28 Aug 2023 20:05:03 +0200 Subject: [PATCH 28/39] Make it more linter friendly --- evap/evaluation/tests/test_auth.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/evap/evaluation/tests/test_auth.py b/evap/evaluation/tests/test_auth.py index 65916da4fe..97b82dec16 100644 --- a/evap/evaluation/tests/test_auth.py +++ b/evap/evaluation/tests/test_auth.py @@ -187,7 +187,7 @@ class TestAuthDecorators(WebTest): def setUpTestData(cls): @class_or_function_check_decorator def check_decorator(user: UserProfile) -> bool: - return user.some_condition() # mocked later + return getattr(user, "some_condition") # mocked later @check_decorator def function_based_view(_request): @@ -209,26 +209,22 @@ def make_request(cls): request.user = cls.user return request - @patch("evap.evaluation.models.UserProfile.some_condition", return_value=True, create=True) - def test_passing_user_function_based(self, some_condition_mock): + @patch("evap.evaluation.models.UserProfile.some_condition", True, create=True) + def test_passing_user_function_based(self): response = self.function_based_view(self.make_request()) - some_condition_mock.assert_called_once() self.assertEqual(response.status_code, 200) - @patch("evap.evaluation.models.UserProfile.some_condition", return_value=True, create=True) - def test_passing_user_class_based(self, some_condition_mock): + @patch("evap.evaluation.models.UserProfile.some_condition", True, create=True) + def test_passing_user_class_based(self): response = self.class_based_view(self.make_request()) - some_condition_mock.assert_called_once() self.assertEqual(response.status_code, 200) - @patch("evap.evaluation.models.UserProfile.some_condition", return_value=False, create=True) - def test_failing_user_function_based(self, some_condition_mock): + @patch("evap.evaluation.models.UserProfile.some_condition", False, create=True) + def test_failing_user_function_based(self): with self.assertRaises(PermissionDenied): self.function_based_view(self.make_request()) - some_condition_mock.assert_called_once() - @patch("evap.evaluation.models.UserProfile.some_condition", return_value=False, create=True) - def test_failing_user_class_based(self, some_condition_mock): + @patch("evap.evaluation.models.UserProfile.some_condition", False, create=True) + def test_failing_user_class_based(self): with self.assertRaises(PermissionDenied): self.class_based_view(self.make_request()) - some_condition_mock.assert_called_once() From ebb8d3481f26feb3ac0e39eaf9049de33902a233 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 28 Aug 2023 20:16:19 +0200 Subject: [PATCH 29/39] Make linter more code friendly --- evap/evaluation/tests/test_auth.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/evap/evaluation/tests/test_auth.py b/evap/evaluation/tests/test_auth.py index 97b82dec16..7adb6cb061 100644 --- a/evap/evaluation/tests/test_auth.py +++ b/evap/evaluation/tests/test_auth.py @@ -6,8 +6,8 @@ from django.core import mail from django.core.exceptions import PermissionDenied from django.http import HttpRequest, HttpResponse -from django.test import TestCase, override_settings -from django.urls import path, reverse +from django.test import override_settings +from django.urls import reverse from django.views import View from model_bakery import baker @@ -211,7 +211,7 @@ def make_request(cls): @patch("evap.evaluation.models.UserProfile.some_condition", True, create=True) def test_passing_user_function_based(self): - response = self.function_based_view(self.make_request()) + response = self.function_based_view(self.make_request()) # pylint: disable=too-many-arguments self.assertEqual(response.status_code, 200) @patch("evap.evaluation.models.UserProfile.some_condition", True, create=True) @@ -222,7 +222,7 @@ def test_passing_user_class_based(self): @patch("evap.evaluation.models.UserProfile.some_condition", False, create=True) def test_failing_user_function_based(self): with self.assertRaises(PermissionDenied): - self.function_based_view(self.make_request()) + self.function_based_view(self.make_request()) # pylint: disable=too-many-arguments @patch("evap.evaluation.models.UserProfile.some_condition", False, create=True) def test_failing_user_class_based(self): From 2af3830c375b136f17fa9fc31b7e8252cd7df9c9 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 28 Aug 2023 20:18:39 +0200 Subject: [PATCH 30/39] yes --- evap/evaluation/tests/test_auth.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/evap/evaluation/tests/test_auth.py b/evap/evaluation/tests/test_auth.py index 7adb6cb061..899b6e4753 100644 --- a/evap/evaluation/tests/test_auth.py +++ b/evap/evaluation/tests/test_auth.py @@ -211,7 +211,7 @@ def make_request(cls): @patch("evap.evaluation.models.UserProfile.some_condition", True, create=True) def test_passing_user_function_based(self): - response = self.function_based_view(self.make_request()) # pylint: disable=too-many-arguments + response = self.function_based_view(self.make_request()) # pylint: disable=too-many-function-args self.assertEqual(response.status_code, 200) @patch("evap.evaluation.models.UserProfile.some_condition", True, create=True) @@ -222,7 +222,7 @@ def test_passing_user_class_based(self): @patch("evap.evaluation.models.UserProfile.some_condition", False, create=True) def test_failing_user_function_based(self): with self.assertRaises(PermissionDenied): - self.function_based_view(self.make_request()) # pylint: disable=too-many-arguments + self.function_based_view(self.make_request()) # pylint: disable=too-many-function-args @patch("evap.evaluation.models.UserProfile.some_condition", False, create=True) def test_failing_user_class_based(self): From 8fae9618dbcf00d8f4996968a9590a67cbacb50b Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 20:07:33 +0200 Subject: [PATCH 31/39] remove disable_if_archived stuff because we never render the disable case --- evap/grades/templates/grades_course_view.html | 6 +++--- evap/grades/templates/grades_semester_view.html | 4 ++-- evap/grades/views.py | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/evap/grades/templates/grades_course_view.html b/evap/grades/templates/grades_course_view.html index 7ab7b60fcb..1bf12162c8 100644 --- a/evap/grades/templates/grades_course_view.html +++ b/evap/grades/templates/grades_course_view.html @@ -30,7 +30,7 @@

{{ course.name }} ({{ semester.name }})

{% if user.is_grade_publisher %} - {% endif %} @@ -45,8 +45,8 @@

{{ course.name }} ({{ semester.name }})

{% if user.is_grade_publisher %} - {% trans 'Upload new midterm grades' %} - {% trans 'Upload new final grades' %} + {% trans 'Upload new midterm grades' %} + {% trans 'Upload new final grades' %} {% endif %} {% endblock %} diff --git a/evap/grades/templates/grades_semester_view.html b/evap/grades/templates/grades_semester_view.html index ae9d586115..0a026155f0 100644 --- a/evap/grades/templates/grades_semester_view.html +++ b/evap/grades/templates/grades_semester_view.html @@ -72,7 +72,7 @@

{% if not course.gets_no_grade_documents %} {% if num_final_grades == 0 %} -
@@ -85,7 +85,7 @@

{% else %} - + {% endif %} {% endif %} diff --git a/evap/grades/views.py b/evap/grades/views.py index 576cb61bbe..21f9f458ab 100644 --- a/evap/grades/views.py +++ b/evap/grades/views.py @@ -49,6 +49,8 @@ class SemesterView(DetailView): model = Semester pk_url_kwarg = "semester_id" + object: Semester + def get_object(self, *args, **kwargs): semester = super().get_object(*args, **kwargs) if semester.grade_documents_are_deleted: @@ -65,7 +67,6 @@ def get_context_data(self, **kwargs): return super().get_context_data(**kwargs) | { "courses": courses, - "disable_if_archived": "disabled", "disable_breadcrumb_semester": True, } @@ -86,7 +87,6 @@ def get_context_data(self, **kwargs): return super().get_context_data(**kwargs) | { "semester": self.object.semester, "grade_documents": self.object.grade_documents.all(), - "disable_if_archived": "", "disable_breadcrumb_course": True, } From 52f9b64605b773a630fe9594da85fdb676501ec0 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 20:24:04 +0200 Subject: [PATCH 32/39] add comment to class_or_function_check_decorator --- evap/evaluation/auth.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index 92c24b2e16..9c184517b5 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -49,6 +49,13 @@ def authenticate(self, request, email=None, password=None): # pylint: disable=a def class_or_function_check_decorator(test_func: Callable[[UserProfile], bool]): + """ + Transforms a test function into a decorator that can be used on function-based and class-based views. + + Using the returned decorator on a view enhances the view to return a "Permission Denied" response if the requesting + user does not pass the test function. + """ + def function_decorator(func): @wraps(func) def wrapped(request, *args, **kwargs): @@ -60,6 +67,7 @@ def wrapped(request, *args, **kwargs): def decorator(class_or_function): if inspect.isclass(class_or_function): + # See https://docs.djangoproject.com/en/4.2/topics/class-based-views/intro/#decorating-the-class return method_decorator(function_decorator, name="dispatch")(class_or_function) assert inspect.isfunction(class_or_function) From 6dfe4190996345d4c5bbe1a1c2bfc469d18846c3 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 20:28:08 +0200 Subject: [PATCH 33/39] revert port of `semester_export` --- evap/staff/views.py | 40 ++++++++++++++-------------------------- 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/evap/staff/views.py b/evap/staff/views.py index 81b6476b1d..d36f65d1f4 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -684,39 +684,27 @@ def semester_import(request, semester_id): @manager_required -class SemesterExportView(SingleObjectMixin, FormsetView): - model = Semester - pk_url_kwarg = "semester_id" - formset_class = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) - template_name = "staff_semester_export.html" - - object: Semester +def semester_export(request, semester_id): + semester = get_object_or_404(Semester, id=semester_id) - def dispatch(self, *args, **kwargs): - self.object = self.get_object() - return super().dispatch(*args, **kwargs) + ExportSheetFormset = formset_factory(form=ExportSheetForm, can_delete=True, extra=0, min_num=1, validate_min=True) + formset = ExportSheetFormset(request.POST or None, form_kwargs={"semester": semester}) - def get_formset_kwargs(self): - return super().get_formset_kwargs() | {"form_kwargs": {"semester": self.object}} + if formset.is_valid(): + include_not_enough_voters = request.POST.get("include_not_enough_voters") == "on" + include_unpublished = request.POST.get("include_unpublished") == "on" + selection_list = [] + for form in formset: + selection_list.append((form.cleaned_data["selected_degrees"], form.cleaned_data["selected_course_types"])) - def formset_valid(self, formset): - include_not_enough_voters = self.request.POST.get("include_not_enough_voters") == "on" - include_unpublished = self.request.POST.get("include_unpublished") == "on" - selection_list = [ - (form.cleaned_data["selected_degrees"], form.cleaned_data["selected_course_types"]) for form in formset - ] - filename = f"Evaluation-{self.object.name}-{get_language()}.xls" + filename = f"Evaluation-{semester.name}-{get_language()}.xls" response = AttachmentResponse(filename, content_type="application/vnd.ms-excel") - ResultsExporter().export( - response, - [self.object], - selection_list, - include_not_enough_voters, - include_unpublished, - ) + ResultsExporter().export(response, [semester], selection_list, include_not_enough_voters, include_unpublished) return response + return render(request, "staff_semester_export.html", {"semester": semester, "formset": formset}) + @manager_required def semester_raw_export(_request, semester_id): From f326956c487785e2383338d342748d8ed68959f2 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 20:36:47 +0200 Subject: [PATCH 34/39] document `FormsetView` --- evap/evaluation/tools.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index ecda5a97eb..afd575ca0e 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -140,10 +140,23 @@ def assert_not_none(value: T | None) -> T: class FormsetView(FormView): + """ + Just like `FormView`, but with a renaming from "form" to "formset". + """ + @property def form_class(self): return self.formset_class + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context["formset"] = context.pop("form") + return context + + # As an example for the logic, consider the following: Django calls `get_form_kwargs`, which we delegate to + # `get_formset_kwargs`. Users can thus override `get_formset_kwargs` instead. If it is not overridden, we delegate + # to the original `get_form_kwargs` instead. The same approach is used for the other renamed methods. + def get_form_kwargs(self): return self.get_formset_kwargs() @@ -156,11 +169,6 @@ def form_valid(self, form): def formset_valid(self, formset): return super().form_valid(formset) - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - context["formset"] = context.pop("form") - return context - class SaveValidFormMixin: def form_valid(self, form): From 140290a00fc4818343aba302f93ddc18e1edd380 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 20:44:26 +0200 Subject: [PATCH 35/39] document `SaveValidFormMixin` --- evap/evaluation/tools.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/evap/evaluation/tools.py b/evap/evaluation/tools.py index afd575ca0e..68bb8f702b 100644 --- a/evap/evaluation/tools.py +++ b/evap/evaluation/tools.py @@ -171,6 +171,13 @@ def formset_valid(self, formset): class SaveValidFormMixin: + """ + Call `form.save()` if the submitted form is valid. + + Django's `ModelFormMixin` (which inherits from `SingleObjectMixin`) does the same, but cannot always be used, for + example if a formset for a collection of objects is submitted. + """ + def form_valid(self, form): form.save() return super().form_valid(form) From 61cc4a7c49517c63e4314e547cf4634e7bf91c04 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 20:48:44 +0200 Subject: [PATCH 36/39] fix semester export view reference in urls --- evap/staff/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/staff/urls.py b/evap/staff/urls.py index c4296a3f8f..394d779cf4 100644 --- a/evap/staff/urls.py +++ b/evap/staff/urls.py @@ -15,7 +15,7 @@ path("semester/make_active", views.semester_make_active, name="semester_make_active"), path("semester/delete", views.semester_delete, name="semester_delete"), path("semester//import", views.semester_import, name="semester_import"), - path("semester//export", views.SemesterExportView.as_view(), name="semester_export"), + path("semester//export", views.semester_export, name="semester_export"), path("semester//raw_export", views.semester_raw_export, name="semester_raw_export"), path("semester//participation_export", views.semester_participation_export, name="semester_participation_export"), path("semester//vote_timestamps_export", views.vote_timestamps_export, name="vote_timestamps_export"), From 0e5cd94206dd8c709096b27af1da05fee8dc0446 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 20:52:15 +0200 Subject: [PATCH 37/39] remove comments on auth decorators --- evap/evaluation/auth.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/evap/evaluation/auth.py b/evap/evaluation/auth.py index 9c184517b5..b71a8dea46 100644 --- a/evap/evaluation/auth.py +++ b/evap/evaluation/auth.py @@ -103,53 +103,31 @@ def grade_publisher_required(user): @class_or_function_check_decorator def grade_publisher_or_manager_required(user): - """ - Decorator for views that checks that the user is logged in and a grade publisher or a manager - """ return user.is_grade_publisher or user.is_manager @class_or_function_check_decorator def grade_downloader_required(user): - """ - Decorator for views that checks that the user is logged in and can download grades - """ return user.can_download_grades @class_or_function_check_decorator def responsible_or_contributor_or_delegate_required(user): - """ - Decorator for views that checks that the user is logged in, is responsible for a course, or is a contributor, or is - a delegate. - """ return user.is_responsible_or_contributor_or_delegate @class_or_function_check_decorator def editor_or_delegate_required(user): - """ - Decorator for views that checks that the user is logged in, has edit rights - for at least one evaluation or is a delegate for such a person. - """ return user.is_editor_or_delegate @class_or_function_check_decorator def participant_required(user): - """ - Decorator for views that checks that the user is logged in and - participates in at least one evaluation. - """ return user.is_participant @class_or_function_check_decorator def reward_user_required(user): - """ - Decorator for views that checks that the user is logged in and can use - reward points. - """ return can_reward_points_be_used_by(user) From 230e5be96fbc31c90d19132a37f6ef4d7cf8f56e Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 2 Oct 2023 21:02:19 +0200 Subject: [PATCH 38/39] remove unused import --- evap/staff/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/evap/staff/views.py b/evap/staff/views.py index d36f65d1f4..8cd2a63f2a 100644 --- a/evap/staff/views.py +++ b/evap/staff/views.py @@ -25,7 +25,6 @@ from django.utils.translation import gettext_lazy, ngettext from django.views.decorators.http import require_POST from django.views.generic import CreateView, FormView, UpdateView -from django.views.generic.detail import SingleObjectMixin from django_stubs_ext import StrOrPromise from evap.contributor.views import export_contributor_results From 489c0c7dc660205e1d41197911c75b5f5e2fd1e0 Mon Sep 17 00:00:00 2001 From: Niklas Mohrin Date: Mon, 9 Oct 2023 20:11:15 +0200 Subject: [PATCH 39/39] test some uncovered lines --- evap/staff/tests/test_views.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/evap/staff/tests/test_views.py b/evap/staff/tests/test_views.py index e762e8d6a4..c0296fef46 100644 --- a/evap/staff/tests/test_views.py +++ b/evap/staff/tests/test_views.py @@ -1964,6 +1964,10 @@ def test_operation_redirects(self, mock_reverse): self.assertEqual(mock_reverse.call_count, 3) + @patch("evap.evaluation.models.Course.can_be_edited_by_manager", False) + def test_uneditable_course(self): + self.prepare_form(name_en="A different name").submit("operation", value="save", status=400) + class TestCourseDeleteView(DeleteViewTestMixin, WebTestStaffMode): url = reverse("staff:course_delete") @@ -3485,11 +3489,20 @@ def test_emailtemplate(self): self.assertEqual(self.template.plain_content, "plain_content: mflkd862xmnbo5") self.assertEqual(self.template.html_content, "html_content:

mflkd862xmnbo5

") - def test_review_reminder_template_tag(self): - review_reminder_template = EmailTemplate.objects.get(name=EmailTemplate.TEXT_ANSWER_REVIEW_REMINDER) - page = self.app.get(f"/staff/template/{review_reminder_template.pk}", user=self.manager, status=200) + def test_available_variables(self): + # We want to trigger all paths to ensure there are no syntax errors. + expected_variables = { + EmailTemplate.STUDENT_REMINDER: "first_due_in_days", + EmailTemplate.EDITOR_REVIEW_NOTICE: "evaluations", + EmailTemplate.TEXT_ANSWER_REVIEW_REMINDER: "evaluation_url_tuples", + EmailTemplate.EVALUATION_STARTED: "due_evaluations", + EmailTemplate.DIRECT_DELEGATION: "delegate_user", + } - self.assertContains(page, "evaluation_url_tuples") + for name, variable in expected_variables.items(): + template = EmailTemplate.objects.get(name=name) + page = self.app.get(f"/staff/template/{template.pk}", user=self.manager, status=200) + self.assertContains(page, variable) class TestTextAnswerWarningsView(WebTestStaffMode):