fix(scholars): improve validation, fix delete, add status badges
- Extract scholar ID parsing to dedicated module with robust URL handling (trailing slashes, fragments, extra query params, encoded characters) and per-token error feedback showing why each invalid entry was skipped - Wrap delete_scholar DB commit in IntegrityError handler so cascade failures return a proper API error instead of 500 - Add ScholarStatusBadges component showing Pending/Failed/Partial/OK badges on scholar rows based on baseline_completed and last_run_status - Show Pending badge in ScholarSettingsModal for unscraped scholars - Surface failed_count and partial_count in dashboard latest run summary and recent run list items - Extend backend validator tests with edge cases (empty, whitespace, unicode, URL-as-ID) - Add integration tests for DELETE 404, DELETE with cascade, and POST with corrupted scholar_id - Add comprehensive frontend tests for parsing logic and component behavior
This commit is contained in:
parent
0d955c31dc
commit
813da91608
11 changed files with 459 additions and 43 deletions
|
|
@ -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
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -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(
|
||||||
|
|
|
||||||
|
|
@ -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");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -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>
|
||||||
|
|
|
||||||
|
|
@ -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 bg-amber-50 px-1.5 py-0.5 text-[10px] font-medium text-amber-700 ring-1 ring-inset ring-amber-600/20 dark:bg-amber-950 dark:text-amber-400 dark:ring-amber-400/20"
|
||||||
|
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">
|
||||||
|
|
|
||||||
|
|
@ -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 bg-amber-50 px-1.5 py-0.5 text-[10px] font-medium text-amber-700 ring-1 ring-inset ring-amber-600/20 dark:bg-amber-950 dark:text-amber-400 dark:ring-amber-400/20"
|
||||||
|
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 bg-red-50 px-1.5 py-0.5 text-[10px] font-medium text-red-700 ring-1 ring-inset ring-red-600/20 dark:bg-red-950 dark:text-red-400 dark:ring-red-400/20"
|
||||||
|
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 bg-amber-50 px-1.5 py-0.5 text-[10px] font-medium text-amber-700 ring-1 ring-inset ring-amber-600/20 dark:bg-amber-950 dark:text-amber-400 dark:ring-amber-400/20"
|
||||||
|
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 bg-green-50 px-1.5 py-0.5 text-[10px] font-medium text-green-700 ring-1 ring-inset ring-green-600/20 dark:bg-green-950 dark:text-green-400 dark:ring-green-400/20"
|
||||||
|
title="The last scrape run for this scholar completed successfully."
|
||||||
|
>OK</span>
|
||||||
|
</template>
|
||||||
|
|
@ -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!);
|
||||||
|
}
|
||||||
|
|
@ -494,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>
|
||||||
|
|
@ -527,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>
|
||||||
|
|
|
||||||
|
|
@ -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>
|
||||||
|
|
|
||||||
|
|
@ -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
|
||||||
|
|
|
||||||
|
|
@ -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:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue