From 4f3adbcf4fc0ba6cf60ee8346379b62ff6fa340b Mon Sep 17 00:00:00 2001
From: Kevin Alberts <kevin@kevinalberts.nl>
Date: Mon, 16 Sep 2024 17:05:31 +0200
Subject: [PATCH] Refactor `is_rd` to `is_roomduty`, remove `require_room_duty`
 decorator in favor of directly using the `require_committee` decorator, make
 roomduty checks that use the RequireCommitteeMixin use the setting
 `ROOM_DUTY_ABBREVIATION`. Refactor `require_committee` decorator,
 `RequireCommitteeMixin` mixin and `RequireStrictCommitteeMixin` mixin to use
 the new Person.is_in_committee method.

---
 amelie/activities/templates/activity.html     |  2 +-
 .../activity_enrollment_overview.html         |  2 +-
 amelie/activities/views.py                    | 20 ++++++-------
 amelie/calendar/views.py                      |  3 +-
 amelie/members/ajax_views.py                  | 18 +++++------
 amelie/members/query_views.py                 |  4 +--
 amelie/members/templates/person_mandate.html  |  4 +--
 amelie/members/views.py                       | 30 +++++++++----------
 amelie/settings/environ.py                    |  4 +--
 amelie/settings/generic.py                    |  4 +--
 amelie/settings/local.py.default              |  2 --
 amelie/tools/decorators.py                    | 12 +++-----
 amelie/tools/mixins.py                        | 15 ++++++----
 amelie/views.py                               |  2 +-
 templates/frontpage.html                      |  2 +-
 15 files changed, 61 insertions(+), 63 deletions(-)

diff --git a/amelie/activities/templates/activity.html b/amelie/activities/templates/activity.html
index 5375335..f3de261 100644
--- a/amelie/activities/templates/activity.html
+++ b/amelie/activities/templates/activity.html
@@ -147,7 +147,7 @@ <h2 id="slider-carousel-title" class="h2-with-buttons">{% trans "Pictures of the
 	</div>
 
     {% if obj.enrollment %}
-        {% if obj.organizer in request.person.current_committees or request.is_board or is_rd %}
+        {% if obj.organizer in request.person.current_committees or request.is_board or is_roomduty %}
             <div class="col-xs-12 col-md-12">
                 <div class="current">
                     <h2 class="expand" data-target="export_options">{% trans 'Export enrollments' %}</h2>
diff --git a/amelie/activities/templates/activity_enrollment_overview.html b/amelie/activities/templates/activity_enrollment_overview.html
index bc0e10d..16afbed 100644
--- a/amelie/activities/templates/activity_enrollment_overview.html
+++ b/amelie/activities/templates/activity_enrollment_overview.html
@@ -103,7 +103,7 @@
                     {% for answer in participation.enrollmentoptionanswer_set.all %}
                         <td>{{ answer.display_answer }}</td>
                     {% endfor %}
-                    {% if request.is_board or is_rd %}
+                    {% if request.is_board or is_roomduty %}
                         <td>{% if participation.has_paid %}<p class="icon pos">{{ participation.get_payment_method_display }}{% else %}<p class="icon icon-cancel"><a title="{% trans "By clicking here, you confirm to have received the full amount of money." %}" href="{% url 'calendar:pay_participation' participation.pk %}">{% trans "Register cash payment" %}</a>{% endif %}</p></td>
                     {% else %}
                         <td>{{ participation.get_payment_method_display }}</td>
diff --git a/amelie/activities/views.py b/amelie/activities/views.py
index 366191d..f2b910f 100644
--- a/amelie/activities/views.py
+++ b/amelie/activities/views.py
@@ -51,7 +51,7 @@
 from amelie.members.models import Person, Photographer
 from amelie.members.query_forms import MailingForm
 from amelie.tools import amelie_messages, types
-from amelie.tools.decorators import require_actief, require_lid, require_committee, require_board, require_room_duty
+from amelie.tools.decorators import require_actief, require_lid, require_committee, require_board
 from amelie.tools.forms import PeriodForm, ExportForm, PeriodKeywordForm
 from amelie.tools.calendar import ical_calendar
 from amelie.tools.mixins import RequireActiveMemberMixin, DeleteMessageMixin, PassesTestMixin, RequireBoardMixin, \
@@ -252,7 +252,7 @@ def activity(request, pk, deanonymise=False):
 
     # Enable opengraph on this page
     metadata_enable_opengraph = True
-    is_rd = request.person.is_room_duty()
+    is_roomduty = request.person.is_room_duty()
 
     return render(request, "activity.html", locals())
 
@@ -493,7 +493,7 @@ def activity_editenrollment_self(request, pk):
         return render(request, "activity_enrollment_form.html", locals())
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 @transaction.atomic
 def activity_editenrollment_other(request, pk, person_id):
     """
@@ -522,7 +522,7 @@ def activity_editenrollment_other(request, pk, person_id):
         return render(request, "activity_enrollment_form.html", locals())
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 @transaction.atomic
 def activity_unenrollment(request, pk, person_id):
     """
@@ -569,8 +569,8 @@ def activity_enrollment_person_search(request, pk):
     Search for a person to enroll for this activity.
     """
     activity = get_object_or_404(Activity, pk=pk)
-    is_rd = request.person.is_room_duty()
-    if not (activity.can_edit(request.person) or is_rd):
+    is_roomduty = request.person.is_room_duty()
+    if not (activity.can_edit(request.person) or is_roomduty):
         raise PermissionDenied
 
     form = PersonSearchForm(request.POST if request.POST else None)
@@ -678,8 +678,8 @@ def activity_enrollment_form(request, activity, person=None):
         # Django messages have been set in check_enrollment_allowed
         return redirect(activity)
 
-    is_rd = request.person.is_room_duty()
-    if indirect and not (activity.can_edit(request.person) or is_rd):
+    is_roomduty = request.person.is_room_duty()
+    if indirect and not (activity.can_edit(request.person) or is_roomduty):
         raise PermissionDenied
 
     per_mandate = (request.is_board or settings.PERSONAL_TAB_COMMITTEE_CAN_AUTHORIZE) \
@@ -1309,9 +1309,9 @@ def test_requirement(self, request):
 
         is_board = hasattr(request, 'is_board') and request.is_board
         is_organization = obj.organizer in request.person.current_committees()
-        is_rd = request.person.is_room_duty()
+        is_roomduty = request.person.is_room_duty()
 
-        return is_board or is_organization or is_rd
+        return is_board or is_organization or is_roomduty
 
     def post(self, request, pk=None):
         if pk is None:
diff --git a/amelie/calendar/views.py b/amelie/calendar/views.py
index 963b15d..7ed6b49 100644
--- a/amelie/calendar/views.py
+++ b/amelie/calendar/views.py
@@ -1,3 +1,4 @@
+from django.conf import settings
 from django.contrib import messages
 from django.shortcuts import get_object_or_404, redirect
 from django.utils import timezone
@@ -10,7 +11,7 @@
 
 class PayParticipationView(RequireCommitteeMixin, TemplateView):
     template_name = "confirm_payment.html"
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
 
     def get_context_data(self, **kwargs):
         context = super(PayParticipationView, self).get_context_data()
diff --git a/amelie/members/ajax_views.py b/amelie/members/ajax_views.py
index f9e5d64..0edbe95 100644
--- a/amelie/members/ajax_views.py
+++ b/amelie/members/ajax_views.py
@@ -16,7 +16,7 @@
 from amelie.members.query_views import filter_member_list_public
 from amelie.personal_tab.models import Authorization
 from amelie.personal_tab.pos_views import require_cookie_corner_pos
-from amelie.tools.decorators import require_ajax, require_board, require_actief, require_room_duty
+from amelie.tools.decorators import require_ajax, require_board, require_actief, require_committee
 
 
 def person_data(request, obj, type, form_type, view_template=None, edit_template=None, *args, **kwargs):
@@ -113,7 +113,7 @@ def person_study_new(request, id):
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_payments(request, id, membership):
     obj = get_object_or_404(Person, id=id)
     membership = get_object_or_404(Membership, id=membership)
@@ -135,14 +135,14 @@ def person_payments(request, id, membership):
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_membership(request, id, option=None):
     obj = get_object_or_404(Person, id=id)
     return render(request, "person_membership.html", locals())
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_membership_new(request, id):
     obj = get_object_or_404(Person, id=id)
     if request.method == "POST":
@@ -161,7 +161,7 @@ def person_membership_new(request, id):
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_membership_end(request, id):
     obj = get_object_or_404(Person, id=id)
     if request.method == "POST":
@@ -178,14 +178,14 @@ def person_membership_end(request, id):
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_mandate(request, id):
     obj = get_object_or_404(Person, id=id)
     return render(request, "person_mandate.html", locals())
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_mandate_new(request, id):
     obj = get_object_or_404(Person, id=id)
     if request.method == "POST":
@@ -205,7 +205,7 @@ def person_mandate_new(request, id):
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_mandate_activate(request, id, mandate):
     obj = get_object_or_404(Person, id=id)
     mandate = get_object_or_404(Authorization, id=mandate, person=obj,
@@ -220,7 +220,7 @@ def person_mandate_activate(request, id, mandate):
 
 
 @require_ajax
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_mandate_end(request, id, mandate):
     obj = get_object_or_404(Person, id=id)
     mandate = get_object_or_404(Authorization, id=mandate)
diff --git a/amelie/members/query_views.py b/amelie/members/query_views.py
index d5e3a25..bb74ad4 100644
--- a/amelie/members/query_views.py
+++ b/amelie/members/query_views.py
@@ -24,13 +24,13 @@
 from amelie.members.query_forms import MailingForm, QueryForm, PushNotificationForm
 from amelie.members.tasks import send_push_notification
 from amelie.tools import types
-from amelie.tools.decorators import require_board, require_room_duty
+from amelie.tools.decorators import require_board, require_committee
 from amelie.tools.logic import current_association_year
 from amelie.tools.mixins import RequireBoardMixin
 from amelie.tools.paginator import RangedPaginator
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 @never_cache
 def query(request):
     page = types.get_int(request.GET, 'page', default=1, min_value=1)
diff --git a/amelie/members/templates/person_mandate.html b/amelie/members/templates/person_mandate.html
index 8383bdf..3c7ffef 100644
--- a/amelie/members/templates/person_mandate.html
+++ b/amelie/members/templates/person_mandate.html
@@ -22,7 +22,7 @@
                     <th>{% trans 'Signed' %}</th>
                     <th>{% trans 'Starts on' %}</th>
                     <th>{% trans 'Ends on' %}</th>
-                    {% if request.is_board or is_rd %}<th></th>{% endif %}
+                    {% if request.is_board or is_roomduty %}<th></th>{% endif %}
                 </tr>
             </thead>
             <tbody>
@@ -57,7 +57,7 @@
                         <td>
                             {{ mandate.end_date|default_if_none:"&mdash;" }}
                         </td>
-                        {%  if request.is_board or is_rd %}
+                        {%  if request.is_board or is_roomduty %}
                         <td>
                             {% if not mandate.is_signed and not mandate.end_date %}
                                 <a href="{% url 'members:mandate_form' mandate.id %}" class="icon icon-printer">{% trans 'Print' %}</a><br />
diff --git a/amelie/members/views.py b/amelie/members/views.py
index 50524de..4787e1f 100644
--- a/amelie/members/views.py
+++ b/amelie/members/views.py
@@ -45,7 +45,7 @@
 from amelie.personal_tab.forms import RFIDCardForm
 from amelie.personal_tab.models import Authorization, AuthorizationType, Transaction, SEPA_CHAR_VALIDATOR
 from amelie.tools.auth import get_oauth_link_code, send_oauth_link_code_email, get_user_info
-from amelie.tools.decorators import require_board, require_superuser, require_lid_or_oauth, require_room_duty
+from amelie.tools.decorators import require_board, require_superuser, require_lid_or_oauth, require_committee
 from amelie.tools.encodings import normalize_to_ascii
 from amelie.tools.http import HttpResponseSendfile, HttpJSONResponse
 from amelie.tools.logic import current_academic_year_with_holidays, current_association_year, association_year
@@ -301,7 +301,7 @@ def payment_statistics(request, start_year=2012):
     return render(request, 'statistics/payments.html', locals())
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_view(request, id, slug):
     obj = get_object_or_404(Person, id=id, slug=slug)
     preference_categories = PreferenceCategory.objects.all()
@@ -314,12 +314,12 @@ def person_view(request, id, slug):
         accounts = []
 
     can_be_anonymized, unable_to_anonymize_reasons = _person_can_be_anonymized(obj)
-    is_rd = request.person.is_room_duty()
+    is_roomduty = request.person.is_room_duty()
 
     return render(request, "person.html", locals())
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 @transaction.atomic
 def person_edit(request, id, slug):
     person = get_object_or_404(Person, id=id)
@@ -463,7 +463,7 @@ def person_anonymize(request, id, slug):
 
 
 class RegisterNewGeneralWizardView(RequireCommitteeMixin, SessionWizardView):
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
     template_name = "person_registration_form_general.html"
     form_list = [RegistrationFormPersonalDetails, RegistrationFormStepMemberContactDetails,
                  RegistrationFormStepGeneralStudyDetails, RegistrationFormStepGeneralMembershipDetails,
@@ -579,7 +579,7 @@ def done(self, form_list, **kwargs):
         return HttpResponse(pdf, content_type='application/pdf')
 
 class RegisterNewExternalWizardView(RequireCommitteeMixin, SessionWizardView):
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
     template_name = "person_registration_form_external.html"
     form_list = [RegistrationFormPersonalDetails, RegistrationFormStepMemberContactDetails,
                  RegistrationFormStepAuthorizationDetails, RegistrationFormStepPersonalPreferences,
@@ -690,7 +690,7 @@ def done(self, form_list, **kwargs):
 
 
 class RegisterNewEmployeeWizardView(RequireCommitteeMixin, SessionWizardView):
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
     template_name = "person_registration_form_employee.html"
     form_list = [RegistrationFormPersonalDetailsEmployee, RegistrationFormStepMemberContactDetails,
                  RegistrationFormStepEmployeeDetails, RegistrationFormStepEmployeeMembershipDetails,
@@ -798,7 +798,7 @@ def done(self, form_list, **kwargs):
 
 
 class RegisterNewFreshmanWizardView(RequireCommitteeMixin, SessionWizardView):
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
     template_name = "person_registration_form_freshmen.html"
     form_list = [RegistrationFormPersonalDetails, RegistrationFormStepMemberContactDetails,
                  RegistrationFormStepParentsContactDetails, RegistrationFormStepFreshmenStudyDetails,
@@ -1066,7 +1066,7 @@ class PreRegistrationCompleteView(TemplateView):
 
 class PreRegistrationStatus(RequireCommitteeMixin, TemplateView):
     template_name = "preregistration_status.html"
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
 
     def get_context_data(self, **kwargs):
         context = super(PreRegistrationStatus, self).get_context_data(**kwargs)
@@ -1180,7 +1180,7 @@ def get(self, request, *args, **kwargs):
 
 
 class PreRegistrationPrintDogroup(RequireCommitteeMixin, TemplateView):
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
 
     def get(self, request, *args, **kwargs):
         pre_enrollment_dogroup_id = request.GET.get('did', None)
@@ -1210,7 +1210,7 @@ def get(self, request, *args, **kwargs):
 class PreRegistrationPrintAll(RequireCommitteeMixin, FormView):
     template_name = "preregistration_print_all.html"
     form_class = PreRegistrationPrintAllForm
-    abbreviation = "RD"
+    abbreviation = settings.ROOM_DUTY_ABBREVIATION
 
     def form_valid(self, form):
         sort_by = form.cleaned_data['sort_by']
@@ -1257,7 +1257,7 @@ def form_valid(self, form):
 
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def registration_form(request, user, membership):
     from amelie.tools.pdf import pdf_enrollment_form
 
@@ -1269,7 +1269,7 @@ def registration_form(request, user, membership):
     return HttpResponse(pdf, content_type='application/pdf')
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def membership_form(request, user, membership):
     from amelie.tools.pdf import pdf_membership_form
 
@@ -1281,7 +1281,7 @@ def membership_form(request, user, membership):
     return HttpResponse(pdf, content_type='application/pdf')
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def mandate_form(request, mandate):
     from amelie.tools.pdf import pdf_authorization_form
 
@@ -1609,7 +1609,7 @@ def person_groupinfo(request):
     return HttpJSONResponse({})
 
 
-@require_room_duty()
+@require_committee(settings.ROOM_DUTY_ABBREVIATION)
 def person_send_link_code(request, person_id):
     person = get_object_or_404(Person, id=person_id)
     link_code = get_oauth_link_code(person)
diff --git a/amelie/settings/environ.py b/amelie/settings/environ.py
index 9b4cda7..5f56348 100644
--- a/amelie/settings/environ.py
+++ b/amelie/settings/environ.py
@@ -440,5 +440,5 @@ def get_random_secret_key_no_dollar():
 # Wo4you personal URL
 BOOK_SALES_URL = env("AMELIE_BOOK_SALES_URL", default=BOOK_SALES_URL)
 
-# Room duty
-ROOM_DUTY_ABBREVIATION = env("AMELIE_ROOM_DUTY_ABBREVIATION", default="RD")
+# Abbreviation of the room duty committee for access checks.
+ROOM_DUTY_ABBREVIATION = env("AMELIE_ROOM_DUTY_ABBREVIATION", default="RoomDuty")
diff --git a/amelie/settings/generic.py b/amelie/settings/generic.py
index c8b13f7..d039d99 100644
--- a/amelie/settings/generic.py
+++ b/amelie/settings/generic.py
@@ -923,5 +923,5 @@
 # Wo4you personal URL
 BOOK_SALES_URL = "https://wo4you.nl/"
 
-# Room duty abbreviation setting
-ROOM_DUTY_ABBREVIATION = "RD"
\ No newline at end of file
+# Abbreviation of the room duty committee for access checks.
+ROOM_DUTY_ABBREVIATION = "RoomDuty"
\ No newline at end of file
diff --git a/amelie/settings/local.py.default b/amelie/settings/local.py.default
index 99edae8..72198fa 100644
--- a/amelie/settings/local.py.default
+++ b/amelie/settings/local.py.default
@@ -100,5 +100,3 @@ if ENABLE_REQUEST_PROFILING:
     SILKY_AUTHORISATION = True  # User must be staff
     SILKY_MAX_REQUEST_BODY_SIZE = 0  # Silk takes anything <0 as no limit
     SILKY_MAX_RESPONSE_BODY_SIZE = 0  # If response body>0 bytes, ignore
-
-ROOM_DUTY_ABBREVIATION = "RD"
\ No newline at end of file
diff --git a/amelie/tools/decorators.py b/amelie/tools/decorators.py
index 0ec43c1..08f46be 100644
--- a/amelie/tools/decorators.py
+++ b/amelie/tools/decorators.py
@@ -83,13 +83,9 @@ def require_committee(abbreviation):
     """
 
     def _view(func):
-        return request_passes_test(lambda r: (hasattr(r, 'person') and
-                                              (r.is_board or r.person.function_set.filter(
-                                                  committee__abbreviation=abbreviation, end__isnull=True))),
-                                   needs_login=True,
-                                   reden=_l('Access for members of the committee only.'))(func)
+        return request_passes_test(
+            lambda r: (r.is_board or r.person.is_in_committee(abbreviation)),
+            needs_login=True, reden=_l('Access for members of the committee only.')
+        )(func)
 
     return _view
-
-def require_room_duty():
-    return require_committee(settings.ROOM_DUTY_ABBREVIATION)
diff --git a/amelie/tools/mixins.py b/amelie/tools/mixins.py
index 5452cff..570f899 100644
--- a/amelie/tools/mixins.py
+++ b/amelie/tools/mixins.py
@@ -104,9 +104,11 @@ class RequireCommitteeMixin(PassesTestMixin):
     """ Abbreviation of the committee required for accessing this page. """
 
     def test_requirement(self, request):
-        return (hasattr(request, 'person') and
-                (request.is_board or
-                 request.person.function_set.filter(committee__abbreviation=self.abbreviation, end__isnull=True)))
+        return (
+            hasattr(request, 'person') and
+            (request.is_board or request.person.is_in_committee(self.abbreviation))
+        )
+
 
 class RequireStrictCommitteeMixin(PassesTestMixin):
     """
@@ -118,9 +120,10 @@ class RequireStrictCommitteeMixin(PassesTestMixin):
     """ Abbreviation of the committee required for accessing this page. """
 
     def test_requirement(self, request):
-        return (hasattr(request, 'person') and
-                (request.user.is_superuser or
-                 request.person.function_set.filter(committee__abbreviation=self.abbreviation, end__isnull=True)))
+        return (
+            hasattr(request, 'person') and
+            (request.user.is_superuser or request.person.is_in_committee(self.abbreviation))
+        )
 
 
 class RequireCommitteeOrChildCommitteeMixin(PassesTestMixin):
diff --git a/amelie/views.py b/amelie/views.py
index b375337..4ffd27c 100644
--- a/amelie/views.py
+++ b/amelie/views.py
@@ -237,7 +237,7 @@ def frontpage(request):
                                                                  end__isnull=True).exists()
         
         # Room Duty check
-        context['is_rd'] = request.person.is_room_duty()
+        context['is_roomduty'] = request.person.is_room_duty()
 
         # Birthdays
         context['birthdays'] = Person.objects.members().filter(date_of_birth__day=date.today().day,
diff --git a/templates/frontpage.html b/templates/frontpage.html
index b13ed40..8bd0e74 100644
--- a/templates/frontpage.html
+++ b/templates/frontpage.html
@@ -116,7 +116,7 @@ <h2><a href="{% url 'videos:list_videos' %}">{% trans "Videos" %}</a></h2>
         {# End video things #}
 
         {# Board things #}
-        {% if user.is_authenticated and request.is_board or is_rd %}
+        {% if user.is_authenticated and request.is_board or is_roomduty %}
             <div class="ia">
                 <h2>{% trans 'Board tasks' %}</h2>
                 <ul class="no-bullets">