From 3364355c7385afaefeea0658955be3918e821045 Mon Sep 17 00:00:00 2001 From: emil Date: Sat, 20 Jun 2026 17:48:08 +0000 Subject: [PATCH] GRM-28: fix: Vikunja task resolution pagination in post_merge.py --- .../tasks/download_gitea_runner.yml | 7 +++ scripts/post_merge.py | 36 ++++++++------ src/gitea_runner_manager/api_clients.py | 5 ++ tests/unit/test_api_clients.py | 13 +++++ tests/unit/test_post_merge.py | 49 +++++++++++-------- 5 files changed, 76 insertions(+), 34 deletions(-) diff --git a/ansible/roles/gitea-runner/tasks/download_gitea_runner.yml b/ansible/roles/gitea-runner/tasks/download_gitea_runner.yml index 0ae8c21..75baf3b 100644 --- a/ansible/roles/gitea-runner/tasks/download_gitea_runner.yml +++ b/ansible/roles/gitea-runner/tasks/download_gitea_runner.yml @@ -9,6 +9,9 @@ register: gitea_runner_release when: gitea_runner_version | default('latest') == 'latest' changed_when: false + retries: 3 + delay: 5 + until: gitea_runner_release is not failed - name: Set gitea_runner version from latest release ansible.builtin.set_fact: @@ -38,4 +41,8 @@ dest: "{{ gitea_runner_binary_path }}" mode: "0755" force: true + register: gitea_runner_download notify: Restart gitea-runner + retries: 3 + delay: 5 + until: gitea_runner_download is not failed diff --git a/scripts/post_merge.py b/scripts/post_merge.py index 7c8c1be..b7f8d62 100644 --- a/scripts/post_merge.py +++ b/scripts/post_merge.py @@ -33,21 +33,29 @@ def extract_conventional_msg(commit_msg: str) -> str: def resolve_task_id(client: VikunjaClient, task_id: str) -> int: - """Resolve GRM-N identifier to Vikunja numeric task ID.""" - tasks = client.list_tasks(per_page=DEFAULT_PER_PAGE) - matches = [ - t for t in tasks - if t.get("project_id") == VIKUNJA_PROJECT_ID and t.get("identifier") == task_id - ] - if not matches: - raise click.ClickException( - _( - "Could not find Vikunja task for {task_id} in project {project_id}.", - task_id=task_id, - project_id=VIKUNJA_PROJECT_ID, - ) + """Resolve GRM-N identifier to Vikunja numeric task ID. + + Paginates through the project's tasks to handle projects with more + than 50 tasks. + """ + page = 1 + while True: + tasks = client.list_project_tasks(VIKUNJA_PROJECT_ID, page=page, per_page=DEFAULT_PER_PAGE) + if not tasks: + break + matches = [t for t in tasks if t.get("identifier") == task_id] + if matches: + return int(matches[0]["id"]) + if len(tasks) < DEFAULT_PER_PAGE: + break + page += 1 + raise click.ClickException( + _( + "Could not find Vikunja task for {task_id} in project {project_id}.", + task_id=task_id, + project_id=VIKUNJA_PROJECT_ID, ) - return int(matches[0]["id"]) + ) def build_comment(task_id: str, conv_msg: str, commit_sha: str) -> str: diff --git a/src/gitea_runner_manager/api_clients.py b/src/gitea_runner_manager/api_clients.py index eaa423a..571dfc8 100644 --- a/src/gitea_runner_manager/api_clients.py +++ b/src/gitea_runner_manager/api_clients.py @@ -157,6 +157,11 @@ class VikunjaClient: r = self._request("GET", "/tasks", params=params) return r.json() + def list_project_tasks(self, project_id: int, **params: Any) -> list[dict[str, Any]]: + """List tasks in a specific project (more efficient than listing all tasks).""" + r = self._request("GET", f"/projects/{project_id}/tasks", params=params) + return r.json() + def post_comment(self, task_id: int, comment: str) -> None: self._request("POST", f"/tasks/{task_id}/comments", json={"comment": comment}) diff --git a/tests/unit/test_api_clients.py b/tests/unit/test_api_clients.py index 53137d9..9304e6e 100644 --- a/tests/unit/test_api_clients.py +++ b/tests/unit/test_api_clients.py @@ -249,6 +249,19 @@ class TestVikunjaClient: params={"per_page": DEFAULT_PER_PAGE}, ) + def test_list_project_tasks(self) -> None: + client = VikunjaClient("https://work.example.com", "tok") + client._session.request = MagicMock(return_value=_mock_response([{"id": 1, "identifier": "GRM-19"}])) + + result = client.list_project_tasks(VIKUNJA_PROJECT_ID, page=1, per_page=DEFAULT_PER_PAGE) + assert len(result) == 1 + client._session.request.assert_called_once_with( + "GET", + f"https://work.example.com/projects/{VIKUNJA_PROJECT_ID}/tasks", + timeout=DEFAULT_TIMEOUT, + params={"page": 1, "per_page": DEFAULT_PER_PAGE}, + ) + def test_post_comment(self) -> None: client = VikunjaClient("https://work.example.com", "tok") client._session.request = MagicMock(return_value=_mock_response()) diff --git a/tests/unit/test_post_merge.py b/tests/unit/test_post_merge.py index e3cfdf9..91405a3 100644 --- a/tests/unit/test_post_merge.py +++ b/tests/unit/test_post_merge.py @@ -7,7 +7,6 @@ import click import pytest from click.testing import CliRunner -from gitea_runner_manager.config import VIKUNJA_PROJECT_ID from gitea_runner_manager.exceptions import APIError from scripts.post_merge import ( build_comment, @@ -45,31 +44,41 @@ class TestBuildComment: class TestResolveTaskId: def test_found(self) -> None: mock_client = MagicMock() - mock_client.list_tasks.return_value = [ - {"id": 42, "project_id": VIKUNJA_PROJECT_ID, "identifier": "GRM-19"}, + mock_client.list_project_tasks.return_value = [ + {"id": 42, "identifier": "GRM-19"}, ] assert resolve_task_id(mock_client, "GRM-19") == 42 - mock_client.list_tasks.assert_called_once() + mock_client.list_project_tasks.assert_called_once() def test_not_found_raises(self) -> None: mock_client = MagicMock() - mock_client.list_tasks.return_value = [] + mock_client.list_project_tasks.return_value = [] with pytest.raises(click.ClickException) as exc: resolve_task_id(mock_client, "GRM-99") assert "Could not find" in str(exc.value) - def test_wrong_project_filtered(self) -> None: + def test_found_on_second_page(self) -> None: + """Task is on page 2 when project has more than 50 tasks.""" mock_client = MagicMock() - mock_client.list_tasks.return_value = [ - {"id": 42, "project_id": 999, "identifier": "GRM-19"}, - ] + page1 = [{"id": i, "identifier": f"GRM-{i}"} for i in range(50)] + page2 = [{"id": 100, "identifier": "GRM-99"}] + mock_client.list_project_tasks.side_effect = [page1, page2] + assert resolve_task_id(mock_client, "GRM-99") == 100 + assert mock_client.list_project_tasks.call_count == 2 + + def test_stops_when_page_is_partial(self) -> None: + """Stops paginating when a page has fewer than DEFAULT_PER_PAGE results.""" + mock_client = MagicMock() + page1 = [{"id": i, "identifier": f"GRM-{i}"} for i in range(10)] + mock_client.list_project_tasks.return_value = page1 with pytest.raises(click.ClickException) as exc: - resolve_task_id(mock_client, "GRM-19") + resolve_task_id(mock_client, "GRM-99") assert "Could not find" in str(exc.value) + assert mock_client.list_project_tasks.call_count == 1 def test_http_error_propagates(self) -> None: mock_client = MagicMock() - mock_client.list_tasks.side_effect = APIError(http.HTTPStatus.INTERNAL_SERVER_ERROR, "server error") + mock_client.list_project_tasks.side_effect = APIError(http.HTTPStatus.INTERNAL_SERVER_ERROR, "server error") with pytest.raises(APIError): resolve_task_id(mock_client, "GRM-19") @@ -79,8 +88,8 @@ class TestMain: @patch("scripts.post_merge.VikunjaClient") def test_full_flow(self, mock_client_cls: MagicMock) -> None: mock_client = MagicMock() - mock_client.list_tasks.return_value = [ - {"id": 267, "project_id": VIKUNJA_PROJECT_ID, "identifier": "GRM-20"}, + mock_client.list_project_tasks.return_value = [ + {"id": 267, "identifier": "GRM-20"}, ] mock_client_cls.return_value = mock_client runner = CliRunner() @@ -97,8 +106,8 @@ class TestMain: @patch("scripts.post_merge.VikunjaClient") def test_no_commit_sha(self, mock_client_cls: MagicMock) -> None: mock_client = MagicMock() - mock_client.list_tasks.return_value = [ - {"id": 267, "project_id": VIKUNJA_PROJECT_ID, "identifier": "GRM-20"}, + mock_client.list_project_tasks.return_value = [ + {"id": 267, "identifier": "GRM-20"}, ] mock_client_cls.return_value = mock_client runner = CliRunner() @@ -126,7 +135,7 @@ class TestMain: @patch("scripts.post_merge.VikunjaClient") def test_resolve_failure_propagates(self, mock_client_cls: MagicMock) -> None: mock_client = MagicMock() - mock_client.list_tasks.return_value = [] + mock_client.list_project_tasks.return_value = [] mock_client_cls.return_value = mock_client runner = CliRunner() result = runner.invoke(main, ["GRM-20: fix: bug"]) @@ -137,8 +146,8 @@ class TestMain: @patch("scripts.post_merge.VikunjaClient") def test_post_comment_failure_raises_click(self, mock_client_cls: MagicMock) -> None: mock_client = MagicMock() - mock_client.list_tasks.return_value = [ - {"id": 267, "project_id": VIKUNJA_PROJECT_ID, "identifier": "GRM-20"}, + mock_client.list_project_tasks.return_value = [ + {"id": 267, "identifier": "GRM-20"}, ] mock_client.post_comment.side_effect = APIError(http.HTTPStatus.INTERNAL_SERVER_ERROR, "server error") mock_client_cls.return_value = mock_client @@ -151,8 +160,8 @@ class TestMain: @patch("scripts.post_merge.VikunjaClient") def test_mark_done_failure_raises_click(self, mock_client_cls: MagicMock) -> None: mock_client = MagicMock() - mock_client.list_tasks.return_value = [ - {"id": 267, "project_id": VIKUNJA_PROJECT_ID, "identifier": "GRM-20"}, + mock_client.list_project_tasks.return_value = [ + {"id": 267, "identifier": "GRM-20"}, ] mock_client.post_comment.return_value = None mock_client.update_task.side_effect = APIError(http.HTTPStatus.INTERNAL_SERVER_ERROR, "server error")