Add submission withdrawal workflow #3298

frankduncan · 2023-03-23T10:35:56Z

What it says on the tin. When applicants have edit permissions (requires a change to workflow.py, see #3297), they can withdraw applications that have been submitted. This adds that as a new status for submissions.

frjo

Overall approach ok but implementation needs work.

hypha/apply/funds/templates/funds/applicationsubmission_detail.html

hypha/apply/funds/workflow.py

frjo · 2023-03-23T11:43:44Z

hypha/apply/funds/workflow.py

            },
            'display': _('Need screening'),
            'public': _('Application Received'),
            'stage': RequestExt,
-            'permissions': default_permissions,
+            'permissions': applicant_edit_permissions,


I think withdraw permissions should be separate from edit permissions.

We do not allow applicants to edit submissions that are in review for obvious reasons.

But if we allow an applicant to withdraw an application they should be able to do that in any state up until the submission is approved/declined.

In progress.

I think f4ae001 resolves this, let me know what you think.

hypha/public/utils/context_processors.py

frankduncan · 2023-03-31T12:15:00Z

Thanks for the feedback! Patches incoming

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions.

frankduncan · 2023-04-15T14:09:27Z

Ok! I believe I've made the correct updates. I did testing on all the different workflows, but I'm not as familiar with some of the ones we don't use with ARDC, so there may be edge cases I'm not aware of. Especially the Concept/Proposal, which I believe I did in line with the spirit of those workflows.

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions.

fourthletter · 2023-09-05T09:22:37Z

A 'withdraw' button has been added

bickelj · 2024-07-23T22:50:08Z

@frjo What would we need to do to get this one in? I was about to make an issue and PR but found this existing one.

frjo · 2024-07-24T06:39:55Z

@bickelj Rebase of current main and address the issues above. I think it makes perfect sense to allow applicants to withdraw an application. If this should be the default or optional I'm not sure but make it optional for now.

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions. Issue #3296

bickelj · 2024-07-25T19:49:06Z

@frjo I think this is ready for testing again.

frjo

Nice work! Some questions around implementation details.

Good improvement of Hypha.

hypha/apply/funds/workflow.py

hypha/apply/funds/templates/funds/applicationsubmission_detail.html

frjo · 2024-08-07T09:47:03Z

hypha/apply/funds/views.py

@@ -1579,6 +1583,45 @@ def form_valid(self, form):
        return super().form_valid(form)


+class SubmissionWithdrawView(SingleObjectTemplateResponseMixin, BaseDetailView):


Add

@method_decorator(login_required, name="dispatch")

Is the SingleObjectTemplateResponseMixin needed?

🤷 I suppose I could try without once the other issues are resolved.

hypha/apply/funds/views.py

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions. Issue #3296

frjo · 2024-08-07T19:12:04Z

@bickelj I rebased of main and added some permission fixes that seems to work. (I hope it doesn't mess it up for you.)

Please try it out and see how it behaves.

bickelj · 2024-08-08T15:09:04Z

@frjo Thank you for making the change, it didn't mess anything up for me! It looks great!

From a user perspective, applicants cannot withdraw right after submitting the application as expected. If I add withdraw=[applicant_can] to default_permissions, this problem goes away. I assume we need to add it to most of the others in order for applicants to withdraw from most states, right?

From a programmer perspective, the permissions in transitions seems to now be honored. Good, except the reason it's honored is it is explicitly looked up in a view. I hoped that the django-fsm library (or the Hypha extensions of django-fsm) would somehow honor the permissions declared in the workflow even when not explicitly referenced from the view. I mean I thought one of get_transition, can_proceed, or transition in the perform_transition method would cause a PermissionDenied but it does not. While I think that specifying permissions in the workflow itself is a more sustainable design, it was not clear to me that additional implementation in the view was a requirement. I still am not sure what "method" does there or where else the permissions object in the workflow is used.

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions. Issue #3296

frjo · 2024-08-08T15:27:10Z

I have added withdraw=[applicant_can] to default_permissions, makes total sense.

Unsure of adding it to other places. Can we put this on test and see what people think of it?

bickelj · 2024-08-08T16:12:55Z

@frjo Sure, yes, sorry for the delay!

bickelj · 2024-08-13T14:17:44Z

@frjo While looking at django-fsm docs I noticed django FSM was deprecated in favor of a new viewflow API earlier this year. It probably doesn't affect this PR but I wanted to note it somewhere before I forgot.

frjo · 2024-08-13T15:59:10Z

Created this #4090

bickelj · 2024-10-08T15:19:15Z

@frjo Do you think this one can be in the next release?

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions. Issue #3296

frjo · 2024-10-09T12:24:41Z

I just rebased the PR. It will go to test late this week or next week. So probably the release after the next one.

Issue #3296

This only affects about enabling them, not whether they are in the system. That means that if the configuration is changed over the lifetime of a system, things that were withdrawn when it was enabled retain that status. Issue #3296

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions. Issue #3296

This creates a single withdrawn status for each workflow, living at the end (next to accepted/rejected), and adds transistions for all the different workflow types. Issue #3296

Issue #3296

This affected different calculating pages, like the open round percentage completions. Issue #3296

Withdrawn status needs to be added to a mapping, apparently :) Issue #3296

Removed a redeclared variable and reformatted. Issue #3296

Add distinct withdrawal permissions within stages of workflows. By default, let the applicant withdraw at any stage. On the submission page, display the Withdraw button assuming the setting `ENABLE_SUBMISSION_WITHDRAWAL` is true and the withdrawal permissions are met. Issue #3296

Add a heroicon to "Withdraw" button and style it similar to "Delete". Issue #3296

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

bickelj · 2024-10-18T15:51:06Z

hypha/apply/funds/workflow.py

+                    "display": _("Withdraw"),
+                    "permissions": {UserPermissions.APPLICANT},
+                    "method": "withdraw",
+                },


The previous was more concise. What is the advantage of this repetition?

To give the applicant permission to perform the action.

I could have sworn I tested with applicants but I guess you mean in the context of putting the permissions in a single place rather than multiple places.

bickelj · 2024-10-18T19:55:35Z

hypha/apply/funds/workflow.py

@@ -204,7 +204,7 @@ def make_permissions(edit=None, review=None, view=None, withdraw=None):
            reviewer_can,
            partner_can,
        ],
-        "withdraw": withdraw or [applicant_can],


This is where I was thinking it avoided the repetition but is that incorrect? Perhaps it was the former 1753 in the view that was duplicative/incorrect before?

I meant to say "Line 1753 in the view."

frjo requested changes Mar 23, 2023

View reviewed changes

frankduncan force-pushed the enhancement/gh-3296-add-withdraw branch from 0861d24 to 6f8d05d Compare April 15, 2023 14:07

frjo force-pushed the enhancement/gh-3296-add-withdraw branch from 6f8d05d to 5d0405e Compare July 26, 2023 08:09

frjo approved these changes Jul 26, 2023

View reviewed changes

frjo force-pushed the enhancement/gh-3296-add-withdraw branch from 4fa0e41 to 1d7b829 Compare July 26, 2023 13:01

frjo added Status: Needs testing Tickets that need testing/qa Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter labels Jul 26, 2023

frjo removed the Status: Needs testing Tickets that need testing/qa label Sep 12, 2023

bickelj force-pushed the enhancement/gh-3296-add-withdraw branch 4 times, most recently from cacfc93 to f63f0b2 Compare July 25, 2024 18:10

frjo requested changes Jul 29, 2024

View reviewed changes

hypha/apply/funds/workflow.py Outdated Show resolved Hide resolved

hypha/apply/funds/workflow.py Outdated Show resolved Hide resolved

hypha/apply/funds/workflow.py Outdated Show resolved Hide resolved

hypha/apply/funds/templates/funds/applicationsubmission_detail.html Show resolved Hide resolved

frjo reviewed Aug 7, 2024

View reviewed changes

hypha/apply/funds/views.py Outdated Show resolved Hide resolved

frjo force-pushed the enhancement/gh-3296-add-withdraw branch from 5b656bb to a41caec Compare August 7, 2024 19:10

frjo force-pushed the enhancement/gh-3296-add-withdraw branch from a41caec to 2b72a80 Compare August 8, 2024 15:17

frjo approved these changes Aug 8, 2024

View reviewed changes

frjo requested a review from theskumar August 9, 2024 08:48

frjo force-pushed the enhancement/gh-3296-add-withdraw branch from afd1ef6 to 709f218 Compare October 9, 2024 12:23

chriszs and others added 14 commits October 18, 2024 13:26

feat: add submission withdrawal

a79d080

Issue #3296

fix: remove refs to dismiss while adding withdrawn

589e3c5

Issue #3296

Change permissions to withdraw to just being an applicant

34618bc

Based on Review in #3298, the decision was made that an applicant can withdraw at any time, without edit permissions, and edit permissions should not be to applicants for their own submissions. Issue #3296

Add withdraw to all workflows, move to end of workflow

9cf2caf

This creates a single withdrawn status for each workflow, living at the end (next to accepted/rejected), and adds transistions for all the different workflow types. Issue #3296

Move css from custom file.

ace6616

Issue #3296

Add "withdrawn" to list of inactive statuses

2b54958

This affected different calculating pages, like the open round percentage completions. Issue #3296

Fix error in all submissions table

f06e11c

Withdrawn status needs to be added to a mapping, apparently :) Issue #3296

Clean up from cherry-pick/rebase

e7598a9

Removed a redeclared variable and reformatted. Issue #3296

Make "Withdraw" button more consistent with others

04ec033

Add a heroicon to "Withdraw" button and style it similar to "Delete". Issue #3296

Permission fixes.

0228c5a

Applicant can withdraw in default_permissions.

42d5918

frjo force-pushed the enhancement/gh-3296-add-withdraw branch from 709f218 to 42d5918 Compare October 18, 2024 11:26

bickelj reviewed Oct 18, 2024

View reviewed changes

frjo added the Status: RTBC Internal Dev use only label Oct 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add submission withdrawal workflow #3298

Add submission withdrawal workflow #3298

frankduncan commented Mar 23, 2023

frjo left a comment

frjo Mar 23, 2023

bickelj Jul 25, 2024

bickelj Jul 25, 2024

frankduncan commented Mar 31, 2023

frankduncan commented Apr 15, 2023

fourthletter commented Sep 5, 2023 •

edited

Loading

bickelj commented Jul 23, 2024

frjo commented Jul 24, 2024

bickelj commented Jul 25, 2024

frjo left a comment

frjo Aug 7, 2024

frjo Aug 7, 2024

bickelj Aug 7, 2024

frjo commented Aug 7, 2024

bickelj commented Aug 8, 2024 •

edited

Loading

frjo commented Aug 8, 2024

bickelj commented Aug 8, 2024

bickelj commented Aug 13, 2024

frjo commented Aug 13, 2024 •

edited

Loading

bickelj commented Oct 8, 2024

frjo commented Oct 9, 2024

bickelj Oct 18, 2024 •

edited

Loading

frjo Oct 18, 2024

bickelj Oct 22, 2024

bickelj Oct 18, 2024

bickelj Oct 22, 2024

		@@ -1579,6 +1583,45 @@ def form_valid(self, form):
		return super().form_valid(form)


		class SubmissionWithdrawView(SingleObjectTemplateResponseMixin, BaseDetailView):

Add submission withdrawal workflow #3298

Are you sure you want to change the base?

Add submission withdrawal workflow #3298

Conversation

frankduncan commented Mar 23, 2023

frjo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frankduncan commented Mar 31, 2023

frankduncan commented Apr 15, 2023

fourthletter commented Sep 5, 2023 • edited Loading

bickelj commented Jul 23, 2024

frjo commented Jul 24, 2024

bickelj commented Jul 25, 2024

frjo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frjo commented Aug 7, 2024

bickelj commented Aug 8, 2024 • edited Loading

frjo commented Aug 8, 2024

bickelj commented Aug 8, 2024

bickelj commented Aug 13, 2024

frjo commented Aug 13, 2024 • edited Loading

bickelj commented Oct 8, 2024

frjo commented Oct 9, 2024

bickelj Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fourthletter commented Sep 5, 2023 •

edited

Loading

bickelj commented Aug 8, 2024 •

edited

Loading

frjo commented Aug 13, 2024 •

edited

Loading

bickelj Oct 18, 2024 •

edited

Loading