From 70f6631bee548426c4be0691702fa600e98cb3cb Mon Sep 17 00:00:00 2001
From: Chris van Run <chris.vanrun@radboudumc.nl>
Date: Fri, 24 Jan 2025 15:43:48 +0100
Subject: [PATCH] Address PR comments

---
 app/grandchallenge/challenges/admin.py        |  2 +-
 ...ask_onboardingtaskgroupobjectpermission.py |  4 ++--
 app/grandchallenge/challenges/models.py       | 20 ++++++++++++-------
 .../challenges_tests/test_permissions.py      |  6 +++---
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/app/grandchallenge/challenges/admin.py b/app/grandchallenge/challenges/admin.py
index 6ecfa5294..7221b3855 100644
--- a/app/grandchallenge/challenges/admin.py
+++ b/app/grandchallenge/challenges/admin.py
@@ -209,7 +209,7 @@ class OnboardingTaskAdmin(ModelAdmin):
         "on_time",
         "complete",
         "deadline",
-        "responsible",
+        "responsible_party",
     )
     list_filter = (
         OnTimeFilter,
diff --git a/app/grandchallenge/challenges/migrations/0048_onboardingtask_onboardingtaskgroupobjectpermission.py b/app/grandchallenge/challenges/migrations/0048_onboardingtask_onboardingtaskgroupobjectpermission.py
index 8e051887f..ac5a6c5e9 100644
--- a/app/grandchallenge/challenges/migrations/0048_onboardingtask_onboardingtaskgroupobjectpermission.py
+++ b/app/grandchallenge/challenges/migrations/0048_onboardingtask_onboardingtaskgroupobjectpermission.py
@@ -1,4 +1,4 @@
-# Generated by Django 4.2.17 on 2025-01-22 12:28
+# Generated by Django 4.2.17 on 2025-01-24 14:43
 
 import uuid
 
@@ -60,7 +60,7 @@ class Migration(migrations.Migration):
                     ),
                 ),
                 (
-                    "responsible",
+                    "responsible_party",
                     models.CharField(
                         choices=[
                             ("SUP", "Support"),
diff --git a/app/grandchallenge/challenges/models.py b/app/grandchallenge/challenges/models.py
index eca4123cc..36d13943b 100644
--- a/app/grandchallenge/challenges/models.py
+++ b/app/grandchallenge/challenges/models.py
@@ -1405,13 +1405,13 @@ class ChallengeRequestGroupObjectPermission(GroupObjectPermissionBase):
     )
 
 
-class TaskResponsibleChoices(models.TextChoices):
+class TaskResponsiblePartyChoices(models.TextChoices):
     SUPPORT = "SUP", "Support"
     CHALLENGE_ORGANIZERS = "ORG", "Challenge Organizers"
 
 
 class OnboardingTask(FieldChangeMixin, UUIDModel):
-    ResponsibleChoices = TaskResponsibleChoices
+    ResponsiblePartyChoices = TaskResponsiblePartyChoices
 
     class Meta:
         permissions = [
@@ -1441,11 +1441,11 @@ class Meta:
         blank=True,
         help_text="Time this task was last marked completed.",
     )
-    responsible = models.CharField(
+    responsible_party = models.CharField(
         blank=False,
         max_length=3,
-        choices=ResponsibleChoices.choices,
-        default=ResponsibleChoices.CHALLENGE_ORGANIZERS,
+        choices=ResponsiblePartyChoices.choices,
+        default=ResponsiblePartyChoices.CHALLENGE_ORGANIZERS,
         help_text="Who is responsible for completion of this task.",
     )
     deadline = models.DateTimeField(
@@ -1464,11 +1464,17 @@ def save(self, *args, **kwargs):
 
         super().save(*args, **kwargs)
 
-        if self.responsible == self.ResponsibleChoices.CHALLENGE_ORGANIZERS:
+        self.assign_permissions()
+
+    def assign_permissions(self):
+        if (
+            self.responsible_party
+            == self.ResponsiblePartyChoices.CHALLENGE_ORGANIZERS
+        ):
             assign_perm(
                 "complete_onboaringtask", self.challenge.admins_group, self
             )
-        elif not adding:
+        else:
             remove_perm(
                 "complete_onboaringtask", self.challenge.admins_group, self
             )
diff --git a/app/tests/challenges_tests/test_permissions.py b/app/tests/challenges_tests/test_permissions.py
index c6c69752f..4c1b0780b 100644
--- a/app/tests/challenges_tests/test_permissions.py
+++ b/app/tests/challenges_tests/test_permissions.py
@@ -127,8 +127,8 @@ def test_challenge_request_list_view_permissions(client, challenge_reviewer):
     "responsible,permitted",
     (
         (None, True),  # Default
-        (OnboardingTask.ResponsibleChoices.SUPPORT, False),
-        (OnboardingTask.ResponsibleChoices.CHALLENGE_ORGANIZERS, True),
+        (OnboardingTask.ResponsiblePartyChoices.SUPPORT, False),
+        (OnboardingTask.ResponsiblePartyChoices.CHALLENGE_ORGANIZERS, True),
     ),
 )
 def test_onboarding_task_completion_permissions(responsible, permitted):
@@ -137,7 +137,7 @@ def test_onboarding_task_completion_permissions(responsible, permitted):
 
     kwargs = {}
     if responsible:
-        kwargs["responsible"] = responsible
+        kwargs["responsible_party"] = responsible
 
     task = OnboardingTaskFactory(challenge=ch, **kwargs)