Merge pull request #558 from OpenTechFund/bugfix/status-updates-failing

Short circuit the save for the transition

opentech/apply/determinations/views.py

@@ -11,11 +11,12 @@ from django.views.generic import DetailView
 from opentech.apply.activity.models import Activity
 from opentech.apply.activity.messaging import messenger, MESSAGES
 from opentech.apply.funds.models import ApplicationSubmission
+from opentech.apply.funds.workflow import DETERMINATION_OUTCOMES
 from opentech.apply.utils.views import CreateOrUpdateView, ViewDispatcher
 from opentech.apply.users.decorators import staff_required
 
 from .forms import ConceptDeterminationForm, ProposalDeterminationForm
-from .models import Determination, DeterminationMessageSettings, NEEDS_MORE_INFO
+from .models import Determination, DeterminationMessageSettings, NEEDS_MORE_INFO, TRANSITION_DETERMINATION
 
 from .utils import can_create_determination, can_edit_determination, has_final_determination, transition_from_outcome
 
@@ -109,6 +110,28 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView):
 
         return HttpResponseRedirect(self.submission.get_absolute_url())
 
+    @classmethod
+    def should_redirect(cls, request, submission, action):
+        if has_final_determination(submission):
+            determination = submission.determinations.final().first()
+            if determination.outcome == TRANSITION_DETERMINATION[action]:
+                # We want to progress as normal so don't redirect through form
+                return False
+            else:
+                # Add a helpful message to prompt them to select the correct option
+                messages.warning(
+                    request,
+                    _('A determination of "{current}" exists but you tried to progress as "{target}"').format(
+                        current=determination.get_outcome_display(),
+                        target=action,
+                    )
+                )
+
+        if action in DETERMINATION_OUTCOMES:
+            return HttpResponseRedirect(reverse_lazy(
+                'apply:submissions:determinations:form',
+                args=(submission.id,)) + "?action=" + action)
+
 
 @method_decorator(staff_required, name='dispatch')
 class AdminDeterminationDetailView(DetailView):

opentech/apply/funds/models/mixins.py

@@ -38,21 +38,24 @@ class AccessFormData:
 
     @classmethod
     def stream_file(cls, file):
-        if 'path' in file:
-            file['filename'] = file['name']
-            file['name'] = file['path']
         if isinstance(file, StreamFieldFile):
             return file
         if isinstance(file, File):
             return StreamFieldFile(file, name=file.name, storage=submission_storage)
+
+        # This fixes a backwards compatibility issue with #507
+        # Once every application has been re-saved it should be possible to remove it
+        if 'path' in file:
+            file['filename'] = file['name']
+            file['name'] = file['path']
         return StreamFieldFile(None, name=file['name'], filename=file.get('filename'), storage=submission_storage)
 
     @classmethod
     def process_file(cls, file):
-        try:
-            return cls.stream_file(file)
-        except TypeError:
+        if isinstance(file, list):
             return [cls.stream_file(f) for f in file]
+        else:
+            return cls.stream_file(file)
 
     @classmethod
     def from_db(cls, db, field_names, values):

opentech/apply/funds/models/submissions.py

@@ -225,7 +225,7 @@ class AddTransitions(models.base.ModelBase):
                 raise PermissionDenied(f'You do not have permission to "{ action }"')
 
             transition(by=user, request=request, **kwargs)
-            self.save()
+            self.save(update_fields=['status'])
 
             self.progress_stage_when_possible(user, request)
 
@@ -474,7 +474,11 @@ class ApplicationSubmission(
                         f.save(folder)
                 self.form_data[field.id] = file
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields=list(), **kwargs):
+        if update_fields and 'form_data' not in update_fields:
+            # We don't want to use this approach if the user is sending data
+            return super().save(*args, update_fields=update_fields, **kwargs)
+
         if self.is_draft:
             raise ValueError('Cannot save with draft data')
 

opentech/apply/funds/tests/test_views.py

@@ -2,6 +2,7 @@ from datetime import datetime, timedelta
 import json
 
 from opentech.apply.activity.models import Activity
+from opentech.apply.determinations.tests.factories import DeterminationFactory
 from opentech.apply.funds.tests.factories import (
     ApplicationSubmissionFactory,
     ApplicationRevisionFactory,
@@ -100,6 +101,26 @@ class TestStaffSubmissionView(BaseSubmissionViewTestCase):
         self.assertEqual(submission.status, 'concept_review_discussion')
         self.assertIsNone(submission.next)
 
+    def test_not_redirected_if_determination_submitted(self):
+        submission = ApplicationSubmissionFactory(lead=self.user)
+        DeterminationFactory(submission=submission, rejected=True, submitted=True)
+
+        self.post_page(submission, {'form-submitted-progress_form': '', 'action': 'rejected'})
+
+        submission = self.refresh(submission)
+        self.assertEqual(submission.status, 'rejected')
+
+    def test_not_redirected_if_wrong_determination_selected(self):
+        submission = ApplicationSubmissionFactory(lead=self.user)
+        DeterminationFactory(submission=submission, accepted=True, submitted=True)
+
+        response = self.post_page(submission, {'form-submitted-progress_form': '', 'action': 'rejected'})
+        self.assertContains(response, 'you tried to progress')
+
+        submission = self.refresh(submission)
+        self.assertNotEqual(submission.status, 'accepted')
+        self.assertNotEqual(submission.status, 'rejected')
+
     def test_cant_access_edit_button_when_applicant_editing(self):
         submission = ApplicationSubmissionFactory(status='more_info')
         response = self.get_page(submission)

opentech/apply/funds/views.py

@@ -21,7 +21,7 @@ from opentech.apply.activity.views import (
     DelegatedViewMixin,
 )
 from opentech.apply.activity.messaging import messenger, MESSAGES
-from opentech.apply.funds.workflow import DETERMINATION_OUTCOMES
+from opentech.apply.determinations.views import DeterminationCreateOrUpdateView
 from opentech.apply.review.views import ReviewContextMixin
 from opentech.apply.users.decorators import staff_required
 from opentech.apply.utils.views import DelegateableView, ViewDispatcher
@@ -80,10 +80,9 @@ class ProgressSubmissionView(DelegatedViewMixin, UpdateView):
     def form_valid(self, form):
         action = form.cleaned_data.get('action')
         # Defer to the determination form for any of the determination transitions
-        if action in DETERMINATION_OUTCOMES:
-            return HttpResponseRedirect(reverse_lazy(
-                'apply:submissions:determinations:form',
-                args=(form.instance.id,)) + "?action=" + action)
+        redirect = DeterminationCreateOrUpdateView.should_redirect(self.request, self.object, action)
+        if redirect:
+            return redirect
 
         self.object.perform_transition(action, self.request.user, request=self.request)
         return super().form_valid(form)