From 610b5890ea15a8a9bb3dba403a7c2805fea78b33 Mon Sep 17 00:00:00 2001 From: Dan Serbanoiu <32792904+dserbano@users.noreply.github.com> Date: Tue, 3 Nov 2026 14:11:03 -0400 Subject: [PATCH] DRU-982 -- Return cancelled and closed work to rest only while active --- .../druks/contrib/software_factory/models.py | 1 + .../software_factory/ticketing/base.py | 10 ++++ .../software_factory/ticketing/druks.py | 4 +- .../software_factory/ticketing/github.py | 7 +++ .../software_factory/ticketing/jira.py | 5 +- .../software_factory/ticketing/linear.py | 3 +- backend/druks/core/apis/linear.py | 10 +++- .../test_cancel_ticket_status.py | 57 +++++++++++++++++++ .../tests/software_factory/test_ticketing.py | 19 ++++++- docs/configuration.md | 2 + 10 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 backend/tests/software_factory/test_cancel_ticket_status.py diff --git a/backend/druks/contrib/software_factory/models.py b/backend/druks/contrib/software_factory/models.py index 63812fde4..66dbbd3cd 100644 --- a/backend/druks/contrib/software_factory/models.py +++ b/backend/druks/contrib/software_factory/models.py @@ -322,6 +322,7 @@ async def resolve(self, resolution: Resolution, *, at: datetime) -> None: await github.delete_branch(self.repo, self.branch) except Exception: # noqa: BLE001 — cleanup only logger.warning("Skipped branch cleanup for %s.", self.repo, exc_info=True) + if resolution in (Resolution.CLOSED, Resolution.CANCELLED): await self.set_ticket_status(TicketStatus.BACKLOG) @classmethod diff --git a/backend/druks/contrib/software_factory/ticketing/base.py b/backend/druks/contrib/software_factory/ticketing/base.py index 8d46ceabd..c1973e3b2 100644 --- a/backend/druks/contrib/software_factory/ticketing/base.py +++ b/backend/druks/contrib/software_factory/ticketing/base.py @@ -11,6 +11,8 @@ class Tracker(ABC): known_exceptions = () # The MCP grant issuer whose subjects are this tracker's user ids. authority: str + # The configured name of each mapped status. An unmapped status is absent. + _status_names: dict[TicketStatus, str] async def get_account_id(self, user_id: str | None) -> str | None: """The one Druks account that connected this tracker as the user.""" @@ -23,6 +25,14 @@ async def __aenter__(self) -> Self: async def __aexit__(self, *exc) -> None: await self.aclose() + def get_active_status_names(self) -> set[str]: + """The names of the mapped statuses a build holds a ticket in.""" + return { + self._status_names[status] + for status in (TicketStatus.IN_PROGRESS, TicketStatus.IN_REVIEW) + if status in self._status_names + } + async def list_status_choices(self) -> list[dict[str, str]]: """The statuses an operator can pick, in the shape a ``Choices`` source returns.""" return [] diff --git a/backend/druks/contrib/software_factory/ticketing/druks.py b/backend/druks/contrib/software_factory/ticketing/druks.py index 0229a9db3..94ec06741 100644 --- a/backend/druks/contrib/software_factory/ticketing/druks.py +++ b/backend/druks/contrib/software_factory/ticketing/druks.py @@ -17,10 +17,12 @@ class DruksTracker(Tracker): """The board on this appliance. It writes the ticket row and needs no credentials.""" known_exceptions = (UnknownTicketError,) + _status_names = _BOARD async def set_status(self, key: str, status: TicketStatus) -> None: if ticket := await Ticket.get_for_identifier(key): - await ticket.transition(_BOARD[status]) + if status != TicketStatus.BACKLOG or ticket.status in self.get_active_status_names(): + await ticket.transition(_BOARD[status]) return raise UnknownTicketError(key, "druks") diff --git a/backend/druks/contrib/software_factory/ticketing/github.py b/backend/druks/contrib/software_factory/ticketing/github.py index 40530c876..3cd3d6f29 100644 --- a/backend/druks/contrib/software_factory/ticketing/github.py +++ b/backend/druks/contrib/software_factory/ticketing/github.py @@ -29,6 +29,13 @@ async def set_status(self, key: str, status: TicketStatus) -> None: if "/" not in repo or not number.isdigit(): raise UnknownTicketError(key, "GitHub") client = await services.Github.get_client(repo) + if status == TicketStatus.BACKLOG: + issue = await client.get_issue(repo, int(number)) + active_names = self.get_active_status_names() + if issue["state"] != "open" or not any( + label["name"] in active_names for label in issue["labels"] + ): + return others = {label for label in self._status_names.values() if label != name} await client.replace_issue_labels(repo, int(number), add=name, remove=others) if status is TicketStatus.DONE: diff --git a/backend/druks/contrib/software_factory/ticketing/jira.py b/backend/druks/contrib/software_factory/ticketing/jira.py index 6a36e4620..2937d201d 100644 --- a/backend/druks/contrib/software_factory/ticketing/jira.py +++ b/backend/druks/contrib/software_factory/ticketing/jira.py @@ -32,7 +32,10 @@ async def set_status(self, key: str, status: TicketStatus) -> None: raise ValueError(f"Jira has no configured status name for {status}") # Jira runs a self-transition as a real change: a history entry, watcher # notifications, and automation triggers. A ticket already at the status stays untouched. - if await self._client.get_issue_status(key) != name: + current = await self._client.get_issue_status(key) + if status == TicketStatus.BACKLOG and current not in self.get_active_status_names(): + return + if current != name: await self._client.transition_issue(key, name) async def list_status_choices(self) -> list[dict[str, str]]: diff --git a/backend/druks/contrib/software_factory/ticketing/linear.py b/backend/druks/contrib/software_factory/ticketing/linear.py index e130ac4b6..5b8dc1cf8 100644 --- a/backend/druks/contrib/software_factory/ticketing/linear.py +++ b/backend/druks/contrib/software_factory/ticketing/linear.py @@ -22,7 +22,8 @@ async def set_status(self, key: str, status: TicketStatus) -> None: raise ValueError(f"Linear has no configured status name for {status}") # The status mutation resolves the issue by identifier, so the key is # the id Linear wants. - await self._client.update_issue_status(key, name) + only_from = self.get_active_status_names() if status == TicketStatus.BACKLOG else None + await self._client.update_issue_status(key, name, only_from=only_from) async def list_status_choices(self) -> list[dict[str, str]]: choices = {} diff --git a/backend/druks/core/apis/linear.py b/backend/druks/core/apis/linear.py index 39ee701cc..1ed5e13c8 100644 --- a/backend/druks/core/apis/linear.py +++ b/backend/druks/core/apis/linear.py @@ -53,7 +53,10 @@ def __init__( async def aclose(self) -> None: await self._client.aclose() - async def update_issue_status(self, issue_id: str, status_name: str) -> dict[str, Any]: + async def update_issue_status( + self, issue_id: str, status_name: str, *, only_from: set[str] | None = None + ) -> dict[str, Any]: + """Move an issue if its current status is allowed. None allows any status.""" data = await self._execute( """ query DruksIssueWorkflowStates($issueId: String!) { @@ -76,7 +79,10 @@ async def update_issue_status(self, issue_id: str, status_name: str) -> dict[str if not issue: raise UnknownTicketError(issue_id, "Linear") current_status = issue["state"]["name"] - if current_status == status_name: + if current_status == status_name or ( + # ast-grep-ignore: rule-2 — an empty set allows no status; None allows all. + only_from is not None and current_status not in only_from + ): return { "identifier": issue["identifier"], "status": current_status, diff --git a/backend/tests/software_factory/test_cancel_ticket_status.py b/backend/tests/software_factory/test_cancel_ticket_status.py new file mode 100644 index 000000000..5c8a854c3 --- /dev/null +++ b/backend/tests/software_factory/test_cancel_ticket_status.py @@ -0,0 +1,57 @@ +from datetime import UTC, datetime +from unittest.mock import AsyncMock + +import pytest +from druks.contrib.software_factory.app import SoftwareFactory +from druks.contrib.software_factory.enums import Resolution, Status +from druks.contrib.software_factory.models import Project, ProjectRepo, Ticket +from druks.contrib.software_factory.ticketing.druks import DruksTracker +from druks.contrib.software_factory.ticketing.enums import TicketStatus +from druks.contrib.software_factory.ticketing.github import GitHub +from druks.core import services + +from software_factory.factories import make_test_work_item + + +@pytest.mark.parametrize("current", list(Status)) +async def test_cancel_returns_only_active_board_tickets_to_rest(monkeypatch, current): + project = await Project.create(name="Acme") + repo = await ProjectRepo.create(project_id=project.id, full_name="acme/widget") + ticket = await Ticket.create(project_repo=repo, title="Ticket") + ticket.status = current + item = await make_test_work_item( + repo=repo.full_name, source="druks", ticket_key=ticket.identifier + ) + monkeypatch.setattr(SoftwareFactory, "get_tracker", AsyncMock(return_value=DruksTracker())) + + await item.resolve(Resolution.CANCELLED, at=datetime.now(UTC)) + + expected = Status.BACKLOG if current in (Status.IN_PROGRESS, Status.IN_REVIEW) else current + assert ticket.status == expected + + +@pytest.mark.parametrize( + "state,label,changed", + [ + ("open", "working", True), + ("open", "reviewing", True), + ("open", "done", False), + ("closed", "working", False), + ("open", "ready", False), + ], +) +async def test_github_resting_label_requires_an_open_active_issue( + monkeypatch, state, label, changed +): + client = AsyncMock() + client.get_issue.return_value = {"state": state, "labels": [{"name": label}]} + monkeypatch.setattr(services.Github, "get_client", AsyncMock(return_value=client)) + tracker = GitHub( + status_names={ + TicketStatus.IN_PROGRESS: "working", + TicketStatus.IN_REVIEW: "reviewing", + TicketStatus.BACKLOG: "waiting", + } + ) + await tracker.set_status("acme/widget#1", TicketStatus.BACKLOG) + assert client.replace_issue_labels.await_count == int(changed) diff --git a/backend/tests/software_factory/test_ticketing.py b/backend/tests/software_factory/test_ticketing.py index 88ad190f1..218885a33 100644 --- a/backend/tests/software_factory/test_ticketing.py +++ b/backend/tests/software_factory/test_ticketing.py @@ -329,7 +329,7 @@ class _FakeLinearClient: def __init__(self) -> None: self.calls: list[tuple] = [] - async def update_issue_status(self, issue_id, status_name): + async def update_issue_status(self, issue_id, status_name, *, only_from=None): self.calls.append(("update_issue_status", issue_id, status_name)) async def aclose(self): @@ -445,6 +445,23 @@ def handler(request: httpx.Request) -> httpx.Response: assert requests[1]["variables"] == {"issueId": "ENG-831", "statusId": "target-id"} +async def test_linear_client_skips_the_mutation_from_a_status_not_allowed(): + requests = [] + + def handler(request: httpx.Request) -> httpx.Response: + requests.append(json.loads(request.content)) + return _linear_issue_response( + current="Done", states=[("current-id", "Done"), ("target-id", "Backlog")] + ) + + result = await _linear_client(handler).update_issue_status( + "ENG-831", "Backlog", only_from={"In Progress", "In Review"} + ) + + assert result == {"identifier": "ENG-831", "status": "Done", "changed": False} + assert len(requests) == 1 + + async def test_linear_client_lists_workflow_states_across_pages(): pages = iter( [ diff --git a/docs/configuration.md b/docs/configuration.md index fbe51d163..886278c51 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -569,6 +569,8 @@ progress, in review, done, and resting. When the selected tracker is connected and lists its statuses, the settings page offers them as choices. After you change the tracker, save the settings to list the statuses of the new tracker. An empty in review or resting status leaves the ticket where it is. +An operator cancel or a pull request closed without merge moves the ticket to +the resting status only if it is still in progress or in review. Select **druks** to use the ticket board on this appliance. That choice needs no credentials. The status settings stay hidden. The trigger status is