diff --git a/TASKS.md b/TASKS.md index 282f35f..c469ea3 100644 --- a/TASKS.md +++ b/TASKS.md @@ -2,12 +2,6 @@ ## Todo -### When moving tasks back from archive + done, issue remains closed but status updates - - -### When moving tasks back from archive + done, --archive-done flag does not seem to be idempotent and requires a follow up run without the flag - - ## In Progress ### Implement label syncing for items @@ -20,3 +14,8 @@ corresponding GitHub Issues. This includes: ## Done +### When moving tasks back from archive + done, issue remains closed but status updates + + +### When moving tasks back from archive + done, --archive-done flag does not seem to be idempotent and requires a follow up run without the flag + diff --git a/tasksmd_sync/cli.py b/tasksmd_sync/cli.py index d156ad1..c2cea8f 100644 --- a/tasksmd_sync/cli.py +++ b/tasksmd_sync/cli.py @@ -126,9 +126,12 @@ def main(argv: list[str] | None = None) -> int: task_file = parse_tasks_file(tasks_path) logging.info("Found %d tasks", len(task_file.tasks)) - # If --archive-done is set, we only sync tasks that are NOT Done. - # The Done tasks will be removed from the local file and archived on the board - # by the normal sync process (because they are no longer in the task list). + # If --archive-done is set, remove Done tasks from the file first (for + # idempotency), then filter them from the task list before syncing. + if args.archive_done and not args.dry_run: + if remove_done_tasks(tasks_path): + logging.info("Removed 'Done' tasks from %s", tasks_path) + task_file = parse_tasks_file(tasks_path) if args.archive_done: original_count = len(task_file.tasks) task_file.tasks = [t for t in task_file.tasks if t.status != "Done"] @@ -182,11 +185,6 @@ def main(argv: list[str] | None = None) -> int: else: logging.info("No new IDs to write back") - # If --archive-done is set, remove them from the file now - if args.archive_done and not args.dry_run: - if remove_done_tasks(tasks_path): - logging.info("Removed 'Done' tasks from %s", tasks_path) - # Report logging.info( "Sync complete: %d created, %d updated, %d archived, %d unarchived, %d unchanged", diff --git a/tasksmd_sync/github_projects.py b/tasksmd_sync/github_projects.py index 1926c62..2b69893 100644 --- a/tasksmd_sync/github_projects.py +++ b/tasksmd_sync/github_projects.py @@ -288,6 +288,66 @@ def _parse_item_node( return item + def get_item(self, item_id: str) -> ProjectItem | None: + """Fetch a single project item by its ID. + + Returns the ProjectItem if found, or None if the item doesn't exist. + """ + fields = self.get_fields() + status_field = fields.get("Status") + + query = """ + query($itemId: ID!) { + node(id: $itemId) { + ... on ProjectV2Item { + id + fieldValues(first: 20) { + nodes { + ... on ProjectV2ItemFieldSingleSelectValue { + field { ... on ProjectV2SingleSelectField { name } } + name + } + } + } + content { + __typename + ... on Issue { + id + title + body + repository { + name + owner { login } + } + assignees(first: 5) { + nodes { login } + } + labels(first: 20) { + nodes { name } + } + } + ... on DraftIssue { + id + title + body + assignees(first: 5) { + nodes { login } + } + } + } + } + } + } + """ + try: + data = self._graphql(query, {"itemId": item_id}) + node = data.get("node") + if not node or "id" not in node: + return None + return self._parse_item_node(node, status_field) + except Exception: + return None + # ------------------------------------------------------------------ # Write operations # ------------------------------------------------------------------ @@ -610,6 +670,24 @@ def unarchive_item(self, item_id: str) -> None: """ self._graphql(mutation, {"projectId": project_id, "itemId": item_id}) + def reopen_issue(self, issue_id: str) -> None: + """Reopen a closed GitHub Issue. + + Args: + issue_id: The Issue node ID (I_...), NOT the ProjectV2Item ID. + """ + mutation = """ + mutation($issueId: ID!) { + updateIssue(input: { + id: $issueId, + state: OPEN + }) { + issue { id } + } + } + """ + self._graphql(mutation, {"issueId": issue_id}) + def close(self) -> None: """Close the HTTP client.""" self._client.close() diff --git a/tasksmd_sync/sync.py b/tasksmd_sync/sync.py index 3f29026..22330a0 100644 --- a/tasksmd_sync/sync.py +++ b/tasksmd_sync/sync.py @@ -227,9 +227,25 @@ def execute_sync( "Unarchived board item '%s' (%s)", task.title, task.board_item_id ) result.unarchived += 1 - # After unarchiving, it might still need updates, but for simplicity - # we'll let the NEXT sync run handle the updates now that it's visible. - # Actually, we could try to update it now, but we don't have the ProjectItem object. + + # After unarchiving, apply task fields (e.g. status) and reopen + # the underlying Issue if it was closed. + _apply_task_fields(client, task.board_item_id, task, fields) + try: + bi = client.get_item(task.board_item_id) + if bi and bi.content_type == "Issue" and bi.content_id: + client.reopen_issue(bi.content_id) + logger.info( + "Reopened Issue '%s' (%s)", + task.title, + bi.content_id, + ) + except Exception as e: + logger.warning( + "Failed to reopen Issue after unarchiving '%s': %s", + task.title, + e, + ) except Exception as e: msg = f"Failed to unarchive '{task.title}': {e}" logger.error(msg) diff --git a/tests/test_archive_done.py b/tests/test_archive_done.py index e3f84fd..08c78a4 100644 --- a/tests/test_archive_done.py +++ b/tests/test_archive_done.py @@ -48,3 +48,61 @@ def test_remove_done_tasks_no_done(tmp_path): modified = remove_done_tasks(tasks_file) assert modified is False + + +def test_remove_done_tasks_idempotent(tmp_path): + """Running remove_done_tasks twice should be idempotent.""" + tasks_file = tmp_path / "TASKS.md" + tasks_file.write_text( + """\ +# Tasks + +## Todo + +### Active Task +Keep this. + +## Done + +### Completed Task +Remove this. +""", + encoding="utf-8", + ) + + # First run removes Done tasks + modified = remove_done_tasks(tasks_file) + assert modified is True + content_after_first = tasks_file.read_text(encoding="utf-8") + assert "Completed Task" not in content_after_first + assert "Active Task" in content_after_first + + # Second run should be a no-op + modified = remove_done_tasks(tasks_file) + assert modified is False + content_after_second = tasks_file.read_text(encoding="utf-8") + assert content_after_first == content_after_second + + +def test_remove_done_tasks_empty_done_section(tmp_path): + """An empty Done section should not cause issues.""" + tasks_file = tmp_path / "TASKS.md" + tasks_file.write_text( + """\ +# Tasks + +## Todo + +### Active Task +Keep this. + +## Done + +""", + encoding="utf-8", + ) + + modified = remove_done_tasks(tasks_file) + assert modified is False + content = tasks_file.read_text(encoding="utf-8") + assert "Active Task" in content diff --git a/tests/test_execute_sync.py b/tests/test_execute_sync.py index 2331053..0e8f060 100644 --- a/tests/test_execute_sync.py +++ b/tests/test_execute_sync.py @@ -1075,3 +1075,82 @@ def test_parse_null_body(self): } item = self._parse(node) assert item.description == "" + + +# =================================================================== +# execute_sync — unarchive path +# =================================================================== + + +class TestExecuteSyncUnarchive: + """Tests for unarchive behaviour in execute_sync.""" + + def test_unarchive_reopens_issue(self): + """Unarchiving a task should reopen the underlying Issue.""" + # Board is empty (the item is archived), so the task triggers unarchive + client = _mock_client(board_items=[]) + # After unarchive, get_item returns the now-visible item + client.get_item.return_value = _make_board_item( + "PVTI_archived", + title="Revived task", + status="Done", + content_type="Issue", + content_id="I_archived", + ) + tf = TaskFile( + tasks=[ + _make_task("Revived task", board_id="PVTI_archived", status="Todo"), + ] + ) + + result = execute_sync(client, tf) + + assert result.unarchived == 1 + client.unarchive_item.assert_called_once_with("PVTI_archived") + client.reopen_issue.assert_called_once_with("I_archived") + # Status should also be applied + client.update_item_field_single_select.assert_called_once_with( + "PVTI_archived", "F_status", "OPT_todo" + ) + + def test_unarchive_draft_issue_does_not_reopen(self): + """Unarchiving a DraftIssue should NOT call reopen_issue.""" + client = _mock_client(board_items=[]) + client.get_item.return_value = _make_board_item( + "PVTI_archived", + title="Draft task", + content_type="DraftIssue", + content_id="DI_1", + ) + tf = TaskFile( + tasks=[ + _make_task("Draft task", board_id="PVTI_archived", status="Todo"), + ] + ) + + result = execute_sync(client, tf) + + assert result.unarchived == 1 + client.unarchive_item.assert_called_once_with("PVTI_archived") + client.reopen_issue.assert_not_called() + + def test_unarchive_reopen_failure_does_not_crash(self): + """If reopening the Issue fails, unarchive should still succeed.""" + client = _mock_client(board_items=[]) + client.get_item.return_value = _make_board_item( + "PVTI_archived", + title="Task", + content_type="Issue", + content_id="I_1", + ) + client.reopen_issue.side_effect = RuntimeError("API error") + tf = TaskFile( + tasks=[ + _make_task("Task", board_id="PVTI_archived", status="Todo"), + ] + ) + + result = execute_sync(client, tf) + + assert result.unarchived == 1 + assert len(result.errors) == 0 # reopen failure is warning-logged, not an error