From 68aaee6474558180c56ee9c811063227569e269e Mon Sep 17 00:00:00 2001
From: Jesse Bickel <jbickel@opentechstrategies.com>
Date: Tue, 30 Jul 2024 11:14:11 -0500
Subject: [PATCH] Improve withdrawal permissions and error handling

The permissions on the `*withdraw` items should be `no_permissions`.
After a submission is withdrawn, nothing further should happen to it.

When `perform_transition` is called in the View, that function will
already check for valid transitions and raise an exception if needed.
So do not bother looking into the transitions, instead look directly
for the withdraw action. And expect exactly one of those, otherwise
raise an exception with details of expectations.

Issue #3296
---
 hypha/apply/funds/views.py    | 16 +++++++++++-----
 hypha/apply/funds/workflow.py | 13 ++++---------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/hypha/apply/funds/views.py b/hypha/apply/funds/views.py
index cddd859e34..d45c843112 100644
--- a/hypha/apply/funds/views.py
+++ b/hypha/apply/funds/views.py
@@ -12,7 +12,7 @@
     user_passes_test,
 )
 from django.contrib.auth.mixins import UserPassesTestMixin
-from django.core.exceptions import PermissionDenied
+from django.core.exceptions import ImproperlyConfigured, PermissionDenied
 from django.db.models import Count, Q
 from django.forms import BaseModelForm
 from django.http import (
@@ -1753,16 +1753,22 @@ def withdraw(self, request, *args, **kwargs):
         obj = self.get_object()
 
         withdraw_actions = [
-            action
-            for action in obj.workflow[obj.status].transitions.keys()
-            if "withdraw" in action
+            action for action in obj.workflow.keys() if "withdraw" in action
         ]
 
-        if len(withdraw_actions) > 0:
+        if len(withdraw_actions) == 1:
             action = withdraw_actions[0]
             obj.perform_transition(
                 action, self.request.user, request=self.request, notify=False
             )
+        elif len(withdraw_actions) > 1:
+            raise ImproperlyConfigured(
+                f'In workflow "{obj.workflow}" too many withdraw actions: "{withdraw_actions}"'
+            )
+        elif len(withdraw_actions) < 1:
+            raise ImproperlyConfigured(
+                f'No withdraw actions found in workflow "{obj.workflow}"'
+            )
 
         success_url = obj.get_absolute_url()
         return HttpResponseRedirect(success_url)
diff --git a/hypha/apply/funds/workflow.py b/hypha/apply/funds/workflow.py
index 852783ec26..43761e2599 100644
--- a/hypha/apply/funds/workflow.py
+++ b/hypha/apply/funds/workflow.py
@@ -391,7 +391,7 @@ def make_permissions(edit=None, review=None, view=None, withdraw=None):
         "withdrawn": {
             "display": _("Withdrawn"),
             "stage": Request,
-            "permissions": staff_edit_permissions,
+            "permissions": no_permissions,
         },
     },
 ]
@@ -443,11 +443,6 @@ def make_permissions(edit=None, review=None, view=None, withdraw=None):
             "stage": RequestExt,
             "permissions": applicant_edit_permissions,
         },
-        "ext_withdrawn": {
-            "display": _("Withdrawn"),
-            "stage": RequestExt,
-            "permissions": staff_edit_permissions,
-        },
     },
     {
         "ext_internal_review": {
@@ -585,7 +580,7 @@ def make_permissions(edit=None, review=None, view=None, withdraw=None):
         "ext_withdrawn": {
             "display": _("Withdrawn"),
             "stage": RequestExt,
-            "permissions": staff_edit_permissions,
+            "permissions": no_permissions,
         },
     },
 ]
@@ -803,7 +798,7 @@ def make_permissions(edit=None, review=None, view=None, withdraw=None):
         "com_withdrawn": {
             "display": _("Withdrawn"),
             "stage": RequestCom,
-            "permissions": staff_edit_permissions,
+            "permissions": no_permissions,
         },
     },
 ]
@@ -1140,7 +1135,7 @@ def make_permissions(edit=None, review=None, view=None, withdraw=None):
         "proposal_withdrawn": {
             "display": _("Withdrawn"),
             "stage": Proposal,
-            "permissions": staff_edit_permissions,
+            "permissions": no_permissions,
         },
     },
 ]