Grant reward points on evaluation deletion

evap/rewards/models.py

@@ -36,7 +36,8 @@ class RewardPointGranting(models.Model):
     granting_time = models.DateTimeField(verbose_name=_("granting time"), auto_now_add=True)
     value = models.IntegerField(verbose_name=_("value"), validators=[MinValueValidator(1)])
 
-    granted_by_removal = Signal()
+    granted_by_participation_removal = Signal()
+    granted_by_evaluation_deletion = Signal()
 
 
 class RewardPointRedemption(models.Model):

evap/rewards/tests/test_tools.py

@@ -97,10 +97,12 @@ def setUpTestData(cls):
 
     def test_participant_removed_from_evaluation(self):
         self.evaluation.participants.remove(self.student)
-
         self.assertEqual(reward_points_of_user(self.student), 3)
 
     def test_evaluation_removed_from_participant(self):
         self.student.evaluations_participating_in.remove(self.evaluation)
+        self.assertEqual(reward_points_of_user(self.student), 3)
 
+    def test_evaluation_deleted(self):
+        self.evaluation.delete()
         self.assertEqual(reward_points_of_user(self.student), 3)

evap/rewards/tools.py

@@ -7,6 +7,7 @@
 from django.utils.translation import ngettext
 
 from evap.evaluation.models import Evaluation, Semester, UserProfile
+from evap.evaluation.tools import inside_transaction
 from evap.rewards.models import RewardPointGranting, RewardPointRedemption, SemesterActivation
 
 
@@ -100,21 +101,21 @@ def grant_reward_points_after_evaluate(request, semester, **_kwargs):
 
 
 @receiver(models.signals.m2m_changed, sender=Evaluation.participants.through)
-def grant_reward_points_after_delete(instance, action, reverse, pk_set, **_kwargs):
+def grant_reward_points_on_participation_change(instance, action, reverse, pk_set, **_kwargs):
     # if users do not need to evaluate anymore, they may have earned reward points
     if action == "post_remove":
         grantings = []
 
         if reverse:
-            # an evaluation got removed from a participant
+            # one or more evaluations got removed from a participant
             user = instance
 
             for semester in Semester.objects.filter(courses__evaluations__pk__in=pk_set):
                 granting, __ = grant_reward_points_if_eligible(user, semester)
                 if granting:
                     grantings = [granting]
         else:
-            # a participant got removed from an evaluation
+            # one or more participants got removed from an evaluation
             evaluation = instance
 
             for user in UserProfile.objects.filter(pk__in=pk_set):
@@ -123,4 +124,27 @@ def grant_reward_points_after_delete(instance, action, reverse, pk_set, **_kwarg
                     grantings.append(granting)
 
         if grantings:
-            RewardPointGranting.granted_by_removal.send(sender=RewardPointGranting, grantings=grantings)
+            RewardPointGranting.granted_by_participation_removal.send(sender=RewardPointGranting, grantings=grantings)
+
+
+@receiver(models.signals.pre_delete, sender=Evaluation)
+def grant_reward_points_on_evaluation_delete(instance, **_kwargs):
+    if not inside_transaction():
+        # This will always be true in a django TestCase, so our tests can't meaningfully catch calls that are not
+        # wrapped in a transaction. Requiring a transaction is a precaution so that an (unlikely) failing .delete()
+        # execution afterwards doesn't leave us in half-deleted state. Chances are, if deletion fails, staff users
+        # will still want to delete the instance.
+        # Currently, only staff:evaluation_delete and staff:semester_delete call .delete()
+        logger.exception("Called while not inside transaction")
+
+    grantings = []
+
+    participants = list(instance.participants.all())
+    instance.participants.clear()
+    for user in participants:
+        granting, __ = grant_reward_points_if_eligible(user, instance.course.semester)
+        if granting:
+            grantings.append(granting)
+
+    if grantings:
+        RewardPointGranting.granted_by_evaluation_deletion.send(sender=RewardPointGranting, grantings=grantings)

evap/staff/views.py

@@ -1313,7 +1313,7 @@ def helper_evaluation_edit(request, evaluation):
     # as the callback is captured by a weak reference in the Django Framework
     # and no other strong references are being kept.
     # See https://github.com/e-valuation/EvaP/issues/1361 for more information and discussion.
-    @receiver(RewardPointGranting.granted_by_removal, weak=True)
+    @receiver(RewardPointGranting.granted_by_participation_removal, weak=True)
     def notify_reward_points(grantings, **_kwargs):
         for granting in grantings:
             messages.info(
@@ -1418,6 +1418,7 @@ def helper_single_result_edit(request, evaluation):
 
 @require_POST
 @manager_required
+@transaction.atomic
 def evaluation_delete(request):
     evaluation = get_object_from_dict_pk_entry_or_logged_40x(Evaluation, request.POST, "evaluation_id")
 
@@ -2296,7 +2297,7 @@ def user_import(request):
 @manager_required
 def user_edit(request, user_id):
     # See comment in helper_evaluation_edit
-    @receiver(RewardPointGranting.granted_by_removal, weak=True)
+    @receiver(RewardPointGranting.granted_by_participation_removal, weak=True)
     def notify_reward_points(grantings, **_kwargs):
         assert len(grantings) == 1