From 06b50a56812834cff8ef910ede0ba311eea4d7fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Benjamin=20H=C3=A4ttasch?=
 <benjamin.haettasch@fachschaft.informatik.tu-darmstadt.de>
Date: Thu, 5 Nov 2020 00:03:30 +0100
Subject: [PATCH] Implement first version for OWNER_TWO_SLOTS violation check
 caused by AK owner change

Improve constraint violation string representation to prevent endless recursion
Create violation check for changed owner of AK (bound to new m2m_changed signal)
Add doc comments
---
 AKModel/models.py      |  65 ++++++++++++++++++++++----
 AKScheduling/models.py | 104 +++++++++++++++++++++++++----------------
 2 files changed, 120 insertions(+), 49 deletions(-)

diff --git a/AKModel/models.py b/AKModel/models.py
index 972953a9..1cd08ac8 100644
--- a/AKModel/models.py
+++ b/AKModel/models.py
@@ -463,7 +463,7 @@ class ConstraintViolation(models.Model):
                                             help_text=_('Mark this violation manually as resolved'))
 
     fields = ['ak_owner', 'room', 'requirement', 'category']
-    fields_mm = ['aks', 'ak_slots']
+    fields_mm = ['_aks', '_ak_slots']
 
     def get_details(self):
         """
@@ -471,10 +471,10 @@ class ConstraintViolation(models.Model):
         :return: string of details
         :rtype: str
         """
-        output = []
-        # Stringify all ManyToMany fields
-        for field_mm in self.fields_mm:
-            output.append(f"{field_mm}: {', '.join(str(a) for a in getattr(self, field_mm).all())}")
+        # Stringify aks and ak slots fields (m2m)
+        output = [f"{_('AKs')}: {self._aks_str}",
+                  f"{_('AK Slots')}: {self._ak_slots_str}"]
+
         # Stringify all other fields
         for field in self.fields:
             a = getattr(self, field, None)
@@ -500,7 +500,6 @@ class ConstraintViolation(models.Model):
     def timestamp_display(self):
         return self.timestamp.astimezone(self.event.timezone).strftime('%d.%m.%y %H:%M')
 
-    # TODO Automatically save this
     aks_tmp = set()
 
     @property
@@ -518,9 +517,14 @@ class ConstraintViolation(models.Model):
             return set(self.aks.all())
         return self.aks_tmp
 
-    # TODO Automatically save this
     ak_slots_tmp = set()
 
+    @property
+    def _aks_str(self):
+        if self.pk and self.pk > 0:
+            return ', '.join(str(a) for a in self.aks.all())
+        return ', '.join(str(a) for a in self.aks_tmp)
+
     @property
     def _ak_slots(self):
         """
@@ -536,9 +540,50 @@ class ConstraintViolation(models.Model):
             return set(self.ak_slots.all())
         return self.ak_slots_tmp
 
+    @property
+    def _ak_slots_str(self):
+        if self.pk and self.pk > 0:
+            return ', '.join(str(a) for a in self.ak_slots.all())
+        return ', '.join(str(a) for a in self.ak_slots_tmp)
+
+    def save(self, *args, **kwargs):
+        super().save(*args, **kwargs)
+        # Store temporary m2m-relations in db
+        for ak in self.aks_tmp:
+            self.aks.add(ak)
+        for ak_slot in self.ak_slots_tmp:
+            self.ak_slots.add(ak_slot)
+
     def __str__(self):
         return f"{self.get_level_display()}: {self.get_type_display()} [{self.get_details()}]"
 
-    def __eq__(self, other):
-        # TODO Check if FIELDS and FIELDS_MM are equal
-        return super().__eq__(other)
+    def matches(self, other):
+        """
+        Check whether one constraint violation instance matches another,
+        this means has the same type, room, requirement, owner, category
+        as well as the same lists of aks and ak slots.
+        PK, timestamp, comments and manual resolving are ignored.
+
+        :param other: second instance to compare to
+        :type other: ConstraintViolation
+        :return: true if both instances are similar in the way described, false if not
+        :rtype: bool
+        """
+        if not isinstance(other, ConstraintViolation):
+            return False
+        # Check type
+        if self.type != other.type:
+            return False
+        # Make sure both have the same aks and ak slots
+        for field_mm in self.fields_mm:
+            s: set = getattr(self, field_mm)
+            o: set = getattr(other, field_mm)
+            if len(s) != len(o):
+                return False
+            if len(s.intersection(o)) != len(s):
+                return False
+        # Check other "defining" fields
+        for field in self.fields:
+            if getattr(self, field) != getattr(other, field):
+                return False
+        return True
diff --git a/AKScheduling/models.py b/AKScheduling/models.py
index 990bcd4a..100266fd 100644
--- a/AKScheduling/models.py
+++ b/AKScheduling/models.py
@@ -1,4 +1,4 @@
-from django.db.models.signals import post_save
+from django.db.models.signals import post_save, m2m_changed
 from django.dispatch import receiver
 
 from AKModel.availability.models import Availability
@@ -8,53 +8,79 @@ from AKModel.models import AK, AKSlot, Room, Event, AKOwner, ConstraintViolation
 @receiver(post_save, sender=AK)
 def ak_changed_handler(sender, instance: AK, **kwargs):
     # Changes might affect: Owner(s), Requirements, Conflicts, Prerequisites, Category, Interest
-    print(f"{instance} changed")
+    pass
+
+
+@receiver(m2m_changed, sender=AK.owners.through)
+def ak_changed_handler(sender, instance: AK, action: str, **kwargs):
+    """
+    Owners of AK changed
+    """
+    # Only signal after change (post_add, post_delete, post_clear) are relevant
+    if not action.startswith("post"):
+        return
+
+    # print(f"{instance} changed")
 
     event = instance.event
 
-    # Owner might have changed: Might affect multiple AKs by the same owner at the same time
-    conflicts = []
-    type = ConstraintViolation.ViolationType.OWNER_TWO_SLOTS
-    # For all owners...
+    # Owner(s) changed: Might affect multiple AKs by the same owner(s) at the same time
+    violation_type = ConstraintViolation.ViolationType.OWNER_TWO_SLOTS
+    new_violations = []
+
+    slots_of_this_ak: [AKSlot] = instance.akslot_set.filter(start__isnull=False)
+
+    # For all owners (after recent change)...
     for owner in instance.owners.all():
-        # ...find overlapping AKs...
-        slots_by_owner : [AKSlot] = []
-        slots_by_owner_this_ak : [AKSlot] = []
-        aks_by_owner = owner.ak_set.all()
-        for ak in aks_by_owner:
+        # ...find other slots that might be overlapping...
+
+        for ak in owner.ak_set.all():
+            # ...find overlapping slots...
             if ak != instance:
-                slots_by_owner.extend(ak.akslot_set.filter(start__isnull=False))
-            else:
-                # ToDo Fill this outside of loop?
-                slots_by_owner_this_ak.extend(ak.akslot_set.filter(start__isnull=False))
-        for slot in slots_by_owner_this_ak:
-            for other_slot in slots_by_owner:
-                if slot.overlaps(other_slot):
-                    # TODO Create ConstraintViolation here
-                    c = ConstraintViolation(
-                        type=type,
-                        level=ConstraintViolation.ViolationLevel.VIOLATION,
-                        event=event,
-                        ak_owner=owner
-                    )
-                    c.aks_tmp.add(instance)
-                    c.aks_tmp.add(other_slot.ak)
-                    c.ak_slots_tmp.add(slot)
-                    c.ak_slots_tmp.add(other_slot)
-                    conflicts.append(c)
-        print(f"{owner} has the following conflicts: {conflicts}")
-    # ... and compare to/update list of existing violations of this type:
-    current_violations = instance.constraintviolation_set.filter(type=type)
-    for conflict in conflicts:
-        pass
-        # TODO Remove from list of current_violations if an equal new one is found
-        # TODO Otherwise, store this conflict in db
-    # TODO Remove all violations still in current_violations
+                for slot in slots_of_this_ak:
+                    for other_slot in ak.akslot_set.filter(start__isnull=False):
+                        if slot.overlaps(other_slot):
+                            # ...and create a temporary violation if necessary...
+                            c = ConstraintViolation(
+                                type=violation_type,
+                                level=ConstraintViolation.ViolationLevel.VIOLATION,
+                                event=event,
+                                ak_owner=owner
+                            )
+                            c.aks_tmp.add(instance)
+                            c.aks_tmp.add(other_slot.ak)
+                            c.ak_slots_tmp.add(slot)
+                            c.ak_slots_tmp.add(other_slot)
+                            new_violations.append(c)
+
+        # print(f"{owner} has the following conflicts: {new_violations}")
+
+    # ... and compare to/update list of existing violations of this type
+    # belonging to the AK that was recently changed (important!)
+    existing_violations_to_check = list(instance.constraintviolation_set.filter(type=violation_type))
+    # print(existing_violations_to_check)
+
+    for new_violation in new_violations:
+        found_match = False
+        for existing_violation in existing_violations_to_check:
+            if existing_violation.matches(new_violation):
+                # Remove from existing violations set since it should stay in db
+                existing_violations_to_check.remove(existing_violation)
+                found_match = True
+                break
+
+        # Only save new violation if no match was found
+        if not found_match:
+            new_violation.save()
+
+    # Cleanup obsolete violations (ones without matches computed under current conditions)
+    for outdated_violation in existing_violations_to_check:
+        outdated_violation.delete()
 
 
 @receiver(post_save, sender=AKSlot)
 def akslot_changed_handler(sender, instance, **kwargs):
-    # Changes might affect: Duplicate parallel, Two in room
+    # Changes might affect: Duplicate parallel, Two in room, Resodeadline
     print(f"{sender} changed")
     # TODO Replace with real handling
 
-- 
GitLab