Allow marking an already closed patch as committed

pgcommitfest/commitfest/models.py

@@ -598,7 +598,13 @@ def update_lastmail(self):
             self.lastmail = max(threads, key=lambda t: t.latestmessage).latestmessage
 
     def move(
-        self, from_cf, to_cf, by_user, allow_move_to_in_progress=False, by_cfbot=False
+        self,
+        from_cf,
+        to_cf,
+        by_user,
+        allow_move_to_in_progress=False,
+        allow_move_closed=False,
+        by_cfbot=False,
     ):
         """Returns the new PatchOnCommitFest object, or raises UserInputError"""
 
@@ -609,11 +615,7 @@ def move(
         if from_cf.id == to_cf.id:
             raise UserInputError("Source and target commitfest are the same.")
 
-        if current_poc.status not in (
-            PatchOnCommitFest.STATUS_REVIEW,
-            PatchOnCommitFest.STATUS_AUTHOR,
-            PatchOnCommitFest.STATUS_COMMITTER,
-        ):
+        if current_poc.is_closed and not allow_move_closed:
             raise UserInputError(
                 f"Patch in state {current_poc.statusstring} cannot be moved."
             )
@@ -625,16 +627,18 @@ def move(
             raise UserInputError("Patch can only be moved to an open commitfest")
 
         old_status = current_poc.status
+        was_closed = current_poc.is_closed
 
         current_poc.set_status(PatchOnCommitFest.STATUS_MOVED)
 
+        now = datetime.now()
         new_poc, _ = PatchOnCommitFest.objects.update_or_create(
             patch=current_poc.patch,
             commitfest=to_cf,
             defaults=dict(
                 status=old_status,
-                enterdate=datetime.now(),
-                leavedate=None,
+                enterdate=now,
+                leavedate=now if was_closed else None,
             ),
         )
         new_poc.save()

pgcommitfest/commitfest/tests/test_close_committed.py

@@ -0,0 +1,36 @@
+from datetime import datetime
+
+import pytest
+
+from pgcommitfest.commitfest.models import Committer, Patch, PatchOnCommitFest
+
+pytestmark = pytest.mark.django_db
+
+
+@pytest.mark.parametrize(
+    "status",
+    [PatchOnCommitFest.STATUS_REVIEW, PatchOnCommitFest.STATUS_RETURNED],
+)
+def test_commit_moves_patch_to_in_progress_cf(
+    client, open_cf, in_progress_cf, alice, status
+):
+    Committer.objects.create(user=alice)
+    patch = Patch.objects.create(name="Test patch")
+    poc = PatchOnCommitFest.objects.create(
+        patch=patch,
+        commitfest=open_cf,
+        enterdate=datetime.now(),
+        status=PatchOnCommitFest.STATUS_REVIEW,
+    )
+    poc.set_status(status)
+
+    client.force_login(alice)
+    response = client.post(f"/patch/{patch.id}/close/committed/?c={alice.username}")
+    assert response.status_code == 302
+
+    old_poc = PatchOnCommitFest.objects.get(patch=patch, commitfest=open_cf)
+    assert old_poc.status == PatchOnCommitFest.STATUS_MOVED
+    new_poc = PatchOnCommitFest.objects.get(patch=patch, commitfest=in_progress_cf)
+    assert new_poc.status == PatchOnCommitFest.STATUS_COMMITTED
+    patch.refresh_from_db()
+    assert patch.committer.user == alice

pgcommitfest/commitfest/views.py

@@ -1179,20 +1179,26 @@ def close(request, patchid, status):
         if poc.commitfest.is_open:
             # Needs to be done before the next if condition, so the committer
             # that's set there is set on the correct poc.
+            #
+            # A patch that was already closed (e.g. returned with feedback)
+            # might still get committed later, and then it should also end up
+            # in the commitfest it was committed in.
             in_progress_cf = CommitFest.get_in_progress()
             if in_progress_cf is not None:
                 poc = patch.move(
                     poc.commitfest,
                     in_progress_cf,
                     request.user,
                     allow_move_to_in_progress=True,
+                    allow_move_closed=True,
                 )
             elif poc.commitfest.draft:
                 open_cf = CommitFest.get_open_regular()
                 poc = patch.move(
                     poc.commitfest,
                     open_cf,
                     request.user,
+                    allow_move_closed=True,
                 )
 
         committer_username = request.GET.get("c")