From 4964c534683657a6f77b2309cf5d3b25ba3e65c3 Mon Sep 17 00:00:00 2001
From: Hangzhi Yu <hangzhi@protonmail.com>
Date: Mon, 10 Aug 2020 08:56:48 +0200
Subject: [PATCH] Allow filtering of personal notes displayed in overview of
 last lesson and improve personal notes rules

---
 aleksis/apps/alsijil/preferences.py           |  8 ++
 aleksis/apps/alsijil/rules.py                 | 17 +++--
 .../alsijil/class_register/lesson.html        |  5 +-
 .../templates/alsijil/partials/absences.html  | 11 ++-
 .../alsijil/partials/tardinesses.html         |  6 +-
 aleksis/apps/alsijil/util/predicates.py       | 74 +++++++++++++++++--
 6 files changed, 102 insertions(+), 19 deletions(-)

diff --git a/aleksis/apps/alsijil/preferences.py b/aleksis/apps/alsijil/preferences.py
index e2a55de88..9fe3a6281 100644
--- a/aleksis/apps/alsijil/preferences.py
+++ b/aleksis/apps/alsijil/preferences.py
@@ -14,3 +14,11 @@ class BlockPersonalNotesForCancelled(BooleanPreference):
     name = "block_personal_notes_for_cancelled"
     default = True
     verbose_name = _("Block adding personal notes for cancelled lessons")
+
+
+@site_preferences_registry.register
+class ViewOwnPersonalNotes(BooleanPreference):
+    section = alsijil
+    name = "view_own_personal_notes"
+    default = True
+    verbose_name = _("Allow users to view their own personal notes")
diff --git a/aleksis/apps/alsijil/rules.py b/aleksis/apps/alsijil/rules.py
index 63db59277..f87447f42 100644
--- a/aleksis/apps/alsijil/rules.py
+++ b/aleksis/apps/alsijil/rules.py
@@ -11,12 +11,16 @@ from aleksis.core.util.predicates import (
 
 from .util.predicates import (
     has_lesson_group_object_perm,
+    has_personal_note_group_perm,
     has_person_group_object_perm,
     is_group_member,
     is_group_owner,
     is_lesson_parent_group_owner,
     is_lesson_participant,
     is_lesson_teacher,
+    is_own_personal_note,
+    is_personal_note_lesson_parent_group_owner,
+    is_personal_note_lesson_teacher,
     is_person_group_owner,
 )
 
@@ -33,14 +37,15 @@ add_perm("alsijil.view_lesson", view_lesson_predicate)
 # View lesson in menu
 add_perm("alsijil.view_lesson_menu", has_person)
 
-# View lesson personal notes
-view_lesson_personal_notes_predicate = has_person & (
+# View personal note
+view_personal_note_predicate = has_person & (
     has_global_perm("alsijil.view_personalnote")
-    | has_lesson_group_object_perm("core.view_personalnote_group")
-    | is_lesson_teacher
-    | is_lesson_parent_group_owner
+    | has_personal_note_group_perm("core.view_personalnote_group")
+    | is_personal_note_lesson_teacher
+    | is_own_personal_note
+    | is_personal_note_lesson_parent_group_owner
 )
-add_perm("alsijil.view_lesson_personalnote", view_lesson_personal_notes_predicate)
+add_perm("alsijil.view_personalnote", view_personal_note_predicate)
 
 # Edit lesson personal notes
 edit_lesson_personal_notes_predicate = has_person & (
diff --git a/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html b/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html
index 87941f967..130e697e1 100644
--- a/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html
+++ b/aleksis/apps/alsijil/templates/alsijil/class_register/lesson.html
@@ -131,7 +131,10 @@
                         <th>{{ extra_mark.name }}</th>
                         <td>
                           {% for note in notes %}
-                            <span>{{ note.person }}{% if not forloop.last %},{% endif %}</span>
+                            {% has_perm "alsijil.view_personalnote" user note as can_view_personalnote %}
+                            {% if can_view_personalnote %}
+                              <span>{{ note.person }}{% if not forloop.last %},{% endif %}</span>
+                            {% endif %}
                           {% endfor %}
                         </td>
                       </tr>
diff --git a/aleksis/apps/alsijil/templates/alsijil/partials/absences.html b/aleksis/apps/alsijil/templates/alsijil/partials/absences.html
index 6deaa3891..c72204c55 100644
--- a/aleksis/apps/alsijil/templates/alsijil/partials/absences.html
+++ b/aleksis/apps/alsijil/templates/alsijil/partials/absences.html
@@ -1,6 +1,9 @@
-{% load i18n %}
+{% load i18n rules %}
 {% for note in notes %}
-  <span class="{% if note.excused %}green-text{% else %}red-text{% endif %}">{{ note.person }}
-    {% if note.excused %}{% if note.excuse_type %}({{ note.excuse_type.short_name }}){% else %}{% trans "(e)" %}{% endif %}{% else %}{% trans "(u)" %}{% endif %}{% if not forloop.last %},{% endif %}
-  </span>
+  {% has_perm "alsijil.view_personalnote" user note as can_view_personalnote %}
+  {% if can_view_personalnote %}
+    <span class="{% if note.excused %}green-text{% else %}red-text{% endif %}">{{ note.person }}
+      {% if note.excused %}{% if note.excuse_type %}({{ note.excuse_type.short_name }}){% else %}{% trans "(e)" %}{% endif %}{% else %}{% trans "(u)" %}{% endif %}{% if not forloop.last %},{% endif %}
+    </span>
+  {% endif %}
 {% endfor %}
diff --git a/aleksis/apps/alsijil/templates/alsijil/partials/tardinesses.html b/aleksis/apps/alsijil/templates/alsijil/partials/tardinesses.html
index b3639ea88..3bb861916 100644
--- a/aleksis/apps/alsijil/templates/alsijil/partials/tardinesses.html
+++ b/aleksis/apps/alsijil/templates/alsijil/partials/tardinesses.html
@@ -1,3 +1,7 @@
+{% load rules %}
 {% for note in notes %}
-  <span>{{ note.person }} ({{ note.late }}'){% if not forloop.last %},{% endif %}</span>
+  {% has_perm "alsijil.view_personalnote" user note as can_view_personalnote %}
+  {% if can_view_personalnote %}
+    <span>{{ note.person }} ({{ note.late }}'){% if not forloop.last %},{% endif %}</span>
+  {% endif %}
 {% endfor %}
diff --git a/aleksis/apps/alsijil/util/predicates.py b/aleksis/apps/alsijil/util/predicates.py
index 9fe6949e4..bad3a644c 100644
--- a/aleksis/apps/alsijil/util/predicates.py
+++ b/aleksis/apps/alsijil/util/predicates.py
@@ -6,7 +6,9 @@ from rules import predicate
 
 from aleksis.apps.chronos.models import LessonPeriod
 from aleksis.core.models import Group, Person
-from aleksis.core.util.predicates import check_object_permission
+from aleksis.core.util.predicates import check_object_permission, get_site_preferences
+
+from ..models import PersonalNote
 
 
 @predicate
@@ -16,12 +18,7 @@ def is_lesson_teacher(user: User, obj: LessonPeriod) -> bool:
     Checks whether the person linked to the user is a teacher
     in the lesson or the substitution linked to the given LessonPeriod.
     """
-    if hasattr(obj, "lesson"):
-        return (
-            user.person in obj.lesson.teachers.all()
-            or user.person in Person.objects.filter(lesson_substitutions__lesson_period=obj, lesson_substitutions__week=obj._week)
-        )
-    return True
+    return user.person in Person.objects.filter(lesson_substitutions=obj.get_substitution())
 
 
 @predicate
@@ -122,3 +119,66 @@ def has_lesson_group_object_perm(perm: str):
         return True
 
     return fn
+
+
+def has_personal_note_group_perm(perm: str):
+    """Predicate builder for permissions on personal notes
+
+    Checks whether a user has a permission on any group of a person of a PersonalNote.
+    """
+    name = f"has_personal_note_person_or_group_perm:{perm}"
+
+    @predicate(name)
+    def fn(user: User, obj: PersonalNote) -> bool:
+        if hasattr(obj, "person"):
+            for group in obj.person.member_of.all():
+                if check_object_permission(user, perm, group):
+                    return True
+            return False
+
+    return fn
+
+
+@predicate
+def is_own_personal_note(user: User, obj: PersonalNote):
+    """Predicate for users referred to in a personal note
+
+    Checks whether the user referred to in a PersonalNote is the active user.
+    Is configurable via dynamic preferences.
+    """
+    if hasattr(obj, "person"):
+        if get_site_preferences()["alsijil__view_own_personal_notes"] and obj.person is user.person:
+            return True
+        return False
+
+
+@predicate
+def is_personal_note_lesson_teacher(user: User, obj: PersonalNote):
+    """Predicate for teachers of a lesson referred to in the lesson period of a personal note.
+
+    Checks whether the person linked to the user is a teacher
+    in the lesson or the substitution linked to the LessonPeriod of the given PersonalNote.
+    """
+    if hasattr(obj, "lesson_period"):
+        if hasattr(obj.lesson_period, "lesson"):
+            return (
+                user.person in obj.lesson_period.lesson.teachers.all()
+                or user.person in Person.objects.filter(lesson_substitutions=obj.lesson_period.get_substitution())
+            )
+        return False
+    return False
+
+
+@predicate
+def is_personal_note_lesson_parent_group_owner(user: User, obj: PersonalNote):
+    """
+    Predicate for parent group owners of a lesson referred to in the lesson period of a personal note.
+
+    Checks whether the person linked to the user is the owner of
+    any parent groups of any groups of the given LessonPeriod lesson of the given PersonalNote.
+    """
+    if hasattr(obj, "lesson_period"):
+        if hasattr(obj.lesson_period, "lesson"):
+            return obj.lesson_period.lesson.groups.filter(parent_groups__owners=user.person).exists()
+        return False
+    return False
-- 
GitLab