fix(scholars): improve validation, fix delete, add status badges #48

Merged
JustinZeus merged 5 commits from fix/scholar-validation-delete-refinement into main 2026-03-07 16:11:07 +01:00
17 changed files with 512 additions and 94 deletions

View file

@ -5,7 +5,7 @@ import logging
from fastapi import APIRouter, Depends, Path, Query, Request from fastapi import APIRouter, Depends, Path, Query, Request
from sqlalchemy.ext.asyncio import AsyncSession 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.errors import ApiException
from app.api.responses import success_payload from app.api.responses import success_payload
from app.api.schemas import ( from app.api.schemas import (
@ -185,7 +185,7 @@ async def get_pdf_queue(
offset: int | None = Query(default=None, ge=0), offset: int | None = Query(default=None, ge=0),
status: str | None = Query(default=None), status: str | None = Query(default=None),
db_session: AsyncSession = Depends(get_db_session), 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 normalized_status = (status or "").strip().lower() or None
resolved_page, resolved_limit, resolved_offset = _resolve_pdf_queue_paging( resolved_page, resolved_limit, resolved_offset = _resolve_pdf_queue_paging(
@ -204,7 +204,7 @@ async def get_pdf_queue(
logger, logger,
"info", "info",
"api.admin.db.pdf_queue_listed", "api.admin.db.pdf_queue_listed",
admin_user_id=int(admin_user.id), user_id=int(current_user.id),
page=int(resolved_page), page=int(resolved_page),
page_size=int(resolved_limit), page_size=int(resolved_limit),
offset=int(resolved_offset), offset=int(resolved_offset),

View file

@ -261,11 +261,18 @@ async def delete_scholar(
code="scholar_not_found", code="scholar_not_found",
message="Scholar not found.", message="Scholar not found.",
) )
try:
await scholar_service.delete_scholar( await scholar_service.delete_scholar(
db_session, db_session,
profile=profile, profile=profile,
upload_dir=settings.scholar_image_upload_dir, 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( structured_log(
logger, "info", "api.scholars.deleted", user_id=current_user.id, scholar_profile_id=scholar_profile_id logger, "info", "api.scholars.deleted", user_id=current_user.id, scholar_profile_id=scholar_profile_id
) )

View file

@ -125,7 +125,11 @@ async def delete_scholar(
_safe_remove_upload(upload_root, profile.profile_image_upload_path) _safe_remove_upload(upload_root, profile.profile_image_upload_path)
await db_session.delete(profile) await db_session.delete(profile)
try:
await db_session.commit() 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( async def hydrate_profile_metadata(

View file

@ -2,10 +2,144 @@
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { mount } from "@vue/test-utils"; import { mount } from "@vue/test-utils";
import ScholarBatchAdd from "./ScholarBatchAdd.vue"; import ScholarBatchAdd from "./ScholarBatchAdd.vue";
import {
parseScholarIds,
parseScholarTokens,
extractScholarIdFromUrl,
validateTokenAsId,
} from "./scholar-batch-parsing";
const defaultProps = { saving: false, loading: false }; const defaultProps = { saving: false, loading: false };
describe("ScholarBatchAdd", () => { describe("parseScholarIds (unit)", () => {
it("parses a single bare ID", () => {
expect(parseScholarIds("A-UbBTPM15wL")).toEqual(["A-UbBTPM15wL"]);
});
it("parses multiple IDs separated by commas", () => {
expect(parseScholarIds("A-UbBTPM15wL, B-UbBTPM15wL")).toEqual(["A-UbBTPM15wL", "B-UbBTPM15wL"]);
});
it("deduplicates IDs", () => {
expect(parseScholarIds("A-UbBTPM15wL\nA-UbBTPM15wL")).toEqual(["A-UbBTPM15wL"]);
});
it("extracts ID from standard Google Scholar URL", () => {
expect(parseScholarIds("https://scholar.google.com/citations?hl=en&user=A-UbBTPM15wL")).toEqual(["A-UbBTPM15wL"]);
});
it("extracts ID from URL with trailing slash", () => {
expect(parseScholarIds("https://scholar.google.com/citations?user=A-UbBTPM15wL/")).toEqual(["A-UbBTPM15wL"]);
});
it("extracts ID from URL with fragment", () => {
expect(parseScholarIds("https://scholar.google.com/citations?user=A-UbBTPM15wL#section")).toEqual(["A-UbBTPM15wL"]);
});
it("extracts ID from URL with extra query params", () => {
expect(parseScholarIds("https://scholar.google.com/citations?hl=en&user=A-UbBTPM15wL&view_op=list_works&sortby=pubdate")).toEqual(["A-UbBTPM15wL"]);
});
it("handles URL-encoded characters in non-ID parts", () => {
expect(parseScholarIds("https://scholar.google.com/citations?hl=en&user=A-UbBTPM15wL&label=%E4%B8%AD%E6%96%87")).toEqual(["A-UbBTPM15wL"]);
});
it("rejects malformed IDs (too short)", () => {
expect(parseScholarIds("ABC123")).toEqual([]);
});
it("rejects malformed IDs (too long)", () => {
expect(parseScholarIds("ABCDEF1234567")).toEqual([]);
});
it("rejects IDs with special characters", () => {
expect(parseScholarIds("ABCDEF12345!")).toEqual([]);
});
it("rejects IDs with embedded whitespace", () => {
expect(parseScholarIds("ABC DEF12345")).toEqual([]);
});
it("handles mixed valid and invalid tokens", () => {
expect(parseScholarIds("A-UbBTPM15wL, invalid, B-UbBTPM15wL")).toEqual(["A-UbBTPM15wL", "B-UbBTPM15wL"]);
});
it("returns empty array for empty string", () => {
expect(parseScholarIds("")).toEqual([]);
});
it("returns empty array for whitespace only", () => {
expect(parseScholarIds(" ")).toEqual([]);
});
});
describe("extractScholarIdFromUrl", () => {
it("returns null for non-URL strings", () => {
expect(extractScholarIdFromUrl("not-a-url")).toBeNull();
});
it("returns null for URL without user param", () => {
expect(extractScholarIdFromUrl("https://scholar.google.com/citations?hl=en")).toBeNull();
});
it("extracts from URL with trailing slashes", () => {
expect(extractScholarIdFromUrl("https://scholar.google.com/citations?user=A-UbBTPM15wL///")).toBe("A-UbBTPM15wL");
});
it("strips fragment before parsing", () => {
expect(extractScholarIdFromUrl("https://scholar.google.com/citations?user=A-UbBTPM15wL#foo")).toBe("A-UbBTPM15wL");
});
it("returns null when user param is invalid length", () => {
expect(extractScholarIdFromUrl("https://scholar.google.com/citations?user=short")).toBeNull();
});
});
describe("validateTokenAsId", () => {
it("returns null for valid 12-char ID", () => {
expect(validateTokenAsId("ABCDEF123456")).toBeNull();
});
it("detects wrong length", () => {
expect(validateTokenAsId("ABC")).toContain("12 characters");
});
it("detects invalid characters", () => {
expect(validateTokenAsId("ABCDEF12345!")).toContain("invalid characters");
});
it("detects whitespace", () => {
expect(validateTokenAsId("ABC DEF12345")).toContain("whitespace");
});
it("detects empty input", () => {
expect(validateTokenAsId("")).toContain("empty");
});
});
describe("parseScholarTokens (per-token errors)", () => {
it("marks invalid tokens with error messages", () => {
const tokens = parseScholarTokens("A-UbBTPM15wL, short, B-UbBTPM15wL");
expect(tokens).toHaveLength(3);
expect(tokens[0].id).toBe("A-UbBTPM15wL");
expect(tokens[0].error).toBeNull();
expect(tokens[1].id).toBeNull();
expect(tokens[1].error).toContain("12 characters");
expect(tokens[2].id).toBe("B-UbBTPM15wL");
});
it("marks duplicate tokens", () => {
const tokens = parseScholarTokens("A-UbBTPM15wL, A-UbBTPM15wL");
expect(tokens[1].error).toBe("duplicate");
});
it("marks bad URL tokens", () => {
const tokens = parseScholarTokens("https://scholar.google.com/citations?hl=en");
expect(tokens[0].error).toContain("could not extract");
});
});
describe("ScholarBatchAdd (component)", () => {
it("renders the heading and input", () => { it("renders the heading and input", () => {
const wrapper = mount(ScholarBatchAdd, { props: defaultProps }); const wrapper = mount(ScholarBatchAdd, { props: defaultProps });
expect(wrapper.text()).toContain("Add Scholar Profiles"); expect(wrapper.text()).toContain("Add Scholar Profiles");
@ -73,4 +207,18 @@ describe("ScholarBatchAdd", () => {
const wrapper = mount(ScholarBatchAdd, { props: { ...defaultProps, saving: true } }); const wrapper = mount(ScholarBatchAdd, { props: { ...defaultProps, saving: true } });
expect(wrapper.text()).toContain("Adding..."); expect(wrapper.text()).toContain("Adding...");
}); });
it("shows validation errors for invalid tokens", async () => {
const wrapper = mount(ScholarBatchAdd, { props: defaultProps });
await wrapper.find("textarea").setValue("A-UbBTPM15wL, bad!");
const errors = wrapper.find("[data-testid='validation-errors']");
expect(errors.exists()).toBe(true);
});
it("shows skipped count alongside valid count", async () => {
const wrapper = mount(ScholarBatchAdd, { props: defaultProps });
await wrapper.find("textarea").setValue("A-UbBTPM15wL, tooshort");
expect(wrapper.text()).toContain("1 valid");
expect(wrapper.text()).toContain("1 skipped");
});
}); });

View file

@ -3,6 +3,7 @@ import { computed, ref } from "vue";
import AppButton from "@/components/ui/AppButton.vue"; import AppButton from "@/components/ui/AppButton.vue";
import AppCard from "@/components/ui/AppCard.vue"; import AppCard from "@/components/ui/AppCard.vue";
import AppHelpHint from "@/components/ui/AppHelpHint.vue"; import AppHelpHint from "@/components/ui/AppHelpHint.vue";
import { parseScholarTokens } from "./scholar-batch-parsing";
defineProps<{ defineProps<{
saving: boolean; saving: boolean;
@ -15,38 +16,13 @@ const emit = defineEmits<{
const scholarBatchInput = ref(""); const scholarBatchInput = ref("");
const SCHOLAR_ID_PATTERN = /^[a-zA-Z0-9_-]{12}$/; const parsedTokens = computed(() => parseScholarTokens(scholarBatchInput.value));
const URL_USER_PARAM_PATTERN = /(?:\?|&)user=([a-zA-Z0-9_-]{12})(?:&|#|$)/i; const validIds = computed(() => parsedTokens.value.filter((t) => t.id !== null));
const invalidTokens = computed(() => parsedTokens.value.filter((t) => t.id === null));
function parseScholarIds(raw: string): string[] { const parsedBatchCount = computed(() => validIds.value.length);
const ordered: string[] = [];
const seen = new Set<string>();
const tokens = raw.split(/[\s,;]+/).map((v) => v.trim()).filter((v) => v.length > 0);
for (const token of tokens) {
let candidate: string | null = null;
if (SCHOLAR_ID_PATTERN.test(token)) candidate = token;
if (!candidate) {
const directParamMatch = token.match(URL_USER_PARAM_PATTERN);
if (directParamMatch) candidate = directParamMatch[1];
}
if (!candidate && token.includes("scholar.google")) {
try {
const parsed = new URL(token);
const userParam = parsed.searchParams.get("user");
if (userParam && SCHOLAR_ID_PATTERN.test(userParam)) candidate = userParam;
} catch (_error) { /* Ignore non-URL tokens. */ }
}
if (!candidate || seen.has(candidate)) continue;
seen.add(candidate);
ordered.push(candidate);
}
return ordered;
}
const parsedBatchCount = computed(() => parseScholarIds(scholarBatchInput.value).length);
function onSubmit(): void { function onSubmit(): void {
const ids = parseScholarIds(scholarBatchInput.value); const ids = validIds.value.map((t) => t.id!);
if (ids.length > 0) { if (ids.length > 0) {
emit("add-scholars", ids); emit("add-scholars", ids);
scholarBatchInput.value = ""; scholarBatchInput.value = "";
@ -76,11 +52,33 @@ function onSubmit(): void {
/> />
</label> </label>
<div class="flex flex-wrap items-center justify-between gap-2"> <div v-if="parsedTokens.length > 0" class="space-y-1">
<p class="text-xs text-secondary"> <p class="text-xs text-secondary">
Parsed IDs: <strong class="text-ink-primary">{{ parsedBatchCount }}</strong> <strong class="text-ink-primary">{{ parsedBatchCount }}</strong> valid ID{{ parsedBatchCount === 1 ? "" : "s" }}
<template v-if="invalidTokens.length > 0">
· <span class="text-state-warning-text">{{ invalidTokens.length }} skipped</span>
</template>
</p> </p>
<AppButton type="submit" :disabled="saving || loading"> <ul v-if="invalidTokens.length > 0" class="space-y-0.5" data-testid="validation-errors">
<li
v-for="item in invalidTokens.slice(0, 5)"
:key="item.index"
class="text-xs text-state-warning-text"
>
#{{ item.index }}: {{ item.error }}
<code class="ml-1 break-all text-ink-muted">{{ item.raw.length > 60 ? item.raw.slice(0, 57) + "..." : item.raw }}</code>
</li>
<li v-if="invalidTokens.length > 5" class="text-xs text-state-warning-text">
+{{ invalidTokens.length - 5 }} more skipped
</li>
</ul>
</div>
<div v-else class="text-xs text-secondary">
Parsed IDs: <strong class="text-ink-primary">0</strong>
</div>
<div class="flex flex-wrap items-center justify-end gap-2">
<AppButton type="submit" :disabled="saving || loading || parsedBatchCount === 0">
{{ saving ? "Adding..." : "Add scholars" }} {{ saving ? "Adding..." : "Add scholars" }}
</AppButton> </AppButton>
</div> </div>

View file

@ -47,7 +47,14 @@ function scholarPublicationsRoute(profile: ScholarProfile): { name: string; quer
:image-url="scholar.profile_image_url" :image-url="scholar.profile_image_url"
/> />
<div class="min-w-0 space-y-1"> <div class="min-w-0 space-y-1">
<p class="truncate text-sm font-semibold text-ink-primary">{{ scholarLabel(scholar) }}</p> <p class="truncate text-sm font-semibold text-ink-primary">
{{ scholarLabel(scholar) }}
<span
v-if="!scholar.baseline_completed"
class="ml-1.5 inline-flex items-center rounded-full border border-state-warning-border bg-state-warning-bg px-1.5 py-0.5 text-[10px] font-medium text-state-warning-text"
title="This scholar hasn't been scraped yet. Publications will appear after the next run."
>Pending</span>
</p>
<p class="text-xs text-secondary">ID: <code>{{ scholar.scholar_id }}</code></p> <p class="text-xs text-secondary">ID: <code>{{ scholar.scholar_id }}</code></p>
<div class="flex flex-wrap items-center gap-3"> <div class="flex flex-wrap items-center gap-3">
<RouterLink :to="scholarPublicationsRoute(scholar)" class="link-inline text-xs"> <RouterLink :to="scholarPublicationsRoute(scholar)" class="link-inline text-xs">

View file

@ -0,0 +1,29 @@
<script setup lang="ts">
defineProps<{
baselineCompleted: boolean;
lastRunStatus: string | null;
}>();
</script>
<template>
<span
v-if="!baselineCompleted"
class="ml-1.5 inline-flex items-center rounded-full border border-state-warning-border bg-state-warning-bg px-1.5 py-0.5 text-[10px] font-medium text-state-warning-text"
title="This scholar hasn't been scraped yet. Publications will appear after the next run."
>Pending</span>
<span
v-if="lastRunStatus === 'failed'"
class="ml-1.5 inline-flex items-center rounded-full border border-state-danger-border bg-state-danger-bg px-1.5 py-0.5 text-[10px] font-medium text-state-danger-text"
title="The last scrape run for this scholar failed."
>Failed</span>
<span
v-else-if="lastRunStatus === 'partial'"
class="ml-1.5 inline-flex items-center rounded-full border border-state-warning-border bg-state-warning-bg px-1.5 py-0.5 text-[10px] font-medium text-state-warning-text"
title="The last scrape run for this scholar completed with partial results."
>Partial</span>
<span
v-else-if="lastRunStatus === 'success'"
class="ml-1.5 inline-flex items-center rounded-full border border-state-success-border bg-state-success-bg px-1.5 py-0.5 text-[10px] font-medium text-state-success-text"
title="The last scrape run for this scholar completed successfully."
>OK</span>
</template>

View file

@ -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<string>();
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!);
}

View file

@ -1,7 +1,7 @@
<script setup lang="ts"> <script setup lang="ts">
import { onMounted, ref, watch } from "vue"; import { onMounted, watch } from "vue";
import { ref } from "vue";
import AsyncStateGate from "@/components/patterns/AsyncStateGate.vue";
import RequestStateAlerts from "@/components/patterns/RequestStateAlerts.vue"; import RequestStateAlerts from "@/components/patterns/RequestStateAlerts.vue";
import AdminIntegritySection from "@/features/settings/components/AdminIntegritySection.vue"; import AdminIntegritySection from "@/features/settings/components/AdminIntegritySection.vue";
import AdminPdfQueueSection from "@/features/settings/components/AdminPdfQueueSection.vue"; import AdminPdfQueueSection from "@/features/settings/components/AdminPdfQueueSection.vue";
@ -18,7 +18,6 @@ const props = defineProps<{
section: "users" | "integrity" | "repairs" | "pdf"; section: "users" | "integrity" | "repairs" | "pdf";
}>(); }>();
const loading = ref(true);
const { errorMessage, errorRequestId, successMessage, clearAlerts, assignError } = useRequestState(); const { errorMessage, errorRequestId, successMessage, clearAlerts, assignError } = useRequestState();
const usersRef = ref<InstanceType<typeof AdminUsersSection> | null>(null); const usersRef = ref<InstanceType<typeof AdminUsersSection> | null>(null);
@ -26,34 +25,21 @@ const integrityRef = ref<InstanceType<typeof AdminIntegritySection> | null>(null
const repairsRef = ref<InstanceType<typeof AdminRepairsSection> | null>(null); const repairsRef = ref<InstanceType<typeof AdminRepairsSection> | null>(null);
const pdfQueueRef = ref<InstanceType<typeof AdminPdfQueueSection> | null>(null); const pdfQueueRef = ref<InstanceType<typeof AdminPdfQueueSection> | null>(null);
async function refreshForSection(): Promise<void> {
if (props.section === SECTION_USERS && usersRef.value) {
await usersRef.value.load();
return;
}
if (props.section === SECTION_INTEGRITY && integrityRef.value) {
await integrityRef.value.load();
return;
}
if (props.section === SECTION_REPAIRS) {
await usersRef.value?.load();
await repairsRef.value?.load();
return;
}
if (props.section === SECTION_PDF && pdfQueueRef.value) {
await pdfQueueRef.value.load();
}
}
async function loadSection(): Promise<void> { async function loadSection(): Promise<void> {
loading.value = true;
clearAlerts(); clearAlerts();
try { try {
await refreshForSection(); if (props.section === SECTION_USERS && usersRef.value) {
await usersRef.value.load();
} else if (props.section === SECTION_INTEGRITY && integrityRef.value) {
await integrityRef.value.load();
} else if (props.section === SECTION_REPAIRS) {
await usersRef.value?.load();
await repairsRef.value?.load();
} else if (props.section === SECTION_PDF && pdfQueueRef.value) {
await pdfQueueRef.value.load();
}
} catch (error) { } catch (error) {
assignError(error, "Unable to load admin data."); assignError(error, "Unable to load admin data.");
} finally {
loading.value = false;
} }
} }
@ -72,11 +58,9 @@ watch(() => props.section, loadSection);
@dismiss-success="successMessage = null" @dismiss-success="successMessage = null"
/> />
<AsyncStateGate :loading="loading" :loading-lines="10" :show-empty="false">
<AdminUsersSection v-if="props.section === SECTION_USERS || props.section === SECTION_REPAIRS" v-show="props.section === SECTION_USERS" ref="usersRef" /> <AdminUsersSection v-if="props.section === SECTION_USERS || props.section === SECTION_REPAIRS" v-show="props.section === SECTION_USERS" ref="usersRef" />
<AdminIntegritySection v-if="props.section === SECTION_INTEGRITY" ref="integrityRef" /> <AdminIntegritySection v-if="props.section === SECTION_INTEGRITY" ref="integrityRef" />
<AdminRepairsSection v-if="props.section === SECTION_REPAIRS" ref="repairsRef" :users="usersRef?.users ?? []" /> <AdminRepairsSection v-if="props.section === SECTION_REPAIRS" ref="repairsRef" :users="usersRef?.users ?? []" />
<AdminPdfQueueSection v-if="props.section === SECTION_PDF" ref="pdfQueueRef" /> <AdminPdfQueueSection v-if="props.section === SECTION_PDF" ref="pdfQueueRef" />
</AsyncStateGate>
</section> </section>
</template> </template>

View file

@ -1,6 +1,8 @@
// @vitest-environment happy-dom // @vitest-environment happy-dom
import { describe, expect, it, vi, beforeEach } from "vitest"; import { describe, expect, it, vi, beforeEach } from "vitest";
import { mount, flushPromises } from "@vue/test-utils"; import { mount, flushPromises } from "@vue/test-utils";
import { setActivePinia, createPinia } from "pinia";
import { useAuthStore } from "@/stores/auth";
import AdminPdfQueueSection from "./AdminPdfQueueSection.vue"; import AdminPdfQueueSection from "./AdminPdfQueueSection.vue";
vi.mock("@/features/admin_dbops", () => ({ vi.mock("@/features/admin_dbops", () => ({
@ -36,6 +38,9 @@ function buildQueueItem(overrides: Record<string, unknown> = {}) {
describe("AdminPdfQueueSection", () => { describe("AdminPdfQueueSection", () => {
beforeEach(() => { 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(); mockedListQueue.mockReset();
}); });

View file

@ -1,5 +1,6 @@
<script setup lang="ts"> <script setup lang="ts">
import { computed, ref } from "vue"; import { computed, ref } from "vue";
import { useAuthStore } from "@/stores/auth";
import AppBadge from "@/components/ui/AppBadge.vue"; import AppBadge from "@/components/ui/AppBadge.vue";
import AppButton from "@/components/ui/AppButton.vue"; import AppButton from "@/components/ui/AppButton.vue";
import AppCard from "@/components/ui/AppCard.vue"; import AppCard from "@/components/ui/AppCard.vue";
@ -17,6 +18,7 @@ import {
import { useRequestState } from "@/composables/useRequestState"; import { useRequestState } from "@/composables/useRequestState";
const { errorMessage, errorRequestId, successMessage, clearAlerts, assignError, setSuccess } = useRequestState(); const { errorMessage, errorRequestId, successMessage, clearAlerts, assignError, setSuccess } = useRequestState();
const auth = useAuthStore();
const refreshingPdfQueue = ref(false); const refreshingPdfQueue = ref(false);
const requeueingPublicationId = ref<number | null>(null); const requeueingPublicationId = ref<number | null>(null);
@ -162,7 +164,7 @@ defineExpose({ load });
<option value="50">50 / page</option> <option value="50">50 / page</option>
<option value="100">100 / page</option> <option value="100">100 / page</option>
</AppSelect> </AppSelect>
<AppButton variant="secondary" class="!min-h-8 whitespace-nowrap !px-2.5 !py-1 !text-xs" :disabled="requeueingAllPdfs" title="Queue all missing PDFs" @click="onRequeueAllPdfs"> <AppButton v-if="auth.isAdmin" variant="secondary" class="!min-h-8 whitespace-nowrap !px-2.5 !py-1 !text-xs" :disabled="requeueingAllPdfs" title="Queue all missing PDFs" @click="onRequeueAllPdfs">
{{ requeueingAllPdfs ? "Queueing..." : "Queue all" }} {{ requeueingAllPdfs ? "Queueing..." : "Queue all" }}
</AppButton> </AppButton>
<AppRefreshButton variant="secondary" size="sm" :loading="refreshingPdfQueue" title="Refresh PDF queue" loading-title="Refreshing PDF queue" @click="refreshPdfQueue" /> <AppRefreshButton variant="secondary" size="sm" :loading="refreshingPdfQueue" title="Refresh PDF queue" loading-title="Refreshing PDF queue" @click="refreshPdfQueue" />
@ -194,7 +196,7 @@ defineExpose({ load });
<td>{{ formatTimestamp(item.last_attempt_at) }}</td> <td>{{ formatTimestamp(item.last_attempt_at) }}</td>
<td>{{ formatTimestamp(item.resolved_at) }}</td> <td>{{ formatTimestamp(item.resolved_at) }}</td>
<td> <td>
<AppButton variant="ghost" :disabled="requeueingPublicationId === item.publication_id || !canRequeuePdf(item)" @click="onRequeuePdf(item)"> <AppButton v-if="auth.isAdmin" variant="ghost" :disabled="requeueingPublicationId === item.publication_id || !canRequeuePdf(item)" @click="onRequeuePdf(item)">
{{ requeueingPublicationId === item.publication_id ? "Requeueing..." : "Requeue" }} {{ requeueingPublicationId === item.publication_id ? "Requeueing..." : "Requeue" }}
</AppButton> </AppButton>
</td> </td>

View file

@ -404,7 +404,7 @@ watch(
</div> </div>
<div class="flex items-center gap-2"> <div class="flex items-center gap-2">
<AppButton <AppButton
v-if="auth.isAdmin && runStatus.isLikelyRunning" v-if="runStatus.isLikelyRunning"
variant="danger" variant="danger"
:disabled="!activeRunId || isCancelAnimating" :disabled="!activeRunId || isCancelAnimating"
@click="onCancelRun" @click="onCancelRun"
@ -420,7 +420,6 @@ watch(
</span> </span>
</AppButton> </AppButton>
<AppButton <AppButton
v-if="auth.isAdmin"
:disabled="isStartBlocked" :disabled="isStartBlocked"
:title="startCheckDisabledReason || undefined" :title="startCheckDisabledReason || undefined"
:class="isStartCheckAnimating ? 'shadow-[0_0_0_1px_var(--color-state-info-border)]' : ''" :class="isStartCheckAnimating ? 'shadow-[0_0_0_1px_var(--color-state-info-border)]' : ''"
@ -495,6 +494,11 @@ watch(
<template v-else> <template v-else>
Processed {{ displayedLatestRun.scholar_count }} scholars and discovered Processed {{ displayedLatestRun.scholar_count }} scholars and discovered
{{ displayedLatestRun.new_publication_count }} new publications. {{ displayedLatestRun.new_publication_count }} new publications.
<template v-if="displayedLatestRun.failed_count > 0 || displayedLatestRun.partial_count > 0">
<span class="text-state-warning-text">
{{ displayedLatestRun.failed_count > 0 ? `${displayedLatestRun.failed_count} failed` : "" }}{{ displayedLatestRun.failed_count > 0 && displayedLatestRun.partial_count > 0 ? ", " : "" }}{{ displayedLatestRun.partial_count > 0 ? `${displayedLatestRun.partial_count} partial` : "" }}.
</span>
</template>
</template> </template>
</p> </p>
</div> </div>
@ -528,6 +532,9 @@ watch(
</div> </div>
<span class="text-xs text-secondary"> <span class="text-xs text-secondary">
{{ formatDate(run.start_dt) }} {{ run.new_publication_count }} new {{ formatDate(run.start_dt) }} {{ run.new_publication_count }} new
<template v-if="run.failed_count > 0">
<span class="text-state-warning-text">{{ run.failed_count }} failed</span>
</template>
</span> </span>
</li> </li>
</ul> </ul>

View file

@ -31,6 +31,7 @@ import ScholarAvatar from "@/features/scholars/components/ScholarAvatar.vue";
import ScholarBatchAdd from "@/features/scholars/components/ScholarBatchAdd.vue"; import ScholarBatchAdd from "@/features/scholars/components/ScholarBatchAdd.vue";
import ScholarNameSearch from "@/features/scholars/components/ScholarNameSearch.vue"; import ScholarNameSearch from "@/features/scholars/components/ScholarNameSearch.vue";
import ScholarSettingsModal from "@/features/scholars/components/ScholarSettingsModal.vue"; import ScholarSettingsModal from "@/features/scholars/components/ScholarSettingsModal.vue";
import ScholarStatusBadges from "@/features/scholars/components/ScholarStatusBadges.vue";
import { ApiRequestError } from "@/lib/api/errors"; import { ApiRequestError } from "@/lib/api/errors";
import { useRunStatusStore } from "@/stores/run_status"; import { useRunStatusStore } from "@/stores/run_status";
@ -505,7 +506,10 @@ watch(
<div class="flex items-start gap-3"> <div class="flex items-start gap-3">
<ScholarAvatar :label="item.display_name" :scholar-id="item.scholar_id" :image-url="item.profile_image_url" /> <ScholarAvatar :label="item.display_name" :scholar-id="item.scholar_id" :image-url="item.profile_image_url" />
<div class="min-w-0 flex-1 space-y-1"> <div class="min-w-0 flex-1 space-y-1">
<p class="truncate text-sm font-semibold text-ink-primary">{{ scholarLabel(item) }}</p> <p class="truncate text-sm font-semibold text-ink-primary">
{{ scholarLabel(item) }}
<ScholarStatusBadges :baseline-completed="item.baseline_completed" :last-run-status="item.last_run_status" />
</p>
<div class="flex flex-wrap items-center gap-3"> <div class="flex flex-wrap items-center gap-3">
<RouterLink :to="scholarPublicationsRoute(item)" class="link-inline text-xs">Publications</RouterLink> <RouterLink :to="scholarPublicationsRoute(item)" class="link-inline text-xs">Publications</RouterLink>
<a :href="scholarProfileUrl(item.scholar_id)" target="_blank" rel="noreferrer" class="link-inline text-xs">Open profile</a> <a :href="scholarProfileUrl(item.scholar_id)" target="_blank" rel="noreferrer" class="link-inline text-xs">Open profile</a>
@ -529,7 +533,10 @@ watch(
<div class="flex items-start gap-3"> <div class="flex items-start gap-3">
<ScholarAvatar :label="item.display_name" :scholar-id="item.scholar_id" :image-url="item.profile_image_url" /> <ScholarAvatar :label="item.display_name" :scholar-id="item.scholar_id" :image-url="item.profile_image_url" />
<div class="grid min-w-0 gap-1"> <div class="grid min-w-0 gap-1">
<strong class="truncate text-ink-primary">{{ scholarLabel(item) }}</strong> <strong class="truncate text-ink-primary">
{{ scholarLabel(item) }}
<ScholarStatusBadges :baseline-completed="item.baseline_completed" :last-run-status="item.last_run_status" />
</strong>
<div class="flex flex-wrap items-center gap-3"> <div class="flex flex-wrap items-center gap-3">
<RouterLink :to="scholarPublicationsRoute(item)" class="link-inline text-xs">Publications</RouterLink> <RouterLink :to="scholarPublicationsRoute(item)" class="link-inline text-xs">Publications</RouterLink>
<a :href="scholarProfileUrl(item.scholar_id)" target="_blank" rel="noreferrer" class="link-inline text-xs">Open profile</a> <a :href="scholarProfileUrl(item.scholar_id)" target="_blank" rel="noreferrer" class="link-inline text-xs">Open profile</a>

View file

@ -47,7 +47,7 @@ const savingScholarHttp = ref(false);
const updatingPassword = ref(false); const updatingPassword = ref(false);
const autoRunEnabled = ref(false); const autoRunEnabled = ref(false);
const runIntervalMinutes = ref("60"); const runIntervalHours = ref("1");
const requestDelaySeconds = ref("2"); const requestDelaySeconds = ref("2");
const navVisiblePages = ref<string[]>([]); const navVisiblePages = ref<string[]>([]);
const openalexApiKey = ref(""); const openalexApiKey = ref("");
@ -77,13 +77,13 @@ const tabItems = computed<AppTabItem[]>(() => {
const tabs: AppTabItem[] = [ const tabs: AppTabItem[] = [
{ id: TAB_CHECKING, label: "Checking" }, { id: TAB_CHECKING, label: "Checking" },
{ id: TAB_ACCOUNT, label: "Account" }, { id: TAB_ACCOUNT, label: "Account" },
{ id: TAB_ADMIN_PDF, label: "PDF Queue" },
]; ];
if (auth.isAdmin) { if (auth.isAdmin) {
tabs.push( tabs.push(
{ id: TAB_ADMIN_USERS, label: "Users" }, { id: TAB_ADMIN_USERS, label: "Users" },
{ id: TAB_ADMIN_INTEGRITY, label: "Integrity" }, { id: TAB_ADMIN_INTEGRITY, label: "Integrity" },
{ id: TAB_ADMIN_REPAIRS, label: "Repairs" }, { id: TAB_ADMIN_REPAIRS, label: "Repairs" },
{ id: TAB_ADMIN_PDF, label: "PDF Queue" },
{ id: TAB_ADMIN_INTEGRATIONS, label: "Integrations" }, { id: TAB_ADMIN_INTEGRATIONS, label: "Integrations" },
); );
} }
@ -138,7 +138,7 @@ function hydrateSettings(settings: UserSettings): void {
manualRunAllowed.value = Boolean(settings.policy?.manual_run_allowed ?? true); manualRunAllowed.value = Boolean(settings.policy?.manual_run_allowed ?? true);
autoRunEnabled.value = Boolean(settings.auto_run_enabled) && automationAllowed.value; autoRunEnabled.value = Boolean(settings.auto_run_enabled) && automationAllowed.value;
runIntervalMinutes.value = String(settings.run_interval_minutes); runIntervalHours.value = String(Number(settings.run_interval_minutes) / 60);
requestDelaySeconds.value = String(settings.request_delay_seconds); requestDelaySeconds.value = String(settings.request_delay_seconds);
navVisiblePages.value = normalizeUserNavVisiblePages(settings.nav_visible_pages); navVisiblePages.value = normalizeUserNavVisiblePages(settings.nav_visible_pages);
@ -168,6 +168,18 @@ function parseBoundedInteger(value: string, label: string, minimum: number): num
return parsed; return parsed;
} }
function parseHoursToMinutes(value: string, minMinutes: number): number {
const hours = Number(value);
if (!Number.isFinite(hours) || hours <= 0) {
throw new Error("Check interval must be a positive number of hours.");
}
const minutes = Math.round(hours * 60);
if (minutes < minMinutes) {
throw new Error(`Check interval must be at least ${minMinutes / 60} hours.`);
}
return minutes;
}
async function loadSettings(): Promise<void> { async function loadSettings(): Promise<void> {
loading.value = true; loading.value = true;
errorMessage.value = null; errorMessage.value = null;
@ -225,9 +237,8 @@ async function onSaveSettings(): Promise<void> {
try { try {
const payload: UserSettingsUpdate = { const payload: UserSettingsUpdate = {
auto_run_enabled: autoRunEnabled.value, auto_run_enabled: autoRunEnabled.value,
run_interval_minutes: parseBoundedInteger( run_interval_minutes: parseHoursToMinutes(
runIntervalMinutes.value, runIntervalHours.value,
"Check interval (minutes)",
minCheckIntervalMinutes.value, minCheckIntervalMinutes.value,
), ),
request_delay_seconds: parseBoundedInteger( request_delay_seconds: parseBoundedInteger(
@ -357,11 +368,11 @@ onMounted(async () => {
<label class="grid gap-2 text-sm font-medium text-ink-secondary"> <label class="grid gap-2 text-sm font-medium text-ink-secondary">
<span class="inline-flex items-center gap-1"> <span class="inline-flex items-center gap-1">
Check interval (minutes) Check interval (hours)
<AppHelpHint text="Minimum is controlled by server policy." /> <AppHelpHint text="Minimum is controlled by server policy." />
</span> </span>
<AppInput v-model="runIntervalMinutes" inputmode="numeric" /> <AppInput v-model="runIntervalHours" inputmode="decimal" />
<span class="text-xs text-secondary">Minimum: {{ minCheckIntervalMinutes }}</span> <span class="text-xs text-secondary">Minimum: {{ minCheckIntervalMinutes / 60 }} hours</span>
</label> </label>
<label class="grid gap-2 text-sm font-medium text-ink-secondary"> <label class="grid gap-2 text-sm font-medium text-ink-secondary">

View file

@ -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.status_code == 403
assert forbidden_integrity.json()["error"]["code"] == "forbidden" assert forbidden_integrity.json()["error"]["code"] == "forbidden"
forbidden_pdf_queue = client.get("/api/v1/admin/db/pdf-queue") # PDF queue listing is accessible to all authenticated users (not admin-only).
assert forbidden_pdf_queue.status_code == 403 allowed_pdf_queue = client.get("/api/v1/admin/db/pdf-queue")
assert forbidden_pdf_queue.json()["error"]["code"] == "forbidden" assert allowed_pdf_queue.status_code == 200
# But requeue actions still require admin.
forbidden_requeue = client.post( forbidden_requeue = client.post(
"/api/v1/admin/db/pdf-queue/1/requeue", "/api/v1/admin/db/pdf-queue/1/requeue",
headers=headers, headers=headers,

View file

@ -70,6 +70,110 @@ async def test_api_scholars_crud_flow(db_session: AsyncSession) -> None:
assert delete_response.json()["data"]["message"] == "Scholar deleted." 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.integration
@pytest.mark.db @pytest.mark.db
@pytest.mark.asyncio @pytest.mark.asyncio

View file

@ -42,6 +42,34 @@ class TestValidateScholarId:
with pytest.raises(ScholarServiceError, match="12"): with pytest.raises(ScholarServiceError, match="12"):
validate_scholar_id("ABCDEF12345!") 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: class TestNormalizeDisplayName:
def test_returns_stripped_name(self) -> None: def test_returns_stripped_name(self) -> None: