diff --git a/app/api/routers/admin_dbops.py b/app/api/routers/admin_dbops.py index ce63ea2..048be70 100644 --- a/app/api/routers/admin_dbops.py +++ b/app/api/routers/admin_dbops.py @@ -57,7 +57,7 @@ def _requested_by_value(*, payload, admin_user: User) -> str: return from_payload or admin_user.email -def _serialize_pdf_queue_item(item) -> dict[str, object]: +def _serialize_pdf_queue_item(item, *, is_admin: bool = False) -> dict[str, object]: return { "publication_id": item.publication_id, "title": item.title, @@ -69,7 +69,7 @@ def _serialize_pdf_queue_item(item) -> dict[str, object]: "last_failure_detail": item.last_failure_detail, "last_source": item.last_source, "requested_by_user_id": item.requested_by_user_id, - "requested_by_email": item.requested_by_email, + "requested_by_email": item.requested_by_email if is_admin else None, "queued_at": item.queued_at, "last_attempt_at": item.last_attempt_at, "resolved_at": item.resolved_at, @@ -215,7 +215,7 @@ async def get_pdf_queue( return success_payload( request, data={ - "items": [_serialize_pdf_queue_item(item) for item in queue_page.items], + "items": [_serialize_pdf_queue_item(item, is_admin=current_user.is_admin) for item in queue_page.items], **_pdf_queue_page_data( total_count=queue_page.total_count, page=resolved_page, diff --git a/app/api/routers/scholars.py b/app/api/routers/scholars.py index 92782b1..2a0c276 100644 --- a/app/api/routers/scholars.py +++ b/app/api/routers/scholars.py @@ -51,7 +51,14 @@ def _parse_ids_param(ids: str | None) -> list[int] | None: parts = [p.strip() for p in ids.split(",") if p.strip()] if not parts: return None - return [int(p) for p in parts] + try: + return [int(p) for p in parts] + except ValueError as exc: + raise ApiException( + status_code=400, + code="invalid_ids_param", + message="The 'ids' parameter must be a comma-separated list of integers.", + ) from exc @router.get( @@ -104,12 +111,19 @@ async def bulk_delete_scholars( 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, - ) + try: + 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, + ) + except scholar_service.ScholarServiceError as exc: + raise ApiException( + status_code=409, + code="scholar_bulk_delete_failed", + message=str(exc), + ) from exc structured_log( logger, "info", @@ -331,11 +345,18 @@ async def delete_scholar( code="scholar_not_found", message="Scholar not found.", ) - await scholar_service.delete_scholar( - db_session, - profile=profile, - upload_dir=settings.scholar_image_upload_dir, - ) + try: + await scholar_service.delete_scholar( + db_session, + profile=profile, + upload_dir=settings.scholar_image_upload_dir, + ) + except scholar_service.ScholarServiceError as exc: + raise ApiException( + status_code=409, + code="scholar_delete_failed", + message=str(exc), + ) from exc structured_log( logger, "info", "api.scholars.deleted", user_id=current_user.id, scholar_profile_id=scholar_profile_id ) diff --git a/app/services/scholars/application.py b/app/services/scholars/application.py index a19d9de..3496732 100644 --- a/app/services/scholars/application.py +++ b/app/services/scholars/application.py @@ -1,6 +1,7 @@ from __future__ import annotations import os +from typing import Any from uuid import uuid4 from sqlalchemy.exc import IntegrityError @@ -52,7 +53,11 @@ async def bulk_delete_scholars( _safe_remove_upload(upload_root, profile.profile_image_upload_path) for profile in profiles: await db_session.delete(profile) - await db_session.commit() + try: + await db_session.commit() + except IntegrityError as exc: + await db_session.rollback() + raise ScholarServiceError("Unable to bulk-delete scholars due to a database constraint.") from exc return len(profiles) @@ -63,9 +68,9 @@ async def bulk_toggle_scholars( scholar_profile_ids: list[int], is_enabled: bool, ) -> int: - from sqlalchemy import update + from sqlalchemy import CursorResult, update - cursor = await db_session.execute( + cursor: CursorResult[Any] = await db_session.execute( # type: ignore[assignment] update(ScholarProfile) .where( ScholarProfile.id.in_(scholar_profile_ids), @@ -74,7 +79,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__ = [ @@ -177,7 +182,11 @@ async def delete_scholar( _safe_remove_upload(upload_root, profile.profile_image_upload_path) await db_session.delete(profile) - await db_session.commit() + try: + await db_session.commit() + except IntegrityError as exc: + await db_session.rollback() + raise ScholarServiceError("Unable to delete scholar due to a database constraint.") from exc async def hydrate_profile_metadata( 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(() => {