Fixed #37046 -- Allowed Ready for checkin Trac status in PR checks.

Author: varunkasyap

scripts/pr_quality/check_pr.py

@@ -57,6 +57,8 @@
 PR_TEMPLATE_DATE = date(2024, 3, 4)  # 3fcef50 -- PR template introduced
 AI_DISCLOSURE_DATE = date(2026, 1, 8)  # 4f580c4 -- AI disclosure added
 
+ALLOWED_STAGES = ("Accepted", "Ready for checkin")
+
 logger = logging.getLogger(__name__)
 
 
@@ -215,7 +217,8 @@ def check_trac_ticket(pr_body, total_changes, threshold=LARGE_PR_THRESHOLD):
 
 
 def check_trac_status(ticket_id, ticket_data):
-    """The referenced Trac ticket must be Accepted, unresolved, and assigned.
+    """The referenced Trac ticket must be Accepted or Ready for checkin,
+    unresolved, and assigned.
 
     ticket_data is the dict returned by fetch_trac_ticket(). Passing None
     skips the check (non-fatal fetch error). Passing TICKET_NOT_FOUND fails
@@ -232,10 +235,10 @@ def check_trac_status(ticket_id, ticket_data):
     stage = ticket_data.get("custom", {}).get("stage", "").strip()
     resolution = (ticket_data.get("resolution") or "").strip()
     status = ticket_data.get("status", "").strip()
-    if stage == "Accepted" and not resolution and status == "assigned":
+    if stage in ALLOWED_STAGES and not resolution and status == "assigned":
         return None
     current_state = [
-        f"{stage=}" if stage != "Accepted" else "",
+        f"{stage=}" if stage not in ALLOWED_STAGES else "",
         f"{resolution=}" if resolution else "",
         f"{status=}" if status != "assigned" else "",
     ]
@@ -480,7 +483,7 @@ def main(
 
     results = [
         ("Trac ticket referenced", ticket_result, LEVEL_ERROR),
-        ("Trac ticket status is Accepted", ticket_status_result, LEVEL_ERROR),
+        ("Trac ticket is ready for work", ticket_status_result, LEVEL_ERROR),
         ("Trac ticket has_patch flag set", ticket_has_patch_result, LEVEL_WARNING),
         ("PR title includes ticket number", pr_title_result, LEVEL_WARNING),
         ("Branch description provided", check_branch_description(pr_body), LEVEL_ERROR),

scripts/pr_quality/errors.py

@@ -56,18 +56,17 @@ def as_details(self, level=LEVEL_ERROR):
 INVALID_TRAC_STATUS = (
     "Trac Ticket Not Ready for a Pull Request",
     "The referenced ticket **ticket-{ticket_id}** is not ready for a pull request. "
-    "A ticket must be in the *Accepted* stage, *assigned* status, and have no "
-    "resolution.\n\n"
+    "A ticket must be in the *Accepted* or *Ready for checkin* stage, "
+    "*assigned* status, and have no resolution.\n\n"
     "**Current state:** {current_state}\n\n"
     "**What to do:**\n\n"
     f"1. Check the ticket at {TRAC_URL}/ticket/{{ticket_id}}.\n"
     "2. If *Unreviewed*, wait for a community member to accept it. "
     "A ticket cannot be accepted by its reporter.\n"
-    "3. If *Ready for Checkin*, there is already a solution for it.\n"
-    "4. If *Someday/Maybe*, discuss on the "
+    "3. If *Someday/Maybe*, discuss on the "
     f"[Django Forum]({FORUM_URL}/c/internals/5) before proceeding.\n"
-    "5. If resolved or closed, it is not eligible for a PR.\n"
-    "6. If not *assigned*, claim it by setting yourself as the owner.\n\n"
+    "4. If resolved or closed, it is not eligible for a PR.\n"
+    "5. If not *assigned*, claim it by setting yourself as the owner.\n\n"
     f"For more information on the Django triage process see {TRIAGING_URL}.",
 )
 

scripts/pr_quality/tests/test_check_pr.py

@@ -318,27 +318,34 @@ def test_accepted_assigned_unresolved_passes(self):
         data = make_trac_json(stage="Accepted", status="assigned", resolution=None)
         self.assertIsNone(check_pr.check_trac_status("36991", data))
 
+    def test_ready_for_checkin_assigned_unresolved_passes(self):
+        data = make_trac_json(
+            stage="Ready for checkin", status="assigned", resolution=None
+        )
+        self.assertIsNone(check_pr.check_trac_status("36991", data))
+
     def test_non_accepted_stage_fails(self):
-        for stage in ["Unreviewed", "Ready for Checkin", "Someday/Maybe"]:
+        for stage in ["Unreviewed", "Someday/Maybe"]:
             with self.subTest(stage=stage):
                 data = make_trac_json(stage=stage)
                 self.assertIsNotNone(check_pr.check_trac_status("36991", data))
 
     def test_resolved_ticket_fails(self):
-        for resolution in [
-            "fixed",
-            "wontfix",
-            "duplicate",
-            "invalid",
-            "worksforme",
-            "needsinfo",
-            "needsnewfeatureprocess",
-        ]:
-            with self.subTest(resolution=resolution):
-                data = make_trac_json(
-                    stage="Accepted", status="assigned", resolution=resolution
-                )
-                self.assertIsNotNone(check_pr.check_trac_status("36991", data))
+        for stage in ("Accepted", "Ready for checkin"):
+            for resolution in [
+                "fixed",
+                "wontfix",
+                "duplicate",
+                "invalid",
+                "worksforme",
+                "needsinfo",
+                "needsnewfeatureprocess",
+            ]:
+                with self.subTest(stage=stage, resolution=resolution):
+                    data = make_trac_json(
+                        stage=stage, status="assigned", resolution=resolution
+                    )
+                    self.assertIsNotNone(check_pr.check_trac_status("36991", data))
 
     def test_unassigned_ticket_fails(self):
         for status in ["new", "closed", ""]:
@@ -743,7 +750,7 @@ def test_no_ticket_results_include_skipped_sentinels(self):
         _, mock_summary, _ = self.call_main(pr_body=body)
         _, results, _ = mock_summary.call_args.args
         result_map = {name: result for name, result, _ in results}
-        self.assertIs(result_map["Trac ticket status is Accepted"], check_pr.SKIPPED)
+        self.assertIs(result_map["Trac ticket is ready for work"], check_pr.SKIPPED)
         self.assertIs(result_map["Trac ticket has_patch flag set"], check_pr.SKIPPED)
         self.assertIs(result_map["PR title includes ticket number"], check_pr.SKIPPED)
 
@@ -754,7 +761,7 @@ def test_non_accepted_ticket_skips_has_patch(self):
         )
         _, results, _ = mock_summary.call_args.args
         result_map = {name: result for name, result, _ in results}
-        self.assertIsNotNone(result_map["Trac ticket status is Accepted"])
+        self.assertIsNotNone(result_map["Trac ticket is ready for work"])
         self.assertIs(result_map["Trac ticket has_patch flag set"], check_pr.SKIPPED)
 
     def test_established_author_skips_all_checks(self):