diff --git a/backend/druks/contrib/software_factory/models.py b/backend/druks/contrib/software_factory/models.py index 63812fde..66dbbd3c 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 8d46ceab..c1973e3b 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 0229a9db..94ec0674 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 40530c87..3cd3d6f2 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 6a36e462..2937d201 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 e130ac4b..5b8dc1cf 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 39ee701c..1ed5e13c 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 00000000..5c8a854c --- /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 88ad190f..218885a3 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 fbe51d16..886278c5 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