From 6bd2563eb027de76fb8a3ad280f8653eae5b8d6c Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Tue, 3 Mar 2026 23:02:37 +0100 Subject: [PATCH 01/15] feat(frontend): show run and cancel buttons to all users Remove the auth.isAdmin guard from the Start check and Cancel check buttons in the Activity Monitor. Admin-only route links (check history, diagnostics) are unchanged. Co-Authored-By: Claude Sonnet 4.6 --- frontend/src/pages/DashboardPage.vue | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/src/pages/DashboardPage.vue b/frontend/src/pages/DashboardPage.vue index 8623927..c7da221 100644 --- a/frontend/src/pages/DashboardPage.vue +++ b/frontend/src/pages/DashboardPage.vue @@ -404,7 +404,7 @@ watch(
Date: Wed, 4 Mar 2026 02:17:18 +0100 Subject: [PATCH 02/15] feat(settings): hours interval, pdf queue for all users, fix users tab loading - Convert check interval setting from minutes to hours in the UI - Show PDF queue tab to all authenticated users (not admin-only); requeue actions remain admin-only in both backend and frontend - Fix users tab not loading on first visit by removing the AsyncStateGate wrapper that prevented child component refs from being populated before onMounted fired Co-Authored-By: Claude Sonnet 4.6 --- app/api/routers/admin_dbops.py | 6 +-- .../features/settings/SettingsAdminPanel.vue | 48 +++++++------------ .../components/AdminPdfQueueSection.vue | 6 ++- frontend/src/pages/SettingsPage.vue | 29 +++++++---- 4 files changed, 43 insertions(+), 46 deletions(-) diff --git a/app/api/routers/admin_dbops.py b/app/api/routers/admin_dbops.py index 0f4d1f6..ce63ea2 100644 --- a/app/api/routers/admin_dbops.py +++ b/app/api/routers/admin_dbops.py @@ -5,7 +5,7 @@ import logging from fastapi import APIRouter, Depends, Path, Query, Request from sqlalchemy.ext.asyncio import AsyncSession -from app.api.deps import get_api_admin_user +from app.api.deps import get_api_admin_user, get_api_current_user from app.api.errors import ApiException from app.api.responses import success_payload from app.api.schemas import ( @@ -185,7 +185,7 @@ async def get_pdf_queue( offset: int | None = Query(default=None, ge=0), status: str | None = Query(default=None), db_session: AsyncSession = Depends(get_db_session), - admin_user: User = Depends(get_api_admin_user), + current_user: User = Depends(get_api_current_user), ): normalized_status = (status or "").strip().lower() or None resolved_page, resolved_limit, resolved_offset = _resolve_pdf_queue_paging( @@ -204,7 +204,7 @@ async def get_pdf_queue( logger, "info", "api.admin.db.pdf_queue_listed", - admin_user_id=int(admin_user.id), + user_id=int(current_user.id), page=int(resolved_page), page_size=int(resolved_limit), offset=int(resolved_offset), diff --git a/frontend/src/features/settings/SettingsAdminPanel.vue b/frontend/src/features/settings/SettingsAdminPanel.vue index b306d1e..6e7e326 100644 --- a/frontend/src/features/settings/SettingsAdminPanel.vue +++ b/frontend/src/features/settings/SettingsAdminPanel.vue @@ -1,7 +1,7 @@ + + diff --git a/frontend/src/features/scholars/components/scholar-batch-parsing.ts b/frontend/src/features/scholars/components/scholar-batch-parsing.ts new file mode 100644 index 0000000..45baba7 --- /dev/null +++ b/frontend/src/features/scholars/components/scholar-batch-parsing.ts @@ -0,0 +1,76 @@ +const SCHOLAR_ID_PATTERN = /^[a-zA-Z0-9_-]{12}$/; + +export interface ParsedToken { + index: number; + raw: string; + id: string | null; + error: string | null; +} + +export function extractScholarIdFromUrl(token: string): string | null { + const cleaned = token.replace(/\/+$/, "").replace(/#.*$/, ""); + try { + const parsed = new URL(cleaned); + const userParam = parsed.searchParams.get("user"); + if (!userParam) return null; + const decoded = decodeURIComponent(userParam).trim(); + if (SCHOLAR_ID_PATTERN.test(decoded)) return decoded; + return null; + } catch { + return null; + } +} + +export function validateTokenAsId(token: string): string | null { + const trimmed = token.trim(); + if (!trimmed) return "empty input"; + if (/\s/.test(trimmed)) return "contains whitespace"; + if (trimmed.length !== 12) return `must be 12 characters (got ${trimmed.length})`; + if (!/^[a-zA-Z0-9_-]+$/.test(trimmed)) { + return "contains invalid characters (only a-z, A-Z, 0-9, _ and - allowed)"; + } + return null; +} + +export function parseScholarTokens(raw: string): ParsedToken[] { + const tokens = raw.split(/[\s,;]+/).map((v) => v.trim()).filter((v) => v.length > 0); + const results: ParsedToken[] = []; + const seen = new Set(); + + for (let i = 0; i < tokens.length; i++) { + const token = tokens[i]; + + if (SCHOLAR_ID_PATTERN.test(token)) { + if (seen.has(token)) { + results.push({ index: i + 1, raw: token, id: null, error: "duplicate" }); + continue; + } + seen.add(token); + results.push({ index: i + 1, raw: token, id: token, error: null }); + continue; + } + + if (token.includes("scholar.google") || token.startsWith("http")) { + const extracted = extractScholarIdFromUrl(token); + if (extracted) { + if (seen.has(extracted)) { + results.push({ index: i + 1, raw: token, id: null, error: "duplicate" }); + continue; + } + seen.add(extracted); + results.push({ index: i + 1, raw: token, id: extracted, error: null }); + continue; + } + results.push({ index: i + 1, raw: token, id: null, error: "could not extract scholar ID from URL" }); + continue; + } + + const reason = validateTokenAsId(token); + results.push({ index: i + 1, raw: token, id: null, error: reason ?? "invalid scholar ID" }); + } + return results; +} + +export function parseScholarIds(raw: string): string[] { + return parseScholarTokens(raw).filter((t) => t.id !== null).map((t) => t.id!); +} diff --git a/frontend/src/pages/DashboardPage.vue b/frontend/src/pages/DashboardPage.vue index c7da221..3f7df35 100644 --- a/frontend/src/pages/DashboardPage.vue +++ b/frontend/src/pages/DashboardPage.vue @@ -494,6 +494,11 @@ watch(

@@ -527,6 +532,9 @@ watch( {{ formatDate(run.start_dt) }} • {{ run.new_publication_count }} new + diff --git a/frontend/src/pages/ScholarsPage.vue b/frontend/src/pages/ScholarsPage.vue index 16b2a96..29541d9 100644 --- a/frontend/src/pages/ScholarsPage.vue +++ b/frontend/src/pages/ScholarsPage.vue @@ -31,6 +31,7 @@ import ScholarAvatar from "@/features/scholars/components/ScholarAvatar.vue"; import ScholarBatchAdd from "@/features/scholars/components/ScholarBatchAdd.vue"; import ScholarNameSearch from "@/features/scholars/components/ScholarNameSearch.vue"; import ScholarSettingsModal from "@/features/scholars/components/ScholarSettingsModal.vue"; +import ScholarStatusBadges from "@/features/scholars/components/ScholarStatusBadges.vue"; import { ApiRequestError } from "@/lib/api/errors"; import { useRunStatusStore } from "@/stores/run_status"; @@ -505,7 +506,10 @@ watch(
-

{{ scholarLabel(item) }}

+

+ {{ scholarLabel(item) }} + +

Publications Open profile @@ -529,7 +533,10 @@ watch(
- {{ scholarLabel(item) }} + + {{ scholarLabel(item) }} + +
Publications Open profile diff --git a/tests/integration/test_api_scholars.py b/tests/integration/test_api_scholars.py index 0d6f3a0..1fd8ba9 100644 --- a/tests/integration/test_api_scholars.py +++ b/tests/integration/test_api_scholars.py @@ -70,6 +70,110 @@ async def test_api_scholars_crud_flow(db_session: AsyncSession) -> None: assert delete_response.json()["data"]["message"] == "Scholar deleted." +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_api_delete_scholar_returns_404_for_nonexistent(db_session: AsyncSession) -> None: + await insert_user( + db_session, + email="api-delete-404@example.com", + password="api-password", + ) + + client = TestClient(app) + login_user(client, email="api-delete-404@example.com", password="api-password") + headers = api_csrf_headers(client) + + response = client.delete("/api/v1/scholars/999999", headers=headers) + assert response.status_code == 404 + assert response.json()["error"]["code"] == "scholar_not_found" + + +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_api_delete_scholar_cascades_linked_publications(db_session: AsyncSession) -> None: + user_id = await insert_user( + db_session, + email="api-delete-cascade@example.com", + password="api-password", + ) + scholar_result = await db_session.execute( + text( + """ + INSERT INTO scholar_profiles (user_id, scholar_id, display_name, is_enabled) + VALUES (:user_id, :scholar_id, :display_name, true) + RETURNING id + """ + ), + {"user_id": user_id, "scholar_id": "delCASCADE01", "display_name": "Cascade Test"}, + ) + scholar_profile_id = int(scholar_result.scalar_one()) + pub_result = await db_session.execute( + text( + """ + INSERT INTO publications (fingerprint_sha256, title_raw, title_normalized, citation_count) + VALUES (:fingerprint, :title_raw, :title_normalized, 0) + RETURNING id + """ + ), + { + "fingerprint": f"{(user_id + 7000):064x}", + "title_raw": "Cascade Publication", + "title_normalized": "cascadepublication", + }, + ) + publication_id = int(pub_result.scalar_one()) + await db_session.execute( + text( + """ + INSERT INTO scholar_publications (scholar_profile_id, publication_id, is_read) + VALUES (:scholar_profile_id, :publication_id, false) + """ + ), + {"scholar_profile_id": scholar_profile_id, "publication_id": publication_id}, + ) + await db_session.commit() + + client = TestClient(app) + login_user(client, email="api-delete-cascade@example.com", password="api-password") + headers = api_csrf_headers(client) + + delete_response = client.delete(f"/api/v1/scholars/{scholar_profile_id}", headers=headers) + assert delete_response.status_code == 200 + assert delete_response.json()["data"]["message"] == "Scholar deleted." + + link_result = await db_session.execute( + text("SELECT count(*) FROM scholar_publications WHERE scholar_profile_id = :id"), + {"id": scholar_profile_id}, + ) + assert int(link_result.scalar_one()) == 0 + + +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_api_create_scholar_rejects_corrupted_id(db_session: AsyncSession) -> None: + await insert_user( + db_session, + email="api-bad-id@example.com", + password="api-password", + ) + + client = TestClient(app) + login_user(client, email="api-bad-id@example.com", password="api-password") + headers = api_csrf_headers(client) + + for bad_id in ["short", "ABCDEF12345!", "", " ", "AB CD EF1234"]: + response = client.post( + "/api/v1/scholars", + json={"scholar_id": bad_id}, + headers=headers, + ) + assert response.status_code == 400, f"Expected 400 for scholar_id={bad_id!r}" + assert response.json()["error"]["code"] == "invalid_scholar" + + @pytest.mark.integration @pytest.mark.db @pytest.mark.asyncio diff --git a/tests/unit/test_scholar_validators.py b/tests/unit/test_scholar_validators.py index 460b315..185d805 100644 --- a/tests/unit/test_scholar_validators.py +++ b/tests/unit/test_scholar_validators.py @@ -42,6 +42,34 @@ class TestValidateScholarId: with pytest.raises(ScholarServiceError, match="12"): validate_scholar_id("ABCDEF12345!") + def test_rejects_empty_string(self) -> None: + with pytest.raises(ScholarServiceError, match="12"): + validate_scholar_id("") + + def test_rejects_whitespace_only(self) -> None: + with pytest.raises(ScholarServiceError, match="12"): + validate_scholar_id(" ") + + def test_rejects_embedded_spaces(self) -> None: + with pytest.raises(ScholarServiceError, match="12"): + validate_scholar_id("ABCDEF 12345") + + def test_rejects_tab_characters(self) -> None: + with pytest.raises(ScholarServiceError, match="12"): + validate_scholar_id("ABCDEF\t12345") + + def test_rejects_url_as_id(self) -> None: + with pytest.raises(ScholarServiceError, match="12"): + validate_scholar_id("https://scholar.google.com/citations?user=ABCDEF123456") + + def test_rejects_newline_in_id(self) -> None: + with pytest.raises(ScholarServiceError, match="12"): + validate_scholar_id("ABCDEF\n12345") + + def test_rejects_unicode_characters(self) -> None: + with pytest.raises(ScholarServiceError, match="12"): + validate_scholar_id("ABCDEF12345\u00e9") + class TestNormalizeDisplayName: def test_returns_stripped_name(self) -> None: From afbb25d2178ce99d6659ef90ff9d35a2e2900bd7 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 15:32:22 +0100 Subject: [PATCH 06/15] fix(ci): use theme tokens for badges, update pdf-queue test - Replace raw dark: utility variants with state-* theme tokens in ScholarStatusBadges and ScholarSettingsModal - Update integration test to expect 200 for pdf-queue listing (endpoint now uses get_api_current_user, not admin-only) --- .../features/scholars/components/ScholarSettingsModal.vue | 2 +- .../features/scholars/components/ScholarStatusBadges.vue | 8 ++++---- tests/integration/test_api_admin.py | 7 ++++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/frontend/src/features/scholars/components/ScholarSettingsModal.vue b/frontend/src/features/scholars/components/ScholarSettingsModal.vue index 511bf66..77af0ac 100644 --- a/frontend/src/features/scholars/components/ScholarSettingsModal.vue +++ b/frontend/src/features/scholars/components/ScholarSettingsModal.vue @@ -51,7 +51,7 @@ function scholarPublicationsRoute(profile: ScholarProfile): { name: string; quer {{ scholarLabel(scholar) }} Pending

diff --git a/frontend/src/features/scholars/components/ScholarStatusBadges.vue b/frontend/src/features/scholars/components/ScholarStatusBadges.vue index 5460658..86b2cd5 100644 --- a/frontend/src/features/scholars/components/ScholarStatusBadges.vue +++ b/frontend/src/features/scholars/components/ScholarStatusBadges.vue @@ -8,22 +8,22 @@ defineProps<{ diff --git a/tests/integration/test_api_admin.py b/tests/integration/test_api_admin.py index fb63f34..f80a8e3 100644 --- a/tests/integration/test_api_admin.py +++ b/tests/integration/test_api_admin.py @@ -340,10 +340,11 @@ async def test_api_admin_dbops_forbidden_for_non_admin_and_validates_scope(db_se assert forbidden_integrity.status_code == 403 assert forbidden_integrity.json()["error"]["code"] == "forbidden" - forbidden_pdf_queue = client.get("/api/v1/admin/db/pdf-queue") - assert forbidden_pdf_queue.status_code == 403 - assert forbidden_pdf_queue.json()["error"]["code"] == "forbidden" + # PDF queue listing is accessible to all authenticated users (not admin-only). + allowed_pdf_queue = client.get("/api/v1/admin/db/pdf-queue") + assert allowed_pdf_queue.status_code == 200 + # But requeue actions still require admin. forbidden_requeue = client.post( "/api/v1/admin/db/pdf-queue/1/requeue", headers=headers, From 61ac6f206fbfd30cb2deebb572c51b350a248c31 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 15:51:48 +0100 Subject: [PATCH 07/15] fix(test): add Pinia setup and admin user to AdminPdfQueueSection tests The component uses useAuthStore() which requires an active Pinia instance, and the "Queue all" button is admin-only (v-if="auth.isAdmin"). --- .../settings/components/AdminPdfQueueSection.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/src/features/settings/components/AdminPdfQueueSection.test.ts b/frontend/src/features/settings/components/AdminPdfQueueSection.test.ts index bdd0793..c312922 100644 --- a/frontend/src/features/settings/components/AdminPdfQueueSection.test.ts +++ b/frontend/src/features/settings/components/AdminPdfQueueSection.test.ts @@ -1,6 +1,8 @@ // @vitest-environment happy-dom import { describe, expect, it, vi, beforeEach } from "vitest"; import { mount, flushPromises } from "@vue/test-utils"; +import { setActivePinia, createPinia } from "pinia"; +import { useAuthStore } from "@/stores/auth"; import AdminPdfQueueSection from "./AdminPdfQueueSection.vue"; vi.mock("@/features/admin_dbops", () => ({ @@ -36,6 +38,9 @@ function buildQueueItem(overrides: Record = {}) { describe("AdminPdfQueueSection", () => { beforeEach(() => { + setActivePinia(createPinia()); + const auth = useAuthStore(); + auth.$patch({ state: "authenticated", user: { id: 1, email: "admin@test.com", is_admin: true, is_active: true } }); mockedListQueue.mockReset(); }); From 6742eff773d1a5a27e32aacafaedc3313ae9b70e Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 14:03:06 +0100 Subject: [PATCH 08/15] feat(scholars): add multiselect and bulk actions (delete, toggle, export) Add Set-based selection state to ScholarsPage with checkboxes in both table and card views, select-all toggle, and stale-key pruning. Backend: POST /scholars/bulk-delete, POST /scholars/bulk-toggle, and GET /scholars/export?ids= filter. Service functions use user-scoped queries. Structured logging for bulk operations. Frontend: useScholarBulkActions composable extracts selection and bulk action logic. AppSelect-based action dropdown with dynamic options. Delete prompts window.confirm; enable/disable applies immediately. Export downloads filtered JSON. Includes integration tests for all bulk endpoints and frontend unit tests for selection toggle, select-all, and stale pruning. --- app/api/routers/scholars.py | 66 ++++++ app/api/schemas/scholars.py | 27 +++ app/services/portability/exporting.py | 18 +- app/services/scholars/application.py | 51 +++++ .../composables/useScholarBulkActions.test.ts | 112 ++++++++++ .../composables/useScholarBulkActions.ts | 195 ++++++++++++++++++ frontend/src/features/scholars/index.ts | 26 ++- frontend/src/pages/ScholarsPage.vue | 174 +++++++++------- tests/integration/test_api_scholars_bulk.py | 140 +++++++++++++ 9 files changed, 730 insertions(+), 79 deletions(-) create mode 100644 frontend/src/features/scholars/composables/useScholarBulkActions.test.ts create mode 100644 frontend/src/features/scholars/composables/useScholarBulkActions.ts create mode 100644 tests/integration/test_api_scholars_bulk.py diff --git a/app/api/routers/scholars.py b/app/api/routers/scholars.py index ae3c522..85c4b99 100644 --- a/app/api/routers/scholars.py +++ b/app/api/routers/scholars.py @@ -23,6 +23,9 @@ from app.api.schemas import ( DataImportEnvelope, DataImportRequest, MessageEnvelope, + ScholarBulkCountEnvelope, + ScholarBulkIdsRequest, + ScholarBulkToggleRequest, ScholarCreateRequest, ScholarEnvelope, ScholarImageUrlUpdateRequest, @@ -42,6 +45,15 @@ logger = logging.getLogger(__name__) router = APIRouter(prefix="/scholars", tags=["api-scholars"]) +def _parse_ids_param(ids: str | None) -> list[int] | None: + if not ids: + return None + parts = [p.strip() for p in ids.split(",") if p.strip()] + if not parts: + return None + return [int(p) for p in parts] + + @router.get( "", response_model=ScholarsListEnvelope, @@ -69,16 +81,70 @@ async def list_scholars( ) async def export_scholars_and_publications( request: Request, + ids: str | None = Query(None, description="Comma-separated scholar profile IDs to export"), db_session: AsyncSession = Depends(get_db_session), current_user: User = Depends(get_api_current_user), ): + scholar_profile_ids = _parse_ids_param(ids) data = await import_export_service.export_user_data( db_session, user_id=current_user.id, + scholar_profile_ids=scholar_profile_ids, ) return success_payload(request, data=data) +@router.post( + "/bulk-delete", + response_model=ScholarBulkCountEnvelope, +) +async def bulk_delete_scholars( + payload: ScholarBulkIdsRequest, + request: Request, + db_session: AsyncSession = Depends(get_db_session), + current_user: User = Depends(get_api_current_user), +): + deleted_count = await scholar_service.bulk_delete_scholars( + db_session, + user_id=current_user.id, + scholar_profile_ids=payload.scholar_profile_ids, + upload_dir=settings.scholar_image_upload_dir, + ) + structured_log( + logger, "info", "scholars.bulk_delete", + user_id=current_user.id, + requested_ids=payload.scholar_profile_ids, + deleted_count=deleted_count, + ) + return success_payload(request, data={"deleted_count": deleted_count, "updated_count": 0}) + + +@router.post( + "/bulk-toggle", + response_model=ScholarBulkCountEnvelope, +) +async def bulk_toggle_scholars( + payload: ScholarBulkToggleRequest, + request: Request, + db_session: AsyncSession = Depends(get_db_session), + current_user: User = Depends(get_api_current_user), +): + updated_count = await scholar_service.bulk_toggle_scholars( + db_session, + user_id=current_user.id, + scholar_profile_ids=payload.scholar_profile_ids, + is_enabled=payload.is_enabled, + ) + structured_log( + logger, "info", "scholars.bulk_toggle", + user_id=current_user.id, + requested_ids=payload.scholar_profile_ids, + is_enabled=payload.is_enabled, + updated_count=updated_count, + ) + return success_payload(request, data={"deleted_count": 0, "updated_count": updated_count}) + + @router.post( "/import", response_model=DataImportEnvelope, diff --git a/app/api/schemas/scholars.py b/app/api/schemas/scholars.py index 91cc459..bc1035d 100644 --- a/app/api/schemas/scholars.py +++ b/app/api/schemas/scholars.py @@ -126,6 +126,33 @@ class DataExportEnvelope(BaseModel): model_config = ConfigDict(extra="forbid") +class ScholarBulkIdsRequest(BaseModel): + scholar_profile_ids: list[int] = Field(..., min_length=1, max_length=500) + + model_config = ConfigDict(extra="forbid") + + +class ScholarBulkToggleRequest(BaseModel): + scholar_profile_ids: list[int] = Field(..., min_length=1, max_length=500) + is_enabled: bool + + model_config = ConfigDict(extra="forbid") + + +class ScholarBulkCountData(BaseModel): + deleted_count: int = 0 + updated_count: int = 0 + + model_config = ConfigDict(extra="forbid") + + +class ScholarBulkCountEnvelope(BaseModel): + data: ScholarBulkCountData + meta: ApiMeta + + model_config = ConfigDict(extra="forbid") + + class DataImportRequest(BaseModel): schema_version: int | None = None exported_at: str | None = None diff --git a/app/services/portability/exporting.py b/app/services/portability/exporting.py index 0589498..a11eaef 100644 --- a/app/services/portability/exporting.py +++ b/app/services/portability/exporting.py @@ -56,11 +56,14 @@ async def export_user_data( db_session: AsyncSession, *, user_id: int, + scholar_profile_ids: list[int] | None = None, ) -> dict[str, Any]: - scholars_result = await db_session.execute( - select(ScholarProfile).where(ScholarProfile.user_id == user_id).order_by(ScholarProfile.id.asc()) - ) - publication_result = await db_session.execute( + scholar_query = select(ScholarProfile).where(ScholarProfile.user_id == user_id) + if scholar_profile_ids: + scholar_query = scholar_query.where(ScholarProfile.id.in_(scholar_profile_ids)) + scholars_result = await db_session.execute(scholar_query.order_by(ScholarProfile.id.asc())) + + pub_query = ( select( ScholarProfile.scholar_id, Publication.cluster_id, @@ -77,8 +80,13 @@ async def export_user_data( .join(ScholarPublication, ScholarPublication.scholar_profile_id == ScholarProfile.id) .join(Publication, Publication.id == ScholarPublication.publication_id) .where(ScholarProfile.user_id == user_id) - .order_by(ScholarPublication.created_at.desc(), Publication.id.desc()) ) + if scholar_profile_ids: + pub_query = pub_query.where(ScholarProfile.id.in_(scholar_profile_ids)) + publication_result = await db_session.execute( + pub_query.order_by(ScholarPublication.created_at.desc(), Publication.id.desc()) + ) + scholars = [_serialize_export_scholar(profile) for profile in scholars_result.scalars().all()] publications = [_serialize_export_publication(row) for row in publication_result.all()] return { diff --git a/app/services/scholars/application.py b/app/services/scholars/application.py index 75f5e73..9b85819 100644 --- a/app/services/scholars/application.py +++ b/app/services/scholars/application.py @@ -27,10 +27,61 @@ from app.services.scholars.validators import ( validate_scholar_id, ) +async def bulk_delete_scholars( + db_session: AsyncSession, + *, + user_id: int, + scholar_profile_ids: list[int], + upload_dir: str | None = None, +) -> int: + from sqlalchemy import select + + result = await db_session.execute( + select(ScholarProfile).where( + ScholarProfile.id.in_(scholar_profile_ids), + ScholarProfile.user_id == user_id, + ) + ) + profiles = list(result.scalars().all()) + if not profiles: + return 0 + if upload_dir: + upload_root = _ensure_upload_root(upload_dir, create=True) + for profile in profiles: + _safe_remove_upload(upload_root, profile.profile_image_upload_path) + for profile in profiles: + await db_session.delete(profile) + await db_session.commit() + return len(profiles) + + +async def bulk_toggle_scholars( + db_session: AsyncSession, + *, + user_id: int, + scholar_profile_ids: list[int], + is_enabled: bool, +) -> int: + from sqlalchemy import update + + result = await db_session.execute( + update(ScholarProfile) + .where( + ScholarProfile.id.in_(scholar_profile_ids), + ScholarProfile.user_id == user_id, + ) + .values(is_enabled=is_enabled) + ) + await db_session.commit() + return result.rowcount # type: ignore[return-value] + + __all__ = [ "SEARCH_COOLDOWN_REASON", "SEARCH_DISABLED_REASON", "ScholarServiceError", + "bulk_delete_scholars", + "bulk_toggle_scholars", "clear_profile_image_customization", "create_scholar_for_user", "delete_scholar", diff --git a/frontend/src/features/scholars/composables/useScholarBulkActions.test.ts b/frontend/src/features/scholars/composables/useScholarBulkActions.test.ts new file mode 100644 index 0000000..868aad6 --- /dev/null +++ b/frontend/src/features/scholars/composables/useScholarBulkActions.test.ts @@ -0,0 +1,112 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from "vitest"; +import { nextTick, ref } from "vue"; +import { useScholarBulkActions, type ScholarBulkAction } from "./useScholarBulkActions"; +import type { ScholarProfile } from "@/features/scholars"; + +vi.mock("@/features/scholars", () => ({ + bulkDeleteScholars: vi.fn(), + bulkToggleScholars: vi.fn(), + exportScholarData: vi.fn(), +})); + +function makeProfile(id: number, name: string): ScholarProfile { + return { + id, + scholar_id: `scholar_${id}`, + display_name: name, + profile_image_url: null, + profile_image_source: "none", + is_enabled: true, + baseline_completed: false, + last_run_dt: null, + last_run_status: null, + }; +} + +function setup(profiles: ScholarProfile[] = []) { + const visibleScholars = ref(profiles); + const callbacks = { + clearMessages: vi.fn(), + assignError: vi.fn(), + setSuccess: vi.fn(), + reloadScholars: vi.fn(async () => {}), + }; + const bulk = useScholarBulkActions(visibleScholars, callbacks); + return { visibleScholars, callbacks, bulk }; +} + +describe("useScholarBulkActions", () => { + it("starts with empty selection", () => { + const { bulk } = setup([makeProfile(1, "Alice")]); + expect(bulk.selectedIds.value.size).toBe(0); + expect(bulk.hasSelection.value).toBe(false); + }); + + it("toggles individual row selection", () => { + const { bulk } = setup([makeProfile(1, "Alice"), makeProfile(2, "Bob")]); + bulk.onToggleRow(1, { target: { checked: true } } as unknown as Event); + expect(bulk.selectedIds.value.has(1)).toBe(true); + expect(bulk.selectedCount.value).toBe(1); + + bulk.onToggleRow(1, { target: { checked: false } } as unknown as Event); + expect(bulk.selectedIds.value.has(1)).toBe(false); + expect(bulk.selectedCount.value).toBe(0); + }); + + it("toggles all visible scholars", () => { + const { bulk } = setup([makeProfile(1, "Alice"), makeProfile(2, "Bob")]); + bulk.onToggleAll({ target: { checked: true } } as unknown as Event); + expect(bulk.selectedIds.value.size).toBe(2); + expect(bulk.allVisibleSelected.value).toBe(true); + + bulk.onToggleAll({ target: { checked: false } } as unknown as Event); + expect(bulk.selectedIds.value.size).toBe(0); + }); + + it("prunes stale selections when list changes", async () => { + const { visibleScholars, bulk } = setup([makeProfile(1, "Alice"), makeProfile(2, "Bob")]); + bulk.onToggleAll({ target: { checked: true } } as unknown as Event); + expect(bulk.selectedIds.value.size).toBe(2); + + // Remove Bob from the visible list + visibleScholars.value = [makeProfile(1, "Alice")]; + await nextTick(); + + expect(bulk.selectedIds.value.size).toBe(1); + expect(bulk.selectedIds.value.has(1)).toBe(true); + expect(bulk.selectedIds.value.has(2)).toBe(false); + }); + + it("shows correct bulk action options with and without selection", () => { + const { bulk } = setup([makeProfile(1, "Alice")]); + // Without selection + expect(bulk.bulkActionOptions.value.length).toBe(1); + expect(bulk.bulkActionOptions.value[0].value).toBe("select_all"); + + // With selection + bulk.onToggleRow(1, { target: { checked: true } } as unknown as Event); + expect(bulk.bulkActionOptions.value.length).toBe(5); + const values = bulk.bulkActionOptions.value.map((o) => o.value); + expect(values).toContain("delete_selected"); + expect(values).toContain("enable_selected"); + expect(values).toContain("disable_selected"); + expect(values).toContain("export_selected"); + expect(values).toContain("clear_selection"); + }); + + it("select all action selects all visible", async () => { + const { bulk } = setup([makeProfile(1, "Alice"), makeProfile(2, "Bob")]); + bulk.bulkAction.value = "select_all" as ScholarBulkAction; + await bulk.onApplyBulkAction(); + expect(bulk.selectedIds.value.size).toBe(2); + }); + + it("clear selection action clears all", async () => { + const { bulk } = setup([makeProfile(1, "Alice")]); + bulk.onToggleRow(1, { target: { checked: true } } as unknown as Event); + bulk.bulkAction.value = "clear_selection" as ScholarBulkAction; + await bulk.onApplyBulkAction(); + expect(bulk.selectedIds.value.size).toBe(0); + }); +}); diff --git a/frontend/src/features/scholars/composables/useScholarBulkActions.ts b/frontend/src/features/scholars/composables/useScholarBulkActions.ts new file mode 100644 index 0000000..f4910a7 --- /dev/null +++ b/frontend/src/features/scholars/composables/useScholarBulkActions.ts @@ -0,0 +1,195 @@ +import { computed, ref, watch, type Ref } from "vue"; + +import { + bulkDeleteScholars, + bulkToggleScholars, + exportScholarData, + type ScholarProfile, +} from "@/features/scholars"; +import { ApiRequestError } from "@/lib/api/errors"; + +export type ScholarBulkAction = + | "delete_selected" + | "enable_selected" + | "disable_selected" + | "export_selected" + | "clear_selection" + | "select_all"; + +export interface ScholarBulkActionOption { + value: ScholarBulkAction; + label: string; +} + +export interface BulkActionCallbacks { + clearMessages: () => void; + assignError: (error: unknown, fallback: string) => void; + setSuccess: (msg: string) => void; + reloadScholars: () => Promise; +} + +export function useScholarBulkActions( + visibleScholars: Ref, + callbacks: BulkActionCallbacks, +) { + const selectedIds = ref>(new Set()); + const bulkAction = ref("select_all"); + const bulkBusy = ref(false); + + const selectedCount = computed(() => selectedIds.value.size); + const hasSelection = computed(() => selectedCount.value > 0); + + const allVisibleSelected = computed(() => { + if (visibleScholars.value.length === 0) return false; + for (const item of visibleScholars.value) { + if (!selectedIds.value.has(item.id)) return false; + } + return true; + }); + + const bulkActionOptions = computed(() => { + if (!hasSelection.value) return [{ value: "select_all", label: "Select all" }]; + const n = selectedCount.value; + return [ + { value: "delete_selected", label: `Delete selected (${n})` }, + { value: "enable_selected", label: `Enable selected (${n})` }, + { value: "disable_selected", label: `Disable selected (${n})` }, + { value: "export_selected", label: `Export selected (${n})` }, + { value: "clear_selection", label: "Clear selection" }, + ]; + }); + + const bulkApplyLabel = computed(() => { + if (bulkBusy.value) return "Applying..."; + if (bulkAction.value === "select_all") return "Select"; + if (bulkAction.value === "clear_selection") return "Clear"; + return "Apply"; + }); + + const bulkApplyDisabled = computed(() => { + if (bulkBusy.value) return true; + if (bulkAction.value === "select_all") return visibleScholars.value.length === 0; + return selectedCount.value === 0; + }); + + // Prune stale selections when visible list changes + watch(visibleScholars, (items) => { + const validIds = new Set(items.map((item) => item.id)); + const next = new Set(); + for (const id of selectedIds.value) { + if (validIds.has(id)) next.add(id); + } + if (next.size !== selectedIds.value.size) selectedIds.value = next; + }); + + // Reset bulk action dropdown when selection state changes + watch(hasSelection, (has) => { + const validValues = new Set(bulkActionOptions.value.map((o) => o.value)); + if (validValues.has(bulkAction.value)) return; + bulkAction.value = has ? "delete_selected" : "select_all"; + }); + + function onToggleAll(event: Event): void { + const checked = (event.target as HTMLInputElement).checked; + const next = new Set(selectedIds.value); + for (const item of visibleScholars.value) { + if (checked) { next.add(item.id); } else { next.delete(item.id); } + } + selectedIds.value = next; + } + + function onToggleRow(id: number, event: Event): void { + const checked = (event.target as HTMLInputElement).checked; + const next = new Set(selectedIds.value); + if (checked) { next.add(id); } else { next.delete(id); } + selectedIds.value = next; + } + + function downloadJsonFile(filename: string, payload: unknown): void { + const blob = new Blob([JSON.stringify(payload, null, 2)], { type: "application/json" }); + const url = URL.createObjectURL(blob); + const anchor = document.createElement("a"); + anchor.href = url; + anchor.download = filename; + anchor.click(); + URL.revokeObjectURL(url); + } + + async function onApplyBulkAction(): Promise { + if (bulkApplyDisabled.value) return; + if (bulkAction.value === "select_all") { + selectedIds.value = new Set(visibleScholars.value.map((item) => item.id)); + return; + } + if (bulkAction.value === "clear_selection") { selectedIds.value = new Set(); return; } + if (bulkAction.value === "delete_selected") { await onBulkDelete(); return; } + if (bulkAction.value === "enable_selected") { await onBulkToggle(true); return; } + if (bulkAction.value === "disable_selected") { await onBulkToggle(false); return; } + if (bulkAction.value === "export_selected") { await onBulkExport(); return; } + } + + async function onBulkDelete(): Promise { + const ids = [...selectedIds.value]; + if (!window.confirm(`Delete ${ids.length} scholar(s)? This removes all linked publications and queue data.`)) return; + bulkBusy.value = true; + callbacks.clearMessages(); + try { + const result = await bulkDeleteScholars(ids); + callbacks.setSuccess(`${result.deleted_count} scholar(s) deleted.`); + selectedIds.value = new Set(); + await callbacks.reloadScholars(); + } catch (error) { + callbacks.assignError(error, "Unable to bulk delete scholars."); + } finally { + bulkBusy.value = false; + } + } + + async function onBulkToggle(isEnabled: boolean): Promise { + bulkBusy.value = true; + callbacks.clearMessages(); + try { + const ids = [...selectedIds.value]; + const result = await bulkToggleScholars(ids, isEnabled); + const verb = isEnabled ? "enabled" : "disabled"; + callbacks.setSuccess(`${result.updated_count} scholar(s) ${verb}.`); + selectedIds.value = new Set(); + await callbacks.reloadScholars(); + } catch (error) { + callbacks.assignError(error, `Unable to bulk ${isEnabled ? "enable" : "disable"} scholars.`); + } finally { + bulkBusy.value = false; + } + } + + async function onBulkExport(): Promise { + bulkBusy.value = true; + callbacks.clearMessages(); + try { + const ids = [...selectedIds.value]; + const payload = await exportScholarData(ids); + const dateSlug = payload.exported_at.slice(0, 10) || "unknown-date"; + downloadJsonFile(`scholarr-export-${dateSlug}.json`, payload); + callbacks.setSuccess("Export complete."); + } catch (error) { + callbacks.assignError(error, "Unable to export selected scholars."); + } finally { + bulkBusy.value = false; + } + } + + return { + selectedIds, + selectedCount, + hasSelection, + allVisibleSelected, + bulkAction, + bulkBusy, + bulkActionOptions, + bulkApplyLabel, + bulkApplyDisabled, + onToggleAll, + onToggleRow, + onApplyBulkAction, + }; +} diff --git a/frontend/src/features/scholars/index.ts b/frontend/src/features/scholars/index.ts index ef3c8c4..fedc71b 100644 --- a/frontend/src/features/scholars/index.ts +++ b/frontend/src/features/scholars/index.ts @@ -160,8 +160,30 @@ export async function clearScholarImage( return response.data; } -export async function exportScholarData(): Promise { - const response = await apiRequest("/scholars/export", { +export interface BulkCountResult { + deleted_count: number; + updated_count: number; +} + +export async function bulkDeleteScholars(scholarProfileIds: number[]): Promise { + const response = await apiRequest("/scholars/bulk-delete", { + method: "POST", + body: { scholar_profile_ids: scholarProfileIds }, + }); + return response.data; +} + +export async function bulkToggleScholars(scholarProfileIds: number[], isEnabled: boolean): Promise { + const response = await apiRequest("/scholars/bulk-toggle", { + method: "POST", + body: { scholar_profile_ids: scholarProfileIds, is_enabled: isEnabled }, + }); + return response.data; +} + +export async function exportScholarData(ids?: number[]): Promise { + const params = ids && ids.length > 0 ? `?ids=${ids.join(",")}` : ""; + const response = await apiRequest(`/scholars/export${params}`, { method: "GET", }); return response.data; diff --git a/frontend/src/pages/ScholarsPage.vue b/frontend/src/pages/ScholarsPage.vue index 29541d9..c962242 100644 --- a/frontend/src/pages/ScholarsPage.vue +++ b/frontend/src/pages/ScholarsPage.vue @@ -27,6 +27,7 @@ import { type ScholarProfile, type ScholarSearchCandidate, } from "@/features/scholars"; +import { useScholarBulkActions } from "@/features/scholars/composables/useScholarBulkActions"; import ScholarAvatar from "@/features/scholars/components/ScholarAvatar.vue"; import ScholarBatchAdd from "@/features/scholars/components/ScholarBatchAdd.vue"; import ScholarNameSearch from "@/features/scholars/components/ScholarNameSearch.vue"; @@ -291,82 +292,44 @@ async function onDeleteScholar(): Promise { } async function onSaveImageUrl(): Promise { - const profile = activeScholarSettings.value; - if (!profile) return; - const candidate = (imageUrlDraftByScholarId.value[profile.id] || "").trim(); - if (!candidate) { errorMessage.value = "Enter an image URL before saving, or use Reset image."; return; } - imageSavingScholarId.value = profile.id; + const p = activeScholarSettings.value; + if (!p) return; + const url = (imageUrlDraftByScholarId.value[p.id] || "").trim(); + if (!url) { errorMessage.value = "Enter an image URL before saving, or use Reset image."; return; } + imageSavingScholarId.value = p.id; clearMessages(); - try { - await setScholarImageUrl(profile.id, candidate); - successMessage.value = `Image URL updated for ${scholarLabel(profile)}.`; - await loadScholars(); - } catch (error) { - assignError(error, "Unable to update scholar image URL."); - } finally { - imageSavingScholarId.value = null; - } + try { await setScholarImageUrl(p.id, url); successMessage.value = `Image URL updated for ${scholarLabel(p)}.`; await loadScholars(); } + catch (e) { assignError(e, "Unable to update scholar image URL."); } + finally { imageSavingScholarId.value = null; } } async function onUploadImage(event: Event): Promise { - const profile = activeScholarSettings.value; - if (!profile) return; + const p = activeScholarSettings.value; + if (!p) return; const input = event.target as HTMLInputElement | null; const file = input?.files?.[0] ?? null; if (!file) return; - imageUploadingScholarId.value = profile.id; + imageUploadingScholarId.value = p.id; clearMessages(); - try { - await uploadScholarImage(profile.id, file); - successMessage.value = `Uploaded image for ${scholarLabel(profile)}.`; - await loadScholars(); - } catch (error) { - assignError(error, "Unable to upload scholar image."); - } finally { - imageUploadingScholarId.value = null; - if (input) input.value = ""; - } + try { await uploadScholarImage(p.id, file); successMessage.value = `Uploaded image for ${scholarLabel(p)}.`; await loadScholars(); } + catch (e) { assignError(e, "Unable to upload scholar image."); } + finally { imageUploadingScholarId.value = null; if (input) input.value = ""; } } async function onResetImage(): Promise { - const profile = activeScholarSettings.value; - if (!profile) return; - imageSavingScholarId.value = profile.id; + const p = activeScholarSettings.value; + if (!p) return; + imageSavingScholarId.value = p.id; clearMessages(); - try { - await clearScholarImage(profile.id); - successMessage.value = `Image reset for ${scholarLabel(profile)}.`; - await loadScholars(); - } catch (error) { - assignError(error, "Unable to reset scholar image."); - } finally { - imageSavingScholarId.value = null; - } + try { await clearScholarImage(p.id); successMessage.value = `Image reset for ${scholarLabel(p)}.`; await loadScholars(); } + catch (e) { assignError(e, "Unable to reset scholar image."); } + finally { imageSavingScholarId.value = null; } } // --- Import/export --- -function suggestExportFilename(exportedAt: string): string { - return `scholarr-export-${exportedAt.slice(0, 10) || "unknown-date"}.json`; -} - -function downloadJsonFile(filename: string, payload: unknown): void { - const blob = new Blob([JSON.stringify(payload, null, 2)], { type: "application/json" }); - const url = URL.createObjectURL(blob); - const anchor = document.createElement("a"); - anchor.href = url; - anchor.download = filename; - anchor.click(); - URL.revokeObjectURL(url); -} - -function importSummary(result: DataImportResult): string { - return ( - `Import complete. Scholars +${result.scholars_created}` + - ` / updated ${result.scholars_updated}; publications +${result.publications_created}` + - ` / updated ${result.publications_updated}; links +${result.links_created}` + - ` / updated ${result.links_updated}; skipped ${result.skipped_records}.` - ); +function importSummary(r: DataImportResult): string { + return `Import complete. Scholars +${r.scholars_created} / updated ${r.scholars_updated}; publications +${r.publications_created} / updated ${r.publications_updated}; links +${r.links_created} / updated ${r.links_updated}; skipped ${r.skipped_records}.`; } async function onExportData(): Promise { @@ -374,7 +337,13 @@ async function onExportData(): Promise { clearMessages(); try { const payload = await exportScholarData(); - downloadJsonFile(suggestExportFilename(payload.exported_at), payload); + const blob = new Blob([JSON.stringify(payload, null, 2)], { type: "application/json" }); + const url = URL.createObjectURL(blob); + const a = document.createElement("a"); + a.href = url; + a.download = `scholarr-export-${payload.exported_at.slice(0, 10) || "unknown-date"}.json`; + a.click(); + URL.revokeObjectURL(url); successMessage.value = "Export complete."; } catch (error) { assignError(error, "Unable to export scholars and publications."); @@ -383,10 +352,6 @@ async function onExportData(): Promise { } } -function onOpenImportPicker(): void { - importFileInput.value?.click(); -} - async function onImportFileSelected(event: Event): Promise { const input = event.target as HTMLInputElement | null; const file = input?.files?.[0] ?? null; @@ -396,16 +361,12 @@ async function onImportFileSelected(event: Event): Promise { try { const raw = await file.text(); let parsed = JSON.parse(raw); - // Accept the full export envelope (with data/meta wrapper) - if (parsed?.data && Array.isArray(parsed.data.scholars)) { - parsed = parsed.data; - } + if (parsed?.data && Array.isArray(parsed.data.scholars)) parsed = parsed.data; const payload = parsed as DataImportPayload; if (!payload || !Array.isArray(payload.scholars) || !Array.isArray(payload.publications)) { throw new Error("Invalid import file: expected scholars[] and publications[] arrays."); } - const result = await importScholarData(payload); - successMessage.value = importSummary(result); + successMessage.value = importSummary(await importScholarData(payload)); await loadScholars(); } catch (error) { assignError(error, "Unable to import scholars and publications."); @@ -415,6 +376,15 @@ async function onImportFileSelected(event: Event): Promise { } } +// --- Bulk actions --- + +const bulk = useScholarBulkActions(visibleScholars, { + clearMessages, + assignError, + setSuccess: (msg: string) => { successMessage.value = msg; }, + reloadScholars: loadScholars, +}); + // --- Lifecycle --- onMounted(() => { void loadScholars(); }); @@ -472,7 +442,7 @@ watch( {{ exportingData ? "Exporting..." : "Export" }} - + {{ importingData ? "Importing..." : "Import" }} @@ -497,6 +467,34 @@ watch(
+
+ + {{ trackedCountLabel }} + + +
+ + + + + + {{ bulk.bulkApplyLabel.value }} + +
+
+
@@ -504,6 +502,13 @@ watch(
  • +

    @@ -523,12 +528,31 @@ watch( + + + Scholar Manage + + +

    @@ -575,3 +599,9 @@ watch( /> + + diff --git a/tests/integration/test_api_scholars_bulk.py b/tests/integration/test_api_scholars_bulk.py new file mode 100644 index 0000000..a481c32 --- /dev/null +++ b/tests/integration/test_api_scholars_bulk.py @@ -0,0 +1,140 @@ +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy.ext.asyncio import AsyncSession + +from app.main import app +from tests.integration.helpers import ( + api_csrf_headers, + insert_user, + login_user, +) + + +def _create_scholar(client: TestClient, headers: dict, scholar_id: str) -> int: + resp = client.post("/api/v1/scholars", json={"scholar_id": scholar_id}, headers=headers) + assert resp.status_code == 201 + return int(resp.json()["data"]["id"]) + + +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_bulk_delete_with_valid_ids(db_session: AsyncSession) -> None: + await insert_user(db_session, email="bulk@example.com", password="pw123456") + client = TestClient(app) + login_user(client, email="bulk@example.com", password="pw123456") + headers = api_csrf_headers(client) + + id1 = _create_scholar(client, headers, "aaaBBB111222") + id2 = _create_scholar(client, headers, "cccDDD333444") + + resp = client.post( + "/api/v1/scholars/bulk-delete", + json={"scholar_profile_ids": [id1, id2]}, + headers=headers, + ) + assert resp.status_code == 200 + assert resp.json()["data"]["deleted_count"] == 2 + + list_resp = client.get("/api/v1/scholars") + assert len(list_resp.json()["data"]["scholars"]) == 0 + + +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_bulk_delete_only_deletes_own_scholars(db_session: AsyncSession) -> None: + await insert_user(db_session, email="user1@example.com", password="pw123456") + await insert_user(db_session, email="user2@example.com", password="pw123456") + + client1 = TestClient(app) + login_user(client1, email="user1@example.com", password="pw123456") + headers1 = api_csrf_headers(client1) + + client2 = TestClient(app) + login_user(client2, email="user2@example.com", password="pw123456") + headers2 = api_csrf_headers(client2) + + id_user1 = _create_scholar(client1, headers1, "aaaBBB111222") + id_user2 = _create_scholar(client2, headers2, "cccDDD333444") + + # User1 tries to delete both — should only delete own + resp = client1.post( + "/api/v1/scholars/bulk-delete", + json={"scholar_profile_ids": [id_user1, id_user2]}, + headers=headers1, + ) + assert resp.status_code == 200 + assert resp.json()["data"]["deleted_count"] == 1 + + # User2's scholar still exists + list_resp = client2.get("/api/v1/scholars") + scholars = list_resp.json()["data"]["scholars"] + assert len(scholars) == 1 + assert int(scholars[0]["id"]) == id_user2 + + +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_bulk_toggle_enables_and_disables(db_session: AsyncSession) -> None: + await insert_user(db_session, email="toggle@example.com", password="pw123456") + client = TestClient(app) + login_user(client, email="toggle@example.com", password="pw123456") + headers = api_csrf_headers(client) + + id1 = _create_scholar(client, headers, "aaaBBB111222") + id2 = _create_scholar(client, headers, "cccDDD333444") + + # Disable both + resp = client.post( + "/api/v1/scholars/bulk-toggle", + json={"scholar_profile_ids": [id1, id2], "is_enabled": False}, + headers=headers, + ) + assert resp.status_code == 200 + assert resp.json()["data"]["updated_count"] == 2 + + scholars = client.get("/api/v1/scholars").json()["data"]["scholars"] + for s in scholars: + assert s["is_enabled"] is False + + # Re-enable both + resp = client.post( + "/api/v1/scholars/bulk-toggle", + json={"scholar_profile_ids": [id1, id2], "is_enabled": True}, + headers=headers, + ) + assert resp.status_code == 200 + assert resp.json()["data"]["updated_count"] == 2 + + scholars = client.get("/api/v1/scholars").json()["data"]["scholars"] + for s in scholars: + assert s["is_enabled"] is True + + +@pytest.mark.integration +@pytest.mark.db +@pytest.mark.asyncio +async def test_export_with_ids_filter(db_session: AsyncSession) -> None: + await insert_user(db_session, email="export@example.com", password="pw123456") + client = TestClient(app) + login_user(client, email="export@example.com", password="pw123456") + headers = api_csrf_headers(client) + + id1 = _create_scholar(client, headers, "aaaBBB111222") + _create_scholar(client, headers, "cccDDD333444") + + # Export only id1 + resp = client.get(f"/api/v1/scholars/export?ids={id1}") + assert resp.status_code == 200 + data = resp.json()["data"] + assert len(data["scholars"]) == 1 + assert data["scholars"][0]["scholar_id"] == "aaaBBB111222" + + # Export all (no filter) + resp_all = client.get("/api/v1/scholars/export") + assert resp_all.status_code == 200 + assert len(resp_all.json()["data"]["scholars"]) == 2 From 76c4811405a2355ac53e0ebe1273b84ab43ed7c6 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 15:40:49 +0100 Subject: [PATCH 09/15] fix(ci): fix import sorting, API contract, and pdf-queue test - Add missing blank line after imports in application.py (ruff I001) - Use full path string instead of template literal for export API call to avoid false positive in API contract drift check - Update pdf-queue test to expect 200 for non-admin listing --- app/services/scholars/application.py | 1 + frontend/src/features/scholars/index.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/services/scholars/application.py b/app/services/scholars/application.py index 9b85819..b4b77ab 100644 --- a/app/services/scholars/application.py +++ b/app/services/scholars/application.py @@ -27,6 +27,7 @@ from app.services.scholars.validators import ( validate_scholar_id, ) + async def bulk_delete_scholars( db_session: AsyncSession, *, diff --git a/frontend/src/features/scholars/index.ts b/frontend/src/features/scholars/index.ts index fedc71b..f6ad9d3 100644 --- a/frontend/src/features/scholars/index.ts +++ b/frontend/src/features/scholars/index.ts @@ -182,8 +182,8 @@ export async function bulkToggleScholars(scholarProfileIds: number[], isEnabled: } export async function exportScholarData(ids?: number[]): Promise { - const params = ids && ids.length > 0 ? `?ids=${ids.join(",")}` : ""; - const response = await apiRequest(`/scholars/export${params}`, { + const path = ids && ids.length > 0 ? `/scholars/export?ids=${ids.join(",")}` : "/scholars/export"; + const response = await apiRequest(path, { method: "GET", }); return response.data; From 6f3e0eec39062d0df4c25186f4cf5cbdd8361309 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 15:56:40 +0100 Subject: [PATCH 10/15] fix(ci): fix ruff format in scholars router Put each structured_log argument on its own line. --- app/api/routers/scholars.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/api/routers/scholars.py b/app/api/routers/scholars.py index 85c4b99..bca0d63 100644 --- a/app/api/routers/scholars.py +++ b/app/api/routers/scholars.py @@ -111,7 +111,9 @@ async def bulk_delete_scholars( upload_dir=settings.scholar_image_upload_dir, ) structured_log( - logger, "info", "scholars.bulk_delete", + logger, + "info", + "scholars.bulk_delete", user_id=current_user.id, requested_ids=payload.scholar_profile_ids, deleted_count=deleted_count, @@ -136,7 +138,9 @@ async def bulk_toggle_scholars( is_enabled=payload.is_enabled, ) structured_log( - logger, "info", "scholars.bulk_toggle", + logger, + "info", + "scholars.bulk_toggle", user_id=current_user.id, requested_ids=payload.scholar_profile_ids, is_enabled=payload.is_enabled, From 3e933998a0f9d0d5cd9dda1989837cd2299e7498 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 15:59:31 +0100 Subject: [PATCH 11/15] fix(ci): fix mypy error in bulk_toggle_scholars Use int(cursor.rowcount) instead of type: ignore comment. --- app/services/scholars/application.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/scholars/application.py b/app/services/scholars/application.py index b4b77ab..fcd657c 100644 --- a/app/services/scholars/application.py +++ b/app/services/scholars/application.py @@ -65,7 +65,7 @@ async def bulk_toggle_scholars( ) -> int: from sqlalchemy import update - result = await db_session.execute( + cursor = await db_session.execute( update(ScholarProfile) .where( ScholarProfile.id.in_(scholar_profile_ids), @@ -74,7 +74,7 @@ async def bulk_toggle_scholars( .values(is_enabled=is_enabled) ) await db_session.commit() - return result.rowcount # type: ignore[return-value] + return int(cursor.rowcount) __all__ = [ From 39c7eb686cbf4b5f93f42dfe1f590072655ab33a Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 16:16:55 +0100 Subject: [PATCH 12/15] fix(mypy): use CursorResult type annotation for bulk_toggle_scholars --- app/services/scholars/application.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/services/scholars/application.py b/app/services/scholars/application.py index fcd657c..07cf6b3 100644 --- a/app/services/scholars/application.py +++ b/app/services/scholars/application.py @@ -63,9 +63,11 @@ async def bulk_toggle_scholars( scholar_profile_ids: list[int], is_enabled: bool, ) -> int: - from sqlalchemy import update + from typing import Any - cursor = await db_session.execute( + from sqlalchemy import CursorResult, update + + cursor: CursorResult[Any] = await db_session.execute( # type: ignore[assignment] update(ScholarProfile) .where( ScholarProfile.id.in_(scholar_profile_ids), @@ -74,7 +76,7 @@ async def bulk_toggle_scholars( .values(is_enabled=is_enabled) ) await db_session.commit() - return int(cursor.rowcount) + return int(cursor.rowcount or 0) __all__ = [ From c85fa08b7b5db2d5ccce72cb65c6554b5aabf948 Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Sat, 7 Mar 2026 14:37:18 +0100 Subject: [PATCH 13/15] fix(frontend): add cooldown tooltips and fix settings tab initial load Add AppHelpHint tooltips to ScrapeSafetyBadge and ScrapeSafetyBanner showing rate-limit explanation, cooldown reason, and recommended action. When no cooldown is active, show a simple "ready" message. Fix settings tabs not loading on initial navigation by deferring onMounted load via nextTick, and using flush:'post' on the section watcher so template refs are attached before load() runs. --- .../patterns/ScrapeSafetyBadge.test.ts | 61 ++++++++++++ .../components/patterns/ScrapeSafetyBadge.vue | 25 ++++- .../patterns/ScrapeSafetyBanner.vue | 12 ++- .../settings/SettingsAdminPanel.test.ts | 94 +++++++++++++++++++ .../features/settings/SettingsAdminPanel.vue | 8 +- 5 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 frontend/src/components/patterns/ScrapeSafetyBadge.test.ts create mode 100644 frontend/src/features/settings/SettingsAdminPanel.test.ts diff --git a/frontend/src/components/patterns/ScrapeSafetyBadge.test.ts b/frontend/src/components/patterns/ScrapeSafetyBadge.test.ts new file mode 100644 index 0000000..8d17969 --- /dev/null +++ b/frontend/src/components/patterns/ScrapeSafetyBadge.test.ts @@ -0,0 +1,61 @@ +// @vitest-environment happy-dom +import { describe, expect, it } from "vitest"; +import { mount } from "@vue/test-utils"; +import ScrapeSafetyBadge from "./ScrapeSafetyBadge.vue"; +import { createDefaultSafetyState, type ScrapeSafetyState } from "@/features/safety"; + +function buildState(overrides: Partial = {}): ScrapeSafetyState { + return { ...createDefaultSafetyState(), ...overrides }; +} + +describe("ScrapeSafetyBadge", () => { + it("shows ready tooltip when cooldown is inactive", () => { + const wrapper = mount(ScrapeSafetyBadge, { + props: { state: buildState({ cooldown_active: false }) }, + }); + expect(wrapper.text()).toContain("Safety ready"); + const hint = wrapper.findComponent({ name: "AppHelpHint" }); + expect(hint.exists()).toBe(true); + expect(hint.props("text")).toContain("No active cooldown"); + }); + + it("shows cooldown tooltip with reason and action when active", () => { + const wrapper = mount(ScrapeSafetyBadge, { + props: { + state: buildState({ + cooldown_active: true, + cooldown_reason: "blocked_failure_threshold_exceeded", + cooldown_reason_label: "Too many blocked requests", + recommended_action: "Wait for cooldown to expire", + cooldown_remaining_seconds: 120, + }), + }, + }); + expect(wrapper.text()).toContain("Safety cooldown"); + const hint = wrapper.findComponent({ name: "AppHelpHint" }); + expect(hint.exists()).toBe(true); + const text = hint.props("text") as string; + expect(text).toContain("Google Scholar rate-limits"); + expect(text).toContain("Too many blocked requests"); + expect(text).toContain("Wait for cooldown to expire"); + }); + + it("shows cooldown tooltip without optional fields", () => { + const wrapper = mount(ScrapeSafetyBadge, { + props: { + state: buildState({ + cooldown_active: true, + cooldown_reason: "some_reason", + cooldown_reason_label: null, + recommended_action: null, + cooldown_remaining_seconds: 60, + }), + }, + }); + const hint = wrapper.findComponent({ name: "AppHelpHint" }); + const text = hint.props("text") as string; + expect(text).toContain("Google Scholar rate-limits"); + expect(text).not.toContain("Why:"); + expect(text).not.toContain("Action:"); + }); +}); diff --git a/frontend/src/components/patterns/ScrapeSafetyBadge.vue b/frontend/src/components/patterns/ScrapeSafetyBadge.vue index b40c1c9..845fb2f 100644 --- a/frontend/src/components/patterns/ScrapeSafetyBadge.vue +++ b/frontend/src/components/patterns/ScrapeSafetyBadge.vue @@ -1,6 +1,7 @@ diff --git a/frontend/src/components/patterns/ScrapeSafetyBanner.vue b/frontend/src/components/patterns/ScrapeSafetyBanner.vue index 300b05a..6ea343d 100644 --- a/frontend/src/components/patterns/ScrapeSafetyBanner.vue +++ b/frontend/src/components/patterns/ScrapeSafetyBanner.vue @@ -2,6 +2,7 @@ import { computed, onBeforeUnmount, onMounted, ref } from "vue"; import AppAlert from "@/components/ui/AppAlert.vue"; +import AppHelpHint from "@/components/ui/AppHelpHint.vue"; import { formatCooldownCountdown, type ScrapeSafetyState, @@ -79,6 +80,10 @@ const actionText = computed(() => { return props.safetyState.recommended_action; }); +const bannerHintText = computed(() => { + return "Google Scholar rate-limits automated requests. The cooldown pauses scraping to avoid your IP being blocked."; +}); + onMounted(() => { timer = setInterval(() => { now.value = Date.now(); @@ -95,7 +100,12 @@ onBeforeUnmount(() => {