From e3d26b7df130de915c104aa79ecca49219e9bf99 Mon Sep 17 00:00:00 2001
From: Jonathan Weth <git@jonathanweth.de>
Date: Thu, 20 Aug 2020 20:07:17 +0200
Subject: [PATCH] Add permissions for students view and list

---
 aleksis/apps/alsijil/menus.py                 |  14 ++-
 aleksis/apps/alsijil/rules.py                 |  65 +++++++++-
 .../alsijil/class_register/lesson.html        |  10 +-
 .../alsijil/class_register/person.html        | 115 ++++++++++--------
 .../alsijil/class_register/week_view.html     |   7 +-
 aleksis/apps/alsijil/util/predicates.py       |  17 ++-
 aleksis/apps/alsijil/views.py                 |  97 +++++++++------
 7 files changed, 225 insertions(+), 100 deletions(-)

diff --git a/aleksis/apps/alsijil/menus.py b/aleksis/apps/alsijil/menus.py
index e405afae9..46ca6d023 100644
--- a/aleksis/apps/alsijil/menus.py
+++ b/aleksis/apps/alsijil/menus.py
@@ -38,13 +38,23 @@ MENUS = {
                     "name": _("My overview"),
                     "url": "overview_me",
                     "icon": "insert_chart",
-                    "validators": ["menu_generator.validators.is_authenticated"],
+                    "validators": [
+                        (
+                            "aleksis.core.util.predicates.permission_validator",
+                            "alsijil.view_person_overview_menu",
+                        ),
+                    ],
                 },
                 {
                     "name": _("My students"),
                     "url": "my_students",
                     "icon": "people",
-                    "validators": ["menu_generator.validators.is_authenticated"],
+                    "validators": [
+                        (
+                            "aleksis.core.util.predicates.permission_validator",
+                            "alsijil.view_my_students",
+                        ),
+                    ],
                 },
                 {
                     "name": _("Register absence"),
diff --git a/aleksis/apps/alsijil/rules.py b/aleksis/apps/alsijil/rules.py
index 08f185992..e25f74cc5 100644
--- a/aleksis/apps/alsijil/rules.py
+++ b/aleksis/apps/alsijil/rules.py
@@ -19,6 +19,7 @@ from .util.predicates import (
     is_lesson_teacher,
     is_own_personal_note,
     is_person_group_owner,
+    is_person_primary_group_owner,
     is_personal_note_lesson_parent_group_owner,
     is_personal_note_lesson_teacher,
     is_teacher,
@@ -46,6 +47,16 @@ view_lesson_personal_notes_predicate = has_person & (
 )
 add_perm("alsijil.view_lesson_personalnote", view_lesson_personal_notes_predicate)
 
+# Edit personal note
+edit_lesson_personal_note_predicate = has_person & (
+    has_global_perm("alsijil.change_personalnote")
+    | has_lesson_group_object_perm("core.edit_personalnote_group")
+    | is_lesson_teacher
+    | is_lesson_parent_group_owner
+)
+add_perm("alsijil.edit_lesson_personalnote", edit_lesson_personal_note_predicate)
+
+
 # View personal note
 view_personal_note_predicate = has_person & (
     has_global_perm("alsijil.view_personalnote")
@@ -58,9 +69,10 @@ add_perm("alsijil.view_personalnote", view_personal_note_predicate)
 
 # Edit personal note
 edit_personal_note_predicate = has_person & (
-    has_global_perm("alsijil.change_personalnote")
-    | has_lesson_group_object_perm("core.edit_personalnote_group")
-    | is_lesson_teacher
+    has_global_perm("alsijil.view_personalnote")
+    | has_personal_note_group_perm("core.edit_personalnote_group")
+    | is_personal_note_lesson_teacher
+    | is_personal_note_lesson_parent_group_owner
 )
 add_perm("alsijil.edit_personalnote", edit_personal_note_predicate)
 
@@ -127,6 +139,53 @@ view_full_register_predicate = has_person & (
 )
 add_perm("alsijil.view_full_register", view_full_register_predicate)
 
+# View students list
+view_my_students_predicate = has_person & is_teacher
+add_perm("alsijil.view_my_students", view_my_students_predicate)
+
+# View person overview
+view_person_overview_predicate = has_person & (
+    is_current_person | is_person_group_owner
+)
+add_perm("alsijil.view_person_overview", view_person_overview_predicate)
+
+# View person overview
+view_person_overview_menu_predicate = has_person
+add_perm("alsijil.view_person_overview_menu", view_person_overview_menu_predicate)
+
+# View person overview personal notes
+view_person_overview_personal_notes_predicate = has_person & (
+    has_global_perm("alsijil.view_personalnote")
+    | has_person_group_object_perm("core.view_personalnote_group")
+    | is_person_primary_group_owner
+)
+add_perm(
+    "alsijil.view_person_overview_personalnote",
+    view_person_overview_personal_notes_predicate,
+)
+
+# Edit person overview personal notes
+edit_person_overview_personal_notes_predicate = has_person & (
+    has_global_perm("alsijil.edit_personalnote")
+    | has_person_group_object_perm("core.edit_personalnote_group")
+    | is_person_primary_group_owner
+)
+add_perm(
+    "alsijil.edit_person_overview_personalnote",
+    edit_person_overview_personal_notes_predicate,
+)
+
+# View person statistics on personal notes
+view_person_statistics_personal_notes_predicate = has_person & (
+    has_global_perm("alsijil.view_personalnote")
+    | has_person_group_object_perm("core.view_personalnote_group")
+    | is_person_primary_group_owner
+)
+add_perm(
+    "alsijil.view_person_statistics_personalnote",
+    view_person_statistics_personal_notes_predicate,
+)
+
 # View excuse type list
 view_excusetypes_predicate = has_person & has_global_perm("alsijil.view_excusetype")
 add_perm("alsijil.view_excusetypes", view_excusetypes_predicate)
diff --git a/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html b/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html
index 130e697e1..f5a30a7ab 100644
--- a/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html
+++ b/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html
@@ -30,7 +30,7 @@
 {% block content %}
   {% has_perm "alsijil.view_lessondocumentation" user lesson_period as can_view_lesson_documentation %}
   {% has_perm "alsijil.edit_lessondocumentation" user lesson_period as can_edit_lesson_documentation %}
-  {% has_perm "alsijil.edit_personalnote" user lesson_period as can_edit_personalnote %}
+  {% has_perm "alsijil.edit_lesson_personalnote" user lesson_period as can_edit_lesson_personalnote %}
 
   <div class="row">
     <div class="col s12">
@@ -53,7 +53,7 @@
   </div>
 
   <form method="post">
-    {% if can_edit_lesson_documentation or can_edit_personalnote %}
+    {% if can_edit_lesson_documentation or can_edit_lesson_personalnote %}
       <p>{% include "core/partials/save_button.html" %}</p>
     {% endif %}
 
@@ -194,7 +194,7 @@
               <span class="card-title">
                 {% blocktrans %}Personal notes{% endblocktrans %}
               </span>
-              {% if can_edit_personalnote %}
+              {% if can_edit_lesson_personalnote %}
                 {% form form=personal_note_formset.management_form %}{% endform %}
               {% endif %}
 
@@ -212,7 +212,7 @@
                 </thead>
                 <tbody>
                 {% for form in personal_note_formset %}
-                  {% if can_edit_personalnote %}
+                  {% if can_edit_lesson_personalnote %}
                     <tr>
                       {{ form.id }}
                       <td>{{ form.person_name }}{{ form.person_name.value }}</td>
@@ -303,7 +303,7 @@
       {% endif %}
     </div>
 
-    {% if can_edit_lesson_documentation or can_edit_personalnote %}
+    {% if can_edit_lesson_documentation or can_edit_lesson_personalnote %}
       <p>{% include "core/partials/save_button.html" %}</p>
     {% endif %}
   </form>
diff --git a/aleksis/apps/alsijil/templates/alsijil/class_register/person.html b/aleksis/apps/alsijil/templates/alsijil/class_register/person.html
index 5a442b81c..e0547ce6a 100644
--- a/aleksis/apps/alsijil/templates/alsijil/class_register/person.html
+++ b/aleksis/apps/alsijil/templates/alsijil/class_register/person.html
@@ -1,5 +1,6 @@
 {# -*- engine:django -*- #}
 {% extends "core/base.html" %}
+{% load rules %}
 {% load data_helpers %}
 {% load week_helpers %}
 {% load i18n %}
@@ -14,6 +15,8 @@
 {% endblock %}
 
 {% block content %}
+  {% has_perm "alsijil.edit_person_overview_personalnote" user person as can_mark_all_as_excused %}
+
   <div class="row">
   <div class="col s12 m12 l6">
     <h5>{% trans "Unexcused absences" %}</h5>
@@ -35,12 +38,15 @@
           {% if note.remarks %}
             <p class="no-margin"><em>{{ note.remarks }}</em></p>
           {% endif %}
-          <form action="" method="post" class="hide-on-med-and-up">
-            {% csrf_token %}
-            {% trans "Mark as" %}
-            <input type="hidden" value="{{ note.pk }}" name="personal_note">
-            {% include "alsijil/partials/mark_as_buttons.html" %}
-          </form>
+          {% has_perm "alsijil.edit_personalnote" user note as can_edit_personal_note %}
+          {% if can_edit_personal_note %}
+            <form action="" method="post" class="hide-on-med-and-up">
+              {% csrf_token %}
+              {% trans "Mark as" %}
+              <input type="hidden" value="{{ note.pk }}" name="personal_note">
+              {% include "alsijil/partials/mark_as_buttons.html" %}
+            </form>
+          {% endif %}
         </li>
       {% empty %}
         <li class="collection-item flow-text">
@@ -48,47 +54,49 @@
         </li>
       {% endfor %}
     </ul>
-    <h5>{% trans "Statistics on absences, tardiness and remarks" %}</h5>
-    <ul class="collapsible">
-      {% for school_term, stat in stats %}
-        <li {% if forloop.first %}class="active"{% endif %}>
-          <div class="collapsible-header">
-            <i class="material-icons">date_range</i>{{ school_term }}</div>
-          <div class="collapsible-body">
-            <table>
-              <tr>
-                <th colspan="2">{% trans 'Absences' %}</th>
-                <td>{{ stat.absences_count }}</td>
-              </tr>
-              <tr>
-                <td rowspan="{{ excuse_types.count|add:2 }}" class="hide-on-small-only">{% trans "thereof" %}</td>
-                <td rowspan="{{ excuse_types.count|add:2 }}" class="hide-on-med-and-up"></td>
-                <th class="truncate">{% trans 'Excused' %}</th>
-                <td>{{ stat.excused }}</td>
-              </tr>
-              {% for excuse_type in excuse_types %}
-                <th>{{ excuse_type.name }}</th>
-                <td>{{ stat|get_dict:excuse_type.count_label }}</td>
-              {% endfor %}
-              <tr>
-                <th>{% trans 'Unexcused' %}</th>
-                <td>{{ stat.unexcused }}</td>
-              </tr>
-              <tr>
-                <th colspan="2">{% trans 'Tardiness' %}</th>
-                <td>{{ stat.tardiness }}'</td>
-              </tr>
-              {% for extra_mark in extra_marks %}
+    {% if stats %}
+      <h5>{% trans "Statistics on absences, tardiness and remarks" %}</h5>
+      <ul class="collapsible">
+        {% for school_term, stat in stats %}
+          <li {% if forloop.first %}class="active"{% endif %}>
+            <div class="collapsible-header">
+              <i class="material-icons">date_range</i>{{ school_term }}</div>
+            <div class="collapsible-body">
+              <table>
                 <tr>
-                  <th colspan="2">{{ extra_mark.name }}</th>
-                  <td>{{ stat|get_dict:extra_mark.count_label }}</td>
+                  <th colspan="2">{% trans 'Absences' %}</th>
+                  <td>{{ stat.absences_count }}</td>
                 </tr>
-              {% endfor %}
-            </table>
-          </div>
-        </li>
-      {% endfor %}
-    </ul>
+                <tr>
+                  <td rowspan="{{ excuse_types.count|add:2 }}" class="hide-on-small-only">{% trans "thereof" %}</td>
+                  <td rowspan="{{ excuse_types.count|add:2 }}" class="hide-on-med-and-up"></td>
+                  <th class="truncate">{% trans 'Excused' %}</th>
+                  <td>{{ stat.excused }}</td>
+                </tr>
+                {% for excuse_type in excuse_types %}
+                  <th>{{ excuse_type.name }}</th>
+                  <td>{{ stat|get_dict:excuse_type.count_label }}</td>
+                {% endfor %}
+                <tr>
+                  <th>{% trans 'Unexcused' %}</th>
+                  <td>{{ stat.unexcused }}</td>
+                </tr>
+                <tr>
+                  <th colspan="2">{% trans 'Tardiness' %}</th>
+                  <td>{{ stat.tardiness }}'</td>
+                </tr>
+                {% for extra_mark in extra_marks %}
+                  <tr>
+                    <th colspan="2">{{ extra_mark.name }}</th>
+                    <td>{{ stat|get_dict:extra_mark.count_label }}</td>
+                  </tr>
+                {% endfor %}
+              </table>
+            </div>
+          </li>
+        {% endfor %}
+      </ul>
+    {% endif %}
   </div>
   <div class="col s12 m12 l6">
     <h5>{% trans "Relevant personal notes" %}</h5>
@@ -113,12 +121,14 @@
               {% weekday_to_date note.calendar_week note.lesson_period.period.weekday as note_date %}
               {% ifchanged note_date %}
                 <li class="collection-item">
-                  <form action="" method="post" class="right hide-on-small-only" style="margin-top: -7px;">
-                    {% csrf_token %}
-                    {% trans "Mark all as" %}
-                    <input type="hidden" value="{{ note_date|date:"Y-m-d" }}" name="date">
-                    {% include "alsijil/partials/mark_as_buttons.html" %}
-                  </form>
+                  {% if can_mark_all_as_excused %}
+                    <form action="" method="post" class="right hide-on-small-only" style="margin-top: -7px;">
+                      {% csrf_token %}
+                      {% trans "Mark all as" %}
+                      <input type="hidden" value="{{ note_date|date:"Y-m-d" }}" name="date">
+                      {% include "alsijil/partials/mark_as_buttons.html" %}
+                    </form>
+                  {% endif %}
                   <i class="material-icons left">schedule</i>
                   {{ note_date }}
 
@@ -184,7 +194,8 @@
 
                   </div>
                   <div class="col s12 hide-on-med-and-up">
-                    {% if note.absent and not note.excused %}
+                    {% has_perm "alsijil.edit_personalnote" user note as can_edit_personal_note %}
+                    {% if note.absent and not note.excused and can_edit_personal_note %}
                       <form action="" method="post">
                         {% csrf_token %}
                         {% trans "Mark as" %}
diff --git a/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html b/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html
index eea58f784..de7c30ecd 100644
--- a/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html
+++ b/aleksis/apps/alsijil/templates/alsijil/class_register/week_view.html
@@ -116,7 +116,12 @@
             </span>
             {% for person in persons %}
               <h5 class="card-title">
-                <a href="{% url "overview_person" person.person.pk %}">{{ person.person.full_name }}</a>
+                {% has_perm "alsijil.view_person_overview" user person.person as can_view_person_overview %}
+                {% if can_view_person_overview %}
+                  <a href="{% url "overview_person" person.person.pk %}">{{ person.person.full_name }}</a>
+                {% else %}
+                  {{ person.person.full_name }}
+                {% endif %}
               </h5>
               <p class="card-text">
                 {% trans "Absent" %}: {{ person.person.absences_count }}
diff --git a/aleksis/apps/alsijil/util/predicates.py b/aleksis/apps/alsijil/util/predicates.py
index e047706a9..b33a9e27e 100644
--- a/aleksis/apps/alsijil/util/predicates.py
+++ b/aleksis/apps/alsijil/util/predicates.py
@@ -75,7 +75,22 @@ def is_person_group_owner(user: User, obj: Person) -> bool:
     Checks whether the person linked to the user is
     the owner of any group of the given person.
     """
-    return obj.member_of.filter(owners=user.person).exists()
+    if obj:
+        return obj.member_of.filter(owners=user.person).exists()
+    return False
+
+
+@predicate
+def is_person_primary_group_owner(user: User, obj: Person) -> bool:
+    """
+    Predicate for group owners of the person's primary group.
+
+    Checks whether the person linked to the user is
+    the owner of the primary group of the given person.
+    """
+    if obj.primary_group:
+        return user.person in obj.primary_group.owners.all()
+    return False
 
 
 def has_person_group_object_perm(perm: str):
diff --git a/aleksis/apps/alsijil/views.py b/aleksis/apps/alsijil/views.py
index f1644cc33..ce69a45be 100644
--- a/aleksis/apps/alsijil/views.py
+++ b/aleksis/apps/alsijil/views.py
@@ -129,7 +129,7 @@ def lesson(
             or not get_site_preferences()["alsijil__block_personal_notes_for_cancelled"]
         ):
             if personal_note_formset.is_valid() and request.user.has_perm(
-                "alsijil.edit_personalnote", lesson_period
+                "alsijil.edit_lesson_personalnote", lesson_period
             ):
                 instances = personal_note_formset.save()
 
@@ -470,6 +470,7 @@ def full_register_group(request: HttpRequest, id_: int) -> HttpResponse:
     return render(request, "alsijil/print/full_register.html", context)
 
 
+@permission_required("alsijil.view_my_students")
 def my_students(request: HttpRequest) -> HttpResponse:
     context = {}
     relevant_groups = (
@@ -482,6 +483,10 @@ def my_students(request: HttpRequest) -> HttpResponse:
     return render(request, "alsijil/class_register/persons.html", context)
 
 
+@permission_required(
+    "alsijil.view_person_overview",
+    fn=objectgetter_optional(Person, "request.user.person", True),
+)
 def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResponse:
     context = {}
     person = objectgetter_optional(
@@ -512,6 +517,11 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
                             request.POST["date"], "%Y-%m-%d"
                         ).date()
 
+                        if not request.user.has_perm(
+                            "alsijil.edit_person_overview_personalnote", person
+                        ):
+                            raise PermissionDenied()
+
                         notes = person.personal_notes.filter(
                             week=date.isocalendar()[1],
                             lesson_period__period__weekday=date.weekday(),
@@ -532,6 +542,8 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
                         note = PersonalNote.objects.get(
                             pk=int(request.POST["personal_note"])
                         )
+                        if not request.user.has_perm("alsijil.edit_personalnote", note):
+                            raise PermissionDenied()
                         if note.absent:
                             note.excused = True
                             note.excuse_type = excuse_type
@@ -544,10 +556,20 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
 
                 person.refresh_from_db()
 
-    unexcused_absences = person.personal_notes.filter(absent=True, excused=False)
+    allowed_personal_notes = person.personal_notes.all()
+
+    if not request.user.has_perm("alsijil.view_person_overview_personalnote", person):
+        print("has")
+        allowed_personal_notes = allowed_personal_notes.filter(
+            lesson_period__lesson__groups__owners=request.user.person
+        )
+
+    print(allowed_personal_notes)
+
+    unexcused_absences = allowed_personal_notes.filter(absent=True, excused=False)
     context["unexcused_absences"] = unexcused_absences
 
-    personal_notes = person.personal_notes.filter(
+    personal_notes = allowed_personal_notes.filter(
         Q(absent=True) | Q(late__gt=0) | ~Q(remarks="") | Q(extra_marks__isnull=False)
     ).order_by(
         "-lesson_period__lesson__validity__date_start",
@@ -558,48 +580,51 @@ def overview_person(request: HttpRequest, id_: Optional[int] = None) -> HttpResp
     context["personal_notes"] = personal_notes
     context["excuse_types"] = ExcuseType.objects.all()
 
-    school_terms = SchoolTerm.objects.all().order_by("-date_start")
-    stats = []
-    for school_term in school_terms:
-        stat = {}
-        personal_notes = PersonalNote.objects.filter(
-            person=person, lesson_period__lesson__validity__school_term=school_term
-        )
-
-        if not personal_notes.exists():
-            continue
-
-        stat.update(
-            personal_notes.filter(absent=True).aggregate(absences_count=Count("absent"))
-        )
-        stat.update(
-            personal_notes.filter(
-                absent=True, excused=True, excuse_type__isnull=True
-            ).aggregate(excused=Count("absent"))
-        )
-        stat.update(
-            personal_notes.filter(absent=True, excused=False).aggregate(
-                unexcused=Count("absent")
+    if request.user.has_perm("alsijil.view_person_statistics_personalnote", person):
+        school_terms = SchoolTerm.objects.all().order_by("-date_start")
+        stats = []
+        for school_term in school_terms:
+            stat = {}
+            personal_notes = PersonalNote.objects.filter(
+                person=person, lesson_period__lesson__validity__school_term=school_term
             )
-        )
-        stat.update(personal_notes.aggregate(tardiness=Sum("late")))
 
-        for extra_mark in ExtraMark.objects.all():
+            if not personal_notes.exists():
+                continue
+
             stat.update(
-                personal_notes.filter(extra_marks=extra_mark).aggregate(
-                    **{extra_mark.count_label: Count("pk")}
+                personal_notes.filter(absent=True).aggregate(
+                    absences_count=Count("absent")
                 )
             )
-
-        for excuse_type in ExcuseType.objects.all():
             stat.update(
-                personal_notes.filter(absent=True, excuse_type=excuse_type).aggregate(
-                    **{excuse_type.count_label: Count("absent")}
+                personal_notes.filter(
+                    absent=True, excused=True, excuse_type__isnull=True
+                ).aggregate(excused=Count("absent"))
+            )
+            stat.update(
+                personal_notes.filter(absent=True, excused=False).aggregate(
+                    unexcused=Count("absent")
                 )
             )
+            stat.update(personal_notes.aggregate(tardiness=Sum("late")))
+
+            for extra_mark in ExtraMark.objects.all():
+                stat.update(
+                    personal_notes.filter(extra_marks=extra_mark).aggregate(
+                        **{extra_mark.count_label: Count("pk")}
+                    )
+                )
+
+            for excuse_type in ExcuseType.objects.all():
+                stat.update(
+                    personal_notes.filter(
+                        absent=True, excuse_type=excuse_type
+                    ).aggregate(**{excuse_type.count_label: Count("absent")})
+                )
 
-        stats.append((school_term, stat))
-    context["stats"] = stats
+            stats.append((school_term, stat))
+        context["stats"] = stats
     context["excuse_types"] = ExcuseType.objects.all()
     context["extra_marks"] = ExtraMark.objects.all()
     return render(request, "alsijil/class_register/person.html", context)
-- 
GitLab