Allow changing status of closed patches in last CF

Without this, once a patch was closed (rejected/committed etc),
it was no longer possible to undo that and reopen it.

We now allow this to be done in the very last commitfest a patch
is on. We cannot change it on a previous commitfest, as that could
end up with the patch being open in two CFs at the same time, but
there is no reason it shouldn't be doable in the last one.

diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py
index 55302d9693e56abe7840c597c6f815cef0e29b19..1b2345491ddc3b7230cbdb27132df3f4d10273ac 100644 (file)
--- a/pgcommitfest/commitfest/views.py
+++ b/pgcommitfest/commitfest/views.py
@@ -311,8 +311,13 @@ def comment(request, cfid, patchid, what):
        is_review = (what=='review')
 
        if poc.is_closed:
-               messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
-               return HttpResponseRedirect('..')
+               # We allow modification of patches in closed CFs *only* if it's the
+               # last CF that the patch is part of. If it's part of another CF, that
+               # is later than this one, tell the user to go there instead.
+               lastcf = PatchOnCommitFest.objects.filter(patch=patch).order_by('-commitfest__startdate')[0]
+               if poc != lastcf:
+                       messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
+                       return HttpResponseRedirect('..')
 
        if request.method == 'POST':
                try:
@@ -387,25 +392,31 @@ def status(request, cfid, patchid, status):
        poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cfid, patch__id=patchid)
 
        if poc.is_closed:
-               messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
+               # We allow modification of patches in closed CFs *only* if it's the
+               # last CF that the patch is part of. If it's part of another CF, that
+               # is later than this one, tell the user to go there instead.
+               lastcf = PatchOnCommitFest.objects.filter(patch__id=patchid).order_by('-commitfest__startdate')[0]
+               if poc != lastcf:
+                       messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
+                       return HttpResponseRedirect('/%s/%s/' % (poc.commitfest.id, poc.patch.id))
+
+       if status == 'review':
+               newstatus = PatchOnCommitFest.STATUS_REVIEW
+       elif status == 'author':
+               newstatus = PatchOnCommitFest.STATUS_AUTHOR
+       elif status == 'committer':
+               newstatus = PatchOnCommitFest.STATUS_COMMITTER
        else:
-               if status == 'review':
-                       newstatus = PatchOnCommitFest.STATUS_REVIEW
-               elif status == 'author':
-                       newstatus = PatchOnCommitFest.STATUS_AUTHOR
-               elif status == 'committer':
-                       newstatus = PatchOnCommitFest.STATUS_COMMITTER
-               else:
-                       raise Exception("Can't happen")
+               raise Exception("Can't happen")
 
-               if newstatus != poc.status:
-                       # Only save it if something actually changed
-                       poc.status = newstatus
-                       poc.patch.set_modified()
-                       poc.patch.save()
-                       poc.save()
+       if newstatus != poc.status:
+               # Only save it if something actually changed
+               poc.status = newstatus
+               poc.patch.set_modified()
+               poc.patch.save()
+               poc.save()
 
-                       PatchHistory(patch=poc.patch, by=request.user, what='New status: %s' % poc.statusstring).save()
+               PatchHistory(patch=poc.patch, by=request.user, what='New status: %s' % poc.statusstring).save()
 
        return HttpResponseRedirect('/%s/%s/' % (poc.commitfest.id, poc.patch.id))
 
@@ -416,57 +427,63 @@ def close(request, cfid, patchid, status):
        poc = get_object_or_404(PatchOnCommitFest.objects.select_related(), commitfest__id=cfid, patch__id=patchid)
 
        if poc.is_closed:
-               messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
-       else:
-               poc.leavedate = datetime.now()
-
-               # We know the status can't be one of the ones below, since we
-               # have checked that we're not closed yet. Therefor, we don't
-               # need to check if the individual status has changed.
-               if status == 'reject':
-                       poc.status = PatchOnCommitFest.STATUS_REJECTED
-               elif status == 'feedback':
-                       poc.status = PatchOnCommitFest.STATUS_RETURNED
-                       # Figure out the commitfest to actually put it on
-                       newcf = CommitFest.objects.filter(status=CommitFest.STATUS_OPEN)
+               # We allow modification of patches in closed CFs *only* if it's the
+               # last CF that the patch is part of. If it's part of another CF, that
+               # is later than this one, tell the user to go there instead.
+               lastcf = PatchOnCommitFest.objects.filter(patch__id=patchid).order_by('-commitfest__startdate')[0]
+               if poc != lastcf:
+                       messages.add_message(request, messages.INFO, "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!")
+                       return HttpResponseRedirect('/%s/%s/' % (poc.commitfest.id, poc.patch.id))
+
+       poc.leavedate = datetime.now()
+
+       # We know the status can't be one of the ones below, since we
+       # have checked that we're not closed yet. Therefor, we don't
+       # need to check if the individual status has changed.
+       if status == 'reject':
+               poc.status = PatchOnCommitFest.STATUS_REJECTED
+       elif status == 'feedback':
+               poc.status = PatchOnCommitFest.STATUS_RETURNED
+               # Figure out the commitfest to actually put it on
+               newcf = CommitFest.objects.filter(status=CommitFest.STATUS_OPEN)
+               if len(newcf) == 0:
+                       # Ok, there is no open CF at all. Let's see if there is a
+                       # future one.
+                       newcf = CommitFest.objects.filter(status=CommitFest.STATUS_FUTURE)
                        if len(newcf) == 0:
-                               # Ok, there is no open CF at all. Let's see if there is a
-                               # future one.
-                               newcf = CommitFest.objects.filter(status=CommitFest.STATUS_FUTURE)
-                               if len(newcf) == 0:
-                                       raise Exception("No open and no future commitfest exists!")
-                               elif len(newcf) != 1:
-                                       raise Exception("No open and multiple future commitfests exist!")
+                               raise Exception("No open and no future commitfest exists!")
                        elif len(newcf) != 1:
-                               raise Exception("Multiple open commitfests exists!")
-                       elif newcf[0] == poc.commitfest:
-                               # The current open CF is the same one that we are already on.
-                               # In this case, try to see if there is a future CF we can
-                               # move it to.
-                               newcf = CommitFest.objects.filter(status=CommitFest.STATUS_FUTURE)
-                               if len(newcf) == 0:
-                                       raise Exception("Cannot move patch to the same commitfest, and no future commitfests exist!")
-                               elif len(newcf) != 1:
-                                       raise Exception("Cannot move patch to the same commitfest, and multiple future commitfests exist!")
-                       # Create a mapping to the new commitfest that we are bouncing
-                       # this patch to.
-                       newpoc = PatchOnCommitFest(patch=poc.patch, commitfest=newcf[0], enterdate=datetime.now())
-                       newpoc.save()
-               elif status == 'committed':
-                       committer = get_object_or_404(Committer, user__username=request.GET['c'])
-                       if committer != poc.patch.committer:
-                               # Committer changed!
-                               poc.patch.committer = committer
-                               PatchHistory(patch=poc.patch, by=request.user, what='Changed committer to %s' % committer).save()
-                       poc.status = PatchOnCommitFest.STATUS_COMMITTED
-               else:
-                       raise Exception("Can't happen")
+                               raise Exception("No open and multiple future commitfests exist!")
+               elif len(newcf) != 1:
+                       raise Exception("Multiple open commitfests exists!")
+               elif newcf[0] == poc.commitfest:
+                       # The current open CF is the same one that we are already on.
+                       # In this case, try to see if there is a future CF we can
+                       # move it to.
+                       newcf = CommitFest.objects.filter(status=CommitFest.STATUS_FUTURE)
+                       if len(newcf) == 0:
+                               raise Exception("Cannot move patch to the same commitfest, and no future commitfests exist!")
+                       elif len(newcf) != 1:
+                               raise Exception("Cannot move patch to the same commitfest, and multiple future commitfests exist!")
+               # Create a mapping to the new commitfest that we are bouncing
+               # this patch to.
+               newpoc = PatchOnCommitFest(patch=poc.patch, commitfest=newcf[0], enterdate=datetime.now())
+               newpoc.save()
+       elif status == 'committed':
+               committer = get_object_or_404(Committer, user__username=request.GET['c'])
+               if committer != poc.patch.committer:
+                       # Committer changed!
+                       poc.patch.committer = committer
+                       PatchHistory(patch=poc.patch, by=request.user, what='Changed committer to %s' % committer).save()
+               poc.status = PatchOnCommitFest.STATUS_COMMITTED
+       else:
+               raise Exception("Can't happen")
 
-               poc.patch.set_modified()
-               poc.patch.save()
-               poc.save()
+       poc.patch.set_modified()
+       poc.patch.save()
+       poc.save()
 
-               PatchHistory(patch=poc.patch, by=request.user, what='Closed in commitfest %s with status: %s' % (poc.commitfest, poc.statusstring)).save()
+       PatchHistory(patch=poc.patch, by=request.user, what='Closed in commitfest %s with status: %s' % (poc.commitfest, poc.statusstring)).save()
 
        return HttpResponseRedirect('/%s/%s/' % (poc.commitfest.id, poc.patch.id))