Skip to content

Commit 839c49b

Browse files
Address PR review findings
- Fix papers last_sync timestamp comparison: use _parse_iso_datetime instead of string max() to avoid wrong result with mixed UTC offsets - Fix dashboard /sync/health URL: pass community_id param so health badge reflects the viewed community, not always 'hed' - Add tests for /sync/health community_id param: 404 for unknown, 200 for known community
1 parent b9cf46c commit 839c49b

File tree

3 files changed

+24
-3
lines changed

3 files changed

+24
-3
lines changed

dashboard/osa/index.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ <h2>Communities</h2>
763763
fetch(`${API_BASE}/${encodeURIComponent(communityId)}/metrics/public`),
764764
fetch(`${API_BASE}/${encodeURIComponent(communityId)}/metrics/public/usage?period=${activePeriod}`),
765765
fetch(`${API_BASE}/sync/status?community_id=${encodeURIComponent(communityId)}`).catch(err => { console.warn('Sync status fetch failed (non-critical):', err.message); return null; }),
766-
fetch(`${API_BASE}/sync/health`).catch(err => { console.warn('Health check fetch failed (non-critical):', err.message); return null; }),
766+
fetch(`${API_BASE}/sync/health?community_id=${encodeURIComponent(communityId)}`).catch(err => { console.warn('Health check fetch failed (non-critical):', err.message); return null; }),
767767
]);
768768

769769
const failedStatus = !summaryResp.ok ? summaryResp.status : (!usageResp.ok ? usageResp.status : null);

src/api/routers/sync.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,12 @@ async def get_sync_status(
269269
matching = {
270270
k: v for k, v in metadata.get("papers", {}).items() if k.startswith(f"{source}:")
271271
}
272-
timestamps = [v["last_sync"] for v in matching.values() if v.get("last_sync")]
273-
last_sync = max(timestamps) if timestamps else None
272+
parsed = [
273+
(dt, raw)
274+
for v in matching.values()
275+
if (raw := v.get("last_sync")) and (dt := _parse_iso_datetime(raw))
276+
]
277+
last_sync = max(parsed, key=lambda x: x[0])[1] if parsed else None
274278
papers_sources[source] = RepoStatus(
275279
items=stats.get(f"papers_{source}", 0),
276280
last_sync=last_sync,

tests/test_api/test_sync.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,23 @@ def test_new_install_is_healthy(self, client: TestClient):
192192
data = response.json()
193193
assert data["status"] == "healthy"
194194

195+
def test_health_unknown_community_returns_404(self, client: TestClient):
196+
"""Unknown community_id should return 404 from health endpoint."""
197+
response = client.get("/sync/health?community_id=does-not-exist")
198+
assert response.status_code == 404
199+
200+
@pytest.mark.usefixtures("isolated_db")
201+
def test_health_known_community_returns_200(self, client: TestClient):
202+
"""A registered community_id should return 200 from health endpoint."""
203+
from src.assistants import registry
204+
205+
communities = registry.list_all()
206+
if not communities:
207+
pytest.skip("No communities registered")
208+
community_id = communities[0].id
209+
response = client.get(f"/sync/health?community_id={community_id}")
210+
assert response.status_code == 200
211+
195212

196213
class TestSyncTrigger:
197214
"""Tests for POST /sync/trigger endpoint."""

0 commit comments

Comments
 (0)