External/roboco
1
0
Fork 0
mirror of https://github.com/rennf93/roboco.git synced 2026-08-03 07:23:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

[f8480831] Batch B: extract route helpers in remaining smaller-offender route files (#760)

Browse Source 
* [f8480831] refactor(api): extract route-layer helpers into services/schemas/utils (batch B)

Moves 28 non-@router-decorated helper functions out of 15 route files
(optimal, project, release, dashboard, pitch, x, docs, git, playbooks,
product, provider, research, secretary, system, work_session) into
their paired services module (DB/service-calling helpers), the route's
schemas module as a converter (pure response/request shaping, mirroring
the existing project_to_response/assignment_to_response pattern), or
roboco/utils/converters.py (pure generic helpers). Adds two small
shared role-check helpers to api/deps.py (require_auditor_or_ceo,
require_role_in) for endpoint-specific role gates that had no existing
home. Placement-only: no route paths, schemas, or observable behavior
changed. Fixes the handful of tests that imported the old private
helper names directly.

* [f8480831] docs(map): document Batch B route-helper relocation in api-routes-schemas.md

* [f8480831] docs(map): add Key Symbols rows for require_auditor_or_ceo/require_role_in

---------

Co-authored-by: Backend Developer 2 <be-dev-2@roboco.tech>
Co-authored-by: Backend Documenter <be-doc@roboco.tech>
This commit is contained in:
roboco-app[bot]
2026-07-31 20:23:10 +00:00
committed by GitHub
co-authored by Backend Developer 2 Backend Documenter
parent 109b4d4d82
commit 7804e0fafa
37 changed files with 778 additions and 717 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/unit/api/routes/test_git_project_lookup.py
+23 -29
View File
@@ -1,13 +1,13 @@
"""Unit tests: _resolve_project_slug accepts slug or UUID."""
"""Unit tests: ProjectService.resolve_slug_or_404 accepts slug or UUID."""
from __future__ import annotations
from unittest.mock import AsyncMock, MagicMock, patch
from unittest.mock import AsyncMock, MagicMock
from uuid import UUID, uuid4
import pytest
from fastapi import HTTPException
from roboco.api.routes.git import _resolve_project_slug
from roboco.services.project import ProjectService
_HTTP_404 = 404
@@ -24,15 +24,15 @@ def _make_project(slug: str, uid: UUID) -> MagicMock:
async def test_resolve_project_slug_accepts_slug() -> None:
"""A plain slug string resolves to the project's slug."""
project = _make_project("roboco", uuid4())
mock_service = MagicMock()
mock_service.get_by_slug = AsyncMock(return_value=project)
service = ProjectService(MagicMock())
service.get_by_slug = AsyncMock(return_value=project) # type: ignore[method-assign]
service.get = AsyncMock() # type: ignore[method-assign]
with patch("roboco.api.routes.git.get_project_service", return_value=mock_service):
result = await _resolve_project_slug("roboco", MagicMock())
result = await service.resolve_slug_or_404("roboco")
assert result == "roboco"
mock_service.get_by_slug.assert_awaited_once_with("roboco")
mock_service.get.assert_not_called()
service.get_by_slug.assert_awaited_once_with("roboco")
service.get.assert_not_called()
@pytest.mark.asyncio
@@ -40,28 +40,25 @@ async def test_resolve_project_slug_accepts_uuid() -> None:
"""A UUID string resolves to the project's slug."""
uid = uuid4()
project = _make_project("roboco", uid)
mock_service = MagicMock()
mock_service.get = AsyncMock(return_value=project)
service = ProjectService(MagicMock())
service.get = AsyncMock(return_value=project) # type: ignore[method-assign]
service.get_by_slug = AsyncMock() # type: ignore[method-assign]
with patch("roboco.api.routes.git.get_project_service", return_value=mock_service):
result = await _resolve_project_slug(str(uid), MagicMock())
result = await service.resolve_slug_or_404(str(uid))
assert result == "roboco"
mock_service.get.assert_awaited_once_with(UUID(str(uid)))
mock_service.get_by_slug.assert_not_called()
service.get.assert_awaited_once_with(UUID(str(uid)))
service.get_by_slug.assert_not_called()
@pytest.mark.asyncio
async def test_resolve_project_slug_raises_404_for_missing_slug() -> None:
"""Unknown slug raises HTTPException 404."""
mock_service = MagicMock()
mock_service.get_by_slug = AsyncMock(return_value=None)
service = ProjectService(MagicMock())
service.get_by_slug = AsyncMock(return_value=None) # type: ignore[method-assign]
with (
patch("roboco.api.routes.git.get_project_service", return_value=mock_service),
pytest.raises(HTTPException) as exc_info,
):
await _resolve_project_slug("nonexistent", MagicMock())
with pytest.raises(HTTPException) as exc_info:
await service.resolve_slug_or_404("nonexistent")
assert exc_info.value.status_code == _HTTP_404
assert "nonexistent" in exc_info.value.detail
@@ -71,14 +68,11 @@ async def test_resolve_project_slug_raises_404_for_missing_slug() -> None:
async def test_resolve_project_slug_raises_404_for_missing_uuid() -> None:
"""UUID that matches no project raises HTTPException 404."""
uid = uuid4()
mock_service = MagicMock()
mock_service.get = AsyncMock(return_value=None)
service = ProjectService(MagicMock())
service.get = AsyncMock(return_value=None) # type: ignore[method-assign]
with (
patch("roboco.api.routes.git.get_project_service", return_value=mock_service),
pytest.raises(HTTPException) as exc_info,
):
await _resolve_project_slug(str(uid), MagicMock())
with pytest.raises(HTTPException) as exc_info:
await service.resolve_slug_or_404(str(uid))
assert exc_info.value.status_code == _HTTP_404
assert str(uid) in exc_info.value.detail
tests/unit/api/routes/test_x_project_fields.py
+10 -10
View File
@@ -1,9 +1,9 @@
"""``roboco/api/routes/x.py`` response-builder wiring for project_slug/
"""``roboco/api/schemas/x.py`` response-builder wiring for project_slug/
project_name. The sa_inspect(task).unloaded guard branches themselves are
covered once on the shared helper in tests/unit/api/schemas/test_project_fields.py
— this only asserts _to_response/_to_history_response actually populate
the response from it (loaded case; a real ORM task always resolves the
"loaded" branch since ``project`` is lazy="joined")."""
— this only asserts task_to_post_response/task_to_post_history_response
actually populate the response from it (loaded case; a real ORM task always
resolves the "loaded" branch since ``project`` is lazy="joined")."""
from __future__ import annotations
@@ -11,11 +11,11 @@ from types import SimpleNamespace
from typing import Any
from unittest.mock import MagicMock, patch
from roboco.api.routes.x import _to_history_response, _to_response
from roboco.api.schemas.x import task_to_post_history_response, task_to_post_response
def _stub_task(*, with_project: bool = False) -> Any:
"""A TaskTable stand-in matching _to_response/_to_history_response's reads."""
"""A TaskTable stand-in matching the response builders' reads."""
return SimpleNamespace(
id="task-1",
source="x_post",
@@ -44,7 +44,7 @@ def test_to_response_includes_project_fields_when_loaded() -> None:
"roboco.api.schemas.project_fields.sa_inspect",
return_value=_loaded_inspector(),
):
resp = _to_response(_stub_task(with_project=True))
resp = task_to_post_response(_stub_task(with_project=True))
assert resp.project_slug == "acme-robotics"
assert resp.project_name == "Acme Robotics"
@@ -54,7 +54,7 @@ def test_to_response_omits_project_fields_when_project_unset() -> None:
"roboco.api.schemas.project_fields.sa_inspect",
return_value=_loaded_inspector(),
):
resp = _to_response(_stub_task(with_project=False))
resp = task_to_post_response(_stub_task(with_project=False))
assert resp.project_slug is None
assert resp.project_name is None
@@ -64,7 +64,7 @@ def test_to_history_response_includes_project_fields_when_loaded() -> None:
"roboco.api.schemas.project_fields.sa_inspect",
return_value=_loaded_inspector(),
):
resp = _to_history_response(_stub_task(with_project=True))
resp = task_to_post_history_response(_stub_task(with_project=True))
assert resp.project_slug == "acme-robotics"
assert resp.project_name == "Acme Robotics"
@@ -74,6 +74,6 @@ def test_to_history_response_omits_project_fields_when_project_unset() -> None:
"roboco.api.schemas.project_fields.sa_inspect",
return_value=_loaded_inspector(),
):
resp = _to_history_response(_stub_task(with_project=False))
resp = task_to_post_history_response(_stub_task(with_project=False))
assert resp.project_slug is None
assert resp.project_name is None
tests/unit/api/test_dashboard_auditor_auth.py
+2 -2
View File
@@ -1,7 +1,7 @@
"""Dashboard auditor flag/report mutating routes (``create_auditor_flag``,
``resolve_auditor_flag``, ``create_auditor_report``, ``send_auditor_report``)
are gated to AUDITOR or CEO via a ``CurrentAgentContext`` dependency plus a
coarse role gate, mirroring ``roboco/api/routes/playbooks.py::_require_curator``.
are gated to AUDITOR or CEO via a ``CurrentAgentContext`` dependency plus
``roboco.api.deps.require_auditor_or_ceo`` — the same check playbooks.py uses.
"""
from __future__ import annotations
tests/unit/api/test_git_file_range.py
+57 -20
View File
@@ -1,4 +1,4 @@
"""Unit tests for the /git/file range computation (roboco.api.routes.git).
"""Unit tests for the /git/file range computation (roboco.utils.converters).
Pure logic — no DB, no git. Covers the line/context windowing, explicit
range, whole-file cap, and truncation flag.
@@ -6,50 +6,79 @@ range, whole-file cap, and truncation flag.
from __future__ import annotations
from roboco.api.routes.git import _FILE_MAX_LINES, _compute_file_range
from roboco.api.routes.git import _FILE_MAX_LINES
from roboco.utils.converters import compute_file_range
class TestComputeFileRange:
def test_line_centers_context_window(self) -> None:
s, e_, trunc = _compute_file_range(
total=100, line=50, context=10, start=None, end=None
s, e_, trunc = compute_file_range(
total=100,
line=50,
context=10,
explicit_range=None,
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (40, 60, True)
def test_line_window_clamps_to_file_start(self) -> None:
s, e_, trunc = _compute_file_range(
total=100, line=3, context=10, start=None, end=None
s, e_, trunc = compute_file_range(
total=100,
line=3,
context=10,
explicit_range=None,
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (1, 13, True)
def test_line_window_clamps_to_file_end(self) -> None:
s, e_, trunc = _compute_file_range(
total=100, line=98, context=10, start=None, end=None
s, e_, trunc = compute_file_range(
total=100,
line=98,
context=10,
explicit_range=None,
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (88, 100, False)
def test_explicit_start_end_override_line(self) -> None:
s, e_, trunc = _compute_file_range(
total=100, line=50, context=10, start=5, end=8
s, e_, trunc = compute_file_range(
total=100,
line=50,
context=10,
explicit_range=(5, 8),
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (5, 8, True)
def test_whole_file_when_no_range_args(self) -> None:
s, e_, trunc = _compute_file_range(
total=50, line=None, context=10, start=None, end=None
s, e_, trunc = compute_file_range(
total=50,
line=None,
context=10,
explicit_range=None,
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (1, 50, False)
def test_whole_file_capped_when_huge(self) -> None:
total = _FILE_MAX_LINES + 500
s, e_, trunc = _compute_file_range(
total=total, line=None, context=10, start=None, end=None
s, e_, trunc = compute_file_range(
total=total,
line=None,
context=10,
explicit_range=None,
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (1, _FILE_MAX_LINES, True)
def test_empty_file(self) -> None:
s, e_, trunc = _compute_file_range(
total=0, line=None, context=10, start=None, end=None
s, e_, trunc = compute_file_range(
total=0,
line=None,
context=10,
explicit_range=None,
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (1, 1, False)
@@ -57,14 +86,22 @@ class TestComputeFileRange:
# start=1, end=total-1 is not the exact-whole-file shape, but the
# resolved window is still oversized and must be capped.
total = 50000
s, e_, trunc = _compute_file_range(
total=total, line=None, context=10, start=1, end=total - 1
s, e_, trunc = compute_file_range(
total=total,
line=None,
context=10,
explicit_range=(1, total - 1),
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (1, _FILE_MAX_LINES, True)
def test_oversized_line_context_window_is_capped(self) -> None:
total = 10000
s, e_, trunc = _compute_file_range(
total=total, line=5000, context=3000, start=None, end=None
s, e_, trunc = compute_file_range(
total=total,
line=5000,
context=3000,
explicit_range=None,
max_lines=_FILE_MAX_LINES,
)
assert (s, e_, trunc) == (2000, 2000 + _FILE_MAX_LINES - 1, True)