diff --git a/CHANGELOG.md b/CHANGELOG.md index cb8f4fb2d..d8cba346f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **OC_URL link annotations — clickable hyperlinks anchored to highlighted text** (`opencontractserver/annotations/models.py`, `opencontractserver/annotations/migrations/0072_annotation_link_url.py`, `opencontractserver/constants/annotations.py`, `config/graphql/{annotation_mutations,serializers,mutations}.py`, `frontend/src/assets/configurations/constants.ts`, `frontend/src/types/graphql-api.ts`, `frontend/src/components/annotator/types/annotations.ts`, `frontend/src/components/annotator/utils/urlAnnotation.ts` (new), `frontend/src/graphql/{queries,mutations}.ts`, `frontend/src/components/annotator/hooks/AnnotationHooks.tsx`, `frontend/src/components/annotator/display/components/{Selection,SelectionBoundary}.tsx`, `frontend/src/components/annotator/renderers/pdf/{PDF,PDFPage,SelectionLayer}.tsx`, `frontend/src/components/annotator/renderers/txt/TxtAnnotator.tsx`, `frontend/src/components/annotator/components/wrappers/TxtAnnotatorWrapper.tsx`, `frontend/src/components/annotator/components/modals/CreateUrlAnnotationModal.tsx` (new), `frontend/src/components/knowledge_base/document/DocumentKnowledgeBase.tsx`, `frontend/src/components/knowledge_base/document/document_kb/DocumentViewer.tsx`, `frontend/src/utils/transform.tsx`). Annotations carrying the new `OC_URL` label render as hyperlinks (underline + external-link icon, pointer cursor) and open their `link_url` on click in both the PDF viewer and the text/markdown viewer. The `Annotation.link_url` URL field (max 2048, nullable) carries the target; an "Add link…" item in the selection action menu prompts for a URL and calls a new `add_url_annotation` GraphQL mutation that auto-creates the `OC_URL` label per-corpus via `Corpus.ensure_label_and_labelset` (mirroring `OC_SECTION`). The pencil icon on an existing `OC_URL` annotation opens a URL-edit modal instead of the label modal. Held `Shift`/`Ctrl`/`Cmd` while clicking a link annotation falls back to the normal "toggle selection" behaviour so authors can still pick a link to delete or re-edit it. **Security**: `link_url` is validated both in `Annotation.save()` (always runs, regardless of `VALIDATE_ANNOTATION_JSON`) and in the GraphQL mutation/serializer layer via a shared `validate_link_url(...)` helper — only `http://`, `https://`, and site-relative `/...` paths are accepted, so `javascript:`, `data:`, and other dangerous schemes are rejected before persistence and never reach `window.open`. External targets open in a new tab with `noopener,noreferrer`; site-relative paths navigate in the current tab so the SPA router can resolve them. + ### Security - **Auth0 permissioning audit — privilege escalation, IDOR, and credential storage hardening** (`config/settings/base.py`, `config/graphql_auth0_auth/utils.py`, `config/graphql/annotation_mutations.py`, `config/graphql/extract_mutations.py`, `opencontractserver/analyzer/{models.py,views.py,migrations/0021_hash_analysis_callback_token.py}`, `opencontractserver/utils/analyzer.py`, `opencontractserver/users/{apps.py,checks.py,models.py}`, `opencontractserver/constants/auth.py`, `opencontractserver/tests/{test_analyzers.py,test_security_hardening.py}`, `docs/configuration/authentication.md`). Outcome of an end-to-end audit of every authorization pathway that touches Auth0. Verified findings only — agent-reported "CRITICAL" claims that turned out to be false positives (intentional IDOR-safe `SetCorpusVisibility`, `AnnotationImagesView`, moderation mutations, `conversation_queries.moderation_metrics`, etc.) were dropped after self-verification. Twelve real fixes shipped across three layers: diff --git a/config/graphql/annotation_mutations.py b/config/graphql/annotation_mutations.py index c7dfc72b4..80069fe7c 100644 --- a/config/graphql/annotation_mutations.py +++ b/config/graphql/annotation_mutations.py @@ -5,7 +5,7 @@ import logging import graphene -from django.core.exceptions import ObjectDoesNotExist +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db import transaction from graphene.types.generic import GenericScalar from graphql_jwt.decorators import login_required @@ -26,6 +26,13 @@ AnnotationLabel, Note, Relationship, + validate_link_url, +) +from opencontractserver.constants.annotations import ( + OC_URL_LABEL, + OC_URL_LABEL_COLOR, + OC_URL_LABEL_DESCRIPTION, + OC_URL_LABEL_ICON, ) from opencontractserver.corpuses.models import Corpus from opencontractserver.documents.models import Document, DocumentPath @@ -216,6 +223,21 @@ def mutate(root, info, annotation_id, comment=None) -> "ApproveAnnotation": ) +def _format_link_url_error(exc: ValidationError) -> str: + """Surface a stable, human-readable link_url validation error. + + ``str(ValidationError({"link_url": "..."}))`` returns a Python + ``[" {'link_url': ['...']} "]`` string that leaks internal structure. + Pull the first message off the dict so the user sees a clean sentence. + """ + detail = getattr(exc, "message_dict", None) + if detail: + messages = detail.get("link_url", []) or [] + if messages: + return str(messages[0]) + return "link_url failed validation." + + def _resolve_annotation_parents( user, corpus_pk: int | str, document_pk: int | str ) -> tuple["Document", "Corpus"] | None: @@ -279,6 +301,13 @@ class Arguments: required=False, description="Optional markdown description for this annotation.", ) + link_url = graphene.String( + required=False, + description=( + "Optional URL opened on click. Restricted to http(s):// or " + "site-relative paths; intended for OC_URL annotations." + ), + ) ok = graphene.Boolean() message = graphene.String() @@ -297,6 +326,7 @@ def mutate( annotation_label_id, annotation_type, long_description=None, + link_url=None, ) -> "AddAnnotation": corpus_pk = from_global_id(corpus_id)[1] document_pk = from_global_id(document_id)[1] @@ -304,6 +334,14 @@ def mutate( user = info.context.user + if link_url: + try: + validate_link_url(link_url) + except ValidationError as exc: + return AddAnnotation( + ok=False, annotation=None, message=_format_link_url_error(exc) + ) + parents = _resolve_annotation_parents(user, corpus_pk, document_pk) if parents is None: return AddAnnotation( @@ -323,6 +361,10 @@ def mutate( creator=user, json=json, annotation_type=annotation_type.value, + # Normalise empty string to None so the column ends up NULL + # (the ``if link_url:`` guard above only protects the validator + # call, not the persisted value). + link_url=link_url or None, ) annotation.save() set_permissions_for_obj_to_user(user, annotation, [PermissionTypes.CRUD]) @@ -332,6 +374,113 @@ def mutate( ) +class AddUrlAnnotation(graphene.Mutation): + """Create an annotation labelled ``OC_URL`` with a click-through URL. + + Convenience wrapper over ``AddAnnotation``: ensures the corpus has an + ``OC_URL`` label (creating it if absent) and stamps ``link_url`` on the + resulting annotation so the frontend renders the highlighted text as a + clickable hyperlink. + """ + + class Arguments: + json = GenericScalar( + required=True, description="New-style JSON for multipage annotations." + ) + page = graphene.Int( + required=True, description="What page is this annotation on (0-indexed)." + ) + raw_text = graphene.String( + required=True, description="The raw text being linked." + ) + corpus_id = graphene.String( + required=True, description="ID of the corpus this annotation is for." + ) + document_id = graphene.String( + required=True, description="ID of the document this annotation is on." + ) + annotation_type = graphene.Argument( + graphene.Enum.from_enum(LabelType), + required=True, + description="Annotation type: TOKEN_LABEL for PDFs, SPAN_LABEL for text.", + ) + link_url = graphene.String( + required=True, + description="The target URL to open on click.", + ) + + ok = graphene.Boolean() + message = graphene.String() + annotation = graphene.Field(AnnotationType) + + @login_required + @graphql_ratelimit_dynamic(get_rate=get_user_tier_rate("WRITE_LIGHT")) + def mutate( + root, + info, + json, + page, + raw_text, + corpus_id, + document_id, + annotation_type, + link_url, + ) -> "AddUrlAnnotation": + corpus_pk = from_global_id(corpus_id)[1] + document_pk = from_global_id(document_id)[1] + + user = info.context.user + + try: + validate_link_url(link_url) + except ValidationError as exc: + return AddUrlAnnotation( + ok=False, annotation=None, message=_format_link_url_error(exc) + ) + + parents = _resolve_annotation_parents(user, corpus_pk, document_pk) + if parents is None: + return AddUrlAnnotation( + ok=False, + annotation=None, + message=_ANNOTATION_PARENT_NOT_FOUND_MSG, + ) + document, corpus = parents + + with transaction.atomic(): + # ``ensure_label_and_labelset`` is idempotent per (text, label_type). + # PDF (TOKEN_LABEL) and text (SPAN_LABEL) documents each get their + # own OC_URL row — the lookup filters on both fields, so flipping + # types between calls cannot return a label of the wrong shape to + # the renderer. + label = corpus.ensure_label_and_labelset( + label_text=OC_URL_LABEL, + creator_id=user.pk, + label_type=annotation_type.value, + color=OC_URL_LABEL_COLOR, + icon=OC_URL_LABEL_ICON, + description=OC_URL_LABEL_DESCRIPTION, + ) + + annotation = Annotation( + page=page, + raw_text=raw_text, + corpus_id=corpus.pk, + document_id=document.pk, + annotation_label_id=label.pk, + creator=user, + json=json, + annotation_type=annotation_type.value, + link_url=link_url, + ) + annotation.save() + set_permissions_for_obj_to_user(user, annotation, [PermissionTypes.CRUD]) + + return AddUrlAnnotation( + ok=True, message="URL annotation created", annotation=annotation + ) + + class AddDocTypeAnnotation(graphene.Mutation): class Arguments: corpus_id = graphene.String( @@ -767,6 +916,14 @@ class Arguments: long_description = graphene.String() json = GenericScalar() annotation_label = graphene.String() + link_url = graphene.String( + required=False, + description=( + "Optional click-through URL for OC_URL annotations. Pass an " + "empty string to clear an existing URL. Restricted to " + "http(s):// or site-relative paths." + ), + ) class UpdateRelations(graphene.Mutation): diff --git a/config/graphql/mutations.py b/config/graphql/mutations.py index 15a6716d5..3b433e570 100644 --- a/config/graphql/mutations.py +++ b/config/graphql/mutations.py @@ -28,6 +28,7 @@ AddAnnotation, AddDocTypeAnnotation, AddRelationship, + AddUrlAnnotation, ApproveAnnotation, CreateNote, DeleteNote, @@ -236,6 +237,7 @@ class Mutation(graphene.ObjectType): # ANNOTATION MUTATIONS ###################################################### add_annotation = AddAnnotation.Field() + add_url_annotation = AddUrlAnnotation.Field() remove_annotation = RemoveAnnotation.Field() update_annotation = UpdateAnnotation.Field() add_doc_type_annotation = AddDocTypeAnnotation.Field() diff --git a/config/graphql/serializers.py b/config/graphql/serializers.py index 152ba1407..cffb215d6 100644 --- a/config/graphql/serializers.py +++ b/config/graphql/serializers.py @@ -210,9 +210,25 @@ class Meta: "creator_id", "parent", "parent_id", + "link_url", ] read_only_fields = ["id", "creator", "parent"] + def validate_link_url(self, value: str | None) -> str | None: + """Normalise empty strings to None and reject unsafe schemes. + + The frontend sends `link_url=""` to clear an existing URL; convert + that to None so the column ends up NULL. All non-empty values flow + through ``Annotation.validate_link_url`` to block ``javascript:`` + and other dangerous schemes before reaching persistence. + """ + from opencontractserver.annotations.models import validate_link_url as _validate + + if not value: + return None + _validate(value) + return value + def create(self, validated_data: dict) -> Annotation: """ Create a new `Annotation` instance, mapping `creator_id` and `parent_id` to their respective diff --git a/docs/assets/images/screenshots/auto/annotations--url-annotation--create-empty.png b/docs/assets/images/screenshots/auto/annotations--url-annotation--create-empty.png new file mode 100644 index 000000000..cc640e62d Binary files /dev/null and b/docs/assets/images/screenshots/auto/annotations--url-annotation--create-empty.png differ diff --git a/docs/assets/images/screenshots/auto/annotations--url-annotation--edit-prefilled.png b/docs/assets/images/screenshots/auto/annotations--url-annotation--edit-prefilled.png new file mode 100644 index 000000000..86e124a08 Binary files /dev/null and b/docs/assets/images/screenshots/auto/annotations--url-annotation--edit-prefilled.png differ diff --git a/docs/assets/images/screenshots/auto/annotations--url-annotation--error-empty.png b/docs/assets/images/screenshots/auto/annotations--url-annotation--error-empty.png new file mode 100644 index 000000000..f66579632 Binary files /dev/null and b/docs/assets/images/screenshots/auto/annotations--url-annotation--error-empty.png differ diff --git a/frontend/src/assets/configurations/constants.ts b/frontend/src/assets/configurations/constants.ts index 5386fd353..14f46b7dd 100644 --- a/frontend/src/assets/configurations/constants.ts +++ b/frontend/src/assets/configurations/constants.ts @@ -262,6 +262,21 @@ export const DOCUMENT_ANNOTATION_INDEX_MAX_DEPTH = 6; // Keep in sync with opencontractserver/constants/annotations.py export const STRUCTURAL_LABEL_PREFIX = "OC_"; export const OC_SECTION_LABEL = "OC_SECTION"; +// Annotations carrying the OC_URL label render as clickable hyperlinks; their +// ``linkUrl`` field is opened on click. Keep in sync with +// opencontractserver/constants/annotations.py. +export const OC_URL_LABEL = "OC_URL"; +// Sentinel id used when constructing a placeholder OC_URL ``AnnotationLabel`` +// before the server has assigned a real id (e.g. when the user is creating +// a new link annotation from a selection). Surfaced as a named constant so +// downstream code that needs to recognise "this label is still pending" +// has a single source of truth instead of comparing against a raw string. +export const PENDING_OC_URL_LABEL_ID = "__pending_oc_url__"; +// Default presentation for the OC_URL label. Mirrors the backend constants +// (``opencontractserver/constants/annotations.py``) so the placeholder used +// before the server has assigned a real label, the renderer's hyperlink +// styling, and the auto-created server-side label all agree. +export const OC_URL_LABEL_COLOR = "#2563EB"; // Document search/picker limits export const DOCUMENT_PICKER_SEARCH_LIMIT = 20; diff --git a/frontend/src/components/annotator/components/modals/CreateUrlAnnotationModal.tsx b/frontend/src/components/annotator/components/modals/CreateUrlAnnotationModal.tsx new file mode 100644 index 000000000..f2188b5f6 --- /dev/null +++ b/frontend/src/components/annotator/components/modals/CreateUrlAnnotationModal.tsx @@ -0,0 +1,143 @@ +import { SyntheticEvent, useEffect, useState } from "react"; + +import { + Modal, + ModalHeader, + ModalBody, + ModalFooter, + Button, +} from "@os-legal/ui"; + +import { isSafeUrl } from "../../utils/urlAnnotation"; + +interface CreateUrlAnnotationModalProps { + visible: boolean; + /** Text the user selected; shown read-only for context. */ + selectedText: string; + onCancel: () => void; + /** Called with the trimmed URL when the user confirms. */ + onConfirm: (url: string) => void; + /** Initial value, used by the edit-URL flow. */ + initialUrl?: string; +} + +/** + * Small modal that prompts the user for a target URL when turning a + * selection into an OC_URL link annotation. Validation reuses ``isSafeUrl`` + * from ``urlAnnotation.ts`` — the same allow-list (http(s) absolute or + * site-relative ``/...``) used by the renderer click-handler and the + * backend ``validate_link_url`` helper, so the three checks cannot drift. + */ +export const CreateUrlAnnotationModal = ({ + visible, + selectedText, + onCancel, + onConfirm, + initialUrl = "", +}: CreateUrlAnnotationModalProps) => { + const [url, setUrl] = useState(initialUrl); + const [error, setError] = useState(null); + + useEffect(() => { + if (visible) { + setUrl(initialUrl); + setError(null); + } + }, [visible, initialUrl]); + + const handleConfirm = (event: SyntheticEvent) => { + event.preventDefault(); + event.stopPropagation(); + const trimmed = url.trim(); + if (!trimmed) { + setError("URL is required."); + return; + } + if (!isSafeUrl(trimmed)) { + setError( + "URL must start with http://, https://, or '/' (site-relative path)." + ); + return; + } + onConfirm(trimmed); + }; + + return ( + + {initialUrl ? "Edit link target" : "Add link"} + +
e.stopPropagation()} + style={{ display: "flex", flexDirection: "column", gap: 12 }} + > + {selectedText && ( +
+ Selected text:{" "} + {selectedText} +
+ )} + + { + setUrl(e.target.value); + if (error) setError(null); + }} + onKeyDown={(e) => { + if (e.key === "Enter") { + handleConfirm(e); + } + }} + placeholder="https://example.com or /relative/path" + autoFocus + style={{ + padding: "8px 10px", + border: error ? "1px solid #dc2626" : "1px solid #d1d5db", + borderRadius: 4, + fontSize: 14, + }} + /> + {error && ( +
{error}
+ )} +
+
+ + + + +
+ ); +}; diff --git a/frontend/src/components/annotator/components/modals/__tests__/CreateUrlAnnotationModal.test.tsx b/frontend/src/components/annotator/components/modals/__tests__/CreateUrlAnnotationModal.test.tsx new file mode 100644 index 000000000..e7c0b39cc --- /dev/null +++ b/frontend/src/components/annotator/components/modals/__tests__/CreateUrlAnnotationModal.test.tsx @@ -0,0 +1,86 @@ +/** + * Unit-level coverage for ``CreateUrlAnnotationModal``. + * + * The Playwright component tests in ``frontend/tests/`` cover the modal + * end-to-end, but unit coverage drives the modal-body interaction + * handlers (``onMouseDown`` stopPropagation, error-clearing on input) + * which Playwright's snapshot tests don't reliably wire into the + * Istanbul lcov bundle picked up by codecov's ``frontend-unit`` flag. + */ +import React from "react"; +import { render, fireEvent, screen } from "@testing-library/react"; +import { describe, it, expect, vi } from "vitest"; + +import { CreateUrlAnnotationModal } from "../CreateUrlAnnotationModal"; + +describe("CreateUrlAnnotationModal", () => { + it("stopPropagation on body mousedown does not bubble to wrapping handler", () => { + // The modal body wraps its children in a div that swallows mousedown + // so a click inside the modal cannot start a selection-drag on the + // underlying SelectionLayer / TxtAnnotator. + const parentHandler = vi.fn(); + render( +
+ +
+ ); + const input = screen.getByPlaceholderText(/https:\/\//); + fireEvent.mouseDown(input); + // The body wrapper's stopPropagation must prevent the bubble. + expect(parentHandler).not.toHaveBeenCalled(); + }); + + it("clears an existing error as soon as the user edits the input", () => { + // The change handler is ``setUrl + if (error) setError(null)``. The + // ``if (error)`` partial branch (covered when error is non-null) is + // exercised by first triggering a validation failure, then typing. + const onConfirm = vi.fn(); + render( + + ); + + const input = screen.getByPlaceholderText(/https:\/\//) as HTMLInputElement; + + // 1. Type an unsafe URL and submit so ``setError`` fires. + fireEvent.change(input, { target: { value: "javascript:alert(1)" } }); + const createBtn = screen.getByRole("button", { name: /create link/i }); + fireEvent.click(createBtn); + // The component renders an inline error message when validation fails. + expect(screen.getByText(/must start with/i)).toBeInTheDocument(); + + // 2. Edit the input; the partial-branch (error truthy → clear) must + // fire and the error message must disappear from the DOM. + fireEvent.change(input, { target: { value: "https://safe.example.com" } }); + expect(screen.queryByText(/must start with/i)).not.toBeInTheDocument(); + }); + + it("trims leading/trailing whitespace before passing the URL upstream", () => { + // Mirror of the model layer's whitespace-stripping contract — the + // modal must hand the parent component a canonical URL string. + const onConfirm = vi.fn(); + render( + + ); + const input = screen.getByPlaceholderText(/https:\/\//) as HTMLInputElement; + fireEvent.change(input, { + target: { value: " https://example.com/with-spaces " }, + }); + fireEvent.click(screen.getByRole("button", { name: /create link/i })); + expect(onConfirm).toHaveBeenCalledWith("https://example.com/with-spaces"); + }); +}); diff --git a/frontend/src/components/annotator/components/wrappers/TxtAnnotatorWrapper.tsx b/frontend/src/components/annotator/components/wrappers/TxtAnnotatorWrapper.tsx index 517333959..74a23992d 100644 --- a/frontend/src/components/annotator/components/wrappers/TxtAnnotatorWrapper.tsx +++ b/frontend/src/components/annotator/components/wrappers/TxtAnnotatorWrapper.tsx @@ -10,6 +10,7 @@ import { useSetAtom } from "jotai"; import { useApproveAnnotation, useCreateAnnotation, + useCreateUrlAnnotation, useDeleteAnnotation, usePdfAnnotations, useRejectAnnotation, @@ -65,6 +66,7 @@ export const TxtAnnotatorWrapper: React.FC = ({ const { showStructural } = useAnnotationDisplay(); const handleCreateAnnotation = useCreateAnnotation(); + const handleCreateUrlAnnotation = useCreateUrlAnnotation(); const handleDeleteAnnotation = useDeleteAnnotation(); const handleUpdateAnnotation = useUpdateAnnotation(); const handleApproveAnnotation = useApproveAnnotation(); @@ -201,6 +203,7 @@ export const TxtAnnotatorWrapper: React.FC = ({ read_only={readOnly} allowInput={allowInput} createAnnotation={handleCreateAnnotation} + createUrlAnnotation={handleCreateUrlAnnotation} updateAnnotation={handleUpdateAnnotation} approveAnnotation={handleApproveAnnotation} rejectAnnotation={handleRejectAnnotation} diff --git a/frontend/src/components/annotator/display/components/Selection.tsx b/frontend/src/components/annotator/display/components/Selection.tsx index a12599c08..700bd5579 100644 --- a/frontend/src/components/annotator/display/components/Selection.tsx +++ b/frontend/src/components/annotator/display/components/Selection.tsx @@ -1,8 +1,9 @@ import React, { useState, useEffect, useRef } from "react"; import _ from "lodash"; import styled from "styled-components"; +import { useNavigate } from "react-router-dom"; -import { Pencil, Trash2 } from "lucide-react"; +import { ExternalLink, Pencil, Trash2 } from "lucide-react"; import { VerticallyJustifiedEndDiv } from "../../sidebar/common"; @@ -21,6 +22,10 @@ import RadialButtonCloud, { } from "../../../widgets/buttons/RadialButtonCloud"; import { SelectionTokenGroup } from "./SelectionTokenGroup"; import { EditLabelModal } from "../../components/modals/EditLabelModal"; +import { CreateUrlAnnotationModal } from "../../components/modals/CreateUrlAnnotationModal"; +import { isUrlAnnotation, openAnnotationUrl } from "../../utils/urlAnnotation"; +import { useUpdateAnnotation } from "../../hooks/AnnotationHooks"; +import { OC_URL_LABEL } from "../../../../assets/configurations/constants"; import { useReactiveVar } from "@apollo/client"; import { authToken } from "../../../../graphql/cache"; import { PDFPageInfo } from "../../types/pdf"; @@ -118,11 +123,14 @@ export const Selection: React.FC = ({ showInfo = true, }) => { const auth_token = useReactiveVar(authToken); + const navigate = useNavigate(); const [hovered, setHovered] = useState(false); const [isEditLabelModalVisible, setIsEditLabelModalVisible] = useState(false); + const [isEditUrlModalVisible, setIsEditUrlModalVisible] = useState(false); const [cloudVisible, setCloudVisible] = useState(false); const [hidden, setHidden] = useState(false); const cloudRef = useRef(null); + const updateAnnotation = useUpdateAnnotation(); const { showBoundingBoxes, showSelectedOnly, showLabels } = useAnnotationDisplay(); @@ -214,7 +222,24 @@ export const Selection: React.FC = ({ deleteAnnotation(annotation.id); }; - const onShiftClick = () => { + const isLinkAnnotation = isUrlAnnotation(annotation); + + const onShiftClick = (event?: React.MouseEvent) => { + // OC_URL annotations act as hyperlinks on plain click. Holding Shift or + // a modifier key (Ctrl/Cmd) falls back to the normal "toggle selection" + // behaviour so authors can still pick a link annotation to edit or delete + // it from the radial menu. + if ( + isLinkAnnotation && + !event?.shiftKey && + !event?.metaKey && + !event?.ctrlKey + ) { + // Pass ``navigate`` so site-relative paths stay in the SPA instead + // of triggering a hard page reload that would blow away Apollo cache. + if (openAnnotationUrl(annotation, navigate)) return; + } + const current = selectedAnnotations.slice(0); if (current.some((other) => other === annotation.id)) { const next = current.filter((other) => other !== annotation.id); @@ -276,6 +301,7 @@ export const Selection: React.FC = ({ bounds={bounds} onHover={setHovered} onClick={onShiftClick} + clickThroughOnPlainClick={isLinkAnnotation} approved={approved} rejected={rejected} selected={selected} @@ -323,8 +349,17 @@ export const Selection: React.FC = ({ whiteSpace: "nowrap", overflowX: "visible", marginLeft: "8px", + display: "flex", + alignItems: "center", + gap: "4px", }} > + {isLinkAnnotation && ( + + )} {label.text} {annotation.myPermissions.includes( @@ -340,7 +375,16 @@ export const Selection: React.FC = ({ }} onClick={(e: React.MouseEvent) => { e.stopPropagation(); - setIsEditLabelModalVisible(true); + // OC_URL annotations get the link-target editor; + // every other annotation gets the standard label + // editor. + if ( + annotation.annotationLabel?.text === OC_URL_LABEL + ) { + setIsEditUrlModalVisible(true); + } else { + setIsEditLabelModalVisible(true); + } }} onMouseDown={(e: React.MouseEvent) => { e.stopPropagation(); @@ -412,6 +456,18 @@ export const Selection: React.FC = ({ onHide={() => setIsEditLabelModalVisible(false)} /> )} + {isEditUrlModalVisible && ( + setIsEditUrlModalVisible(false)} + onConfirm={(url) => { + updateAnnotation(annotation.update({ linkUrl: url })); + setIsEditUrlModalVisible(false); + }} + /> + )} ); }; diff --git a/frontend/src/components/annotator/display/components/SelectionBoundary.tsx b/frontend/src/components/annotator/display/components/SelectionBoundary.tsx index d20cef154..49268de17 100644 --- a/frontend/src/components/annotator/display/components/SelectionBoundary.tsx +++ b/frontend/src/components/annotator/display/components/SelectionBoundary.tsx @@ -26,7 +26,15 @@ interface SelectionBoundaryProps { children?: React.ReactNode; annotationId?: string; onHover?: (hovered: boolean) => void; - onClick?: () => void; + onClick?: (event?: React.MouseEvent) => void; + /** + * When true, plain (non-shift) clicks also invoke ``onClick``. Used for + * hyperlink-style annotations (OC_URL) where a single click should open + * the link. When false (default) the shift-click-to-select semantic is + * preserved, so plain clicks fall through to canvas handlers and don't + * interfere with creating a new annotation underneath. + */ + clickThroughOnPlainClick?: boolean; approved?: boolean; rejected?: boolean; } @@ -94,6 +102,7 @@ export const SelectionBoundary: React.FC = ({ children, onHover, onClick, + clickThroughOnPlainClick = false, selected, approved, rejected, @@ -140,15 +149,17 @@ export const SelectionBoundary: React.FC = ({ const handleClick = (e: React.MouseEvent) => { if (isCreatingAnnotation) return; - if (e.shiftKey && onClick) { + if (!onClick) return; + if (e.shiftKey || clickThroughOnPlainClick) { e.stopPropagation(); - onClick(); + onClick(e); } }; const handleMouseDown = (e: React.MouseEvent) => { if (isCreatingAnnotation) return; - if (e.shiftKey && onClick) { + if (!onClick) return; + if (e.shiftKey || clickThroughOnPlainClick) { e.stopPropagation(); } }; @@ -181,7 +192,10 @@ export const SelectionBoundary: React.FC = ({ $bounds={bounds} $approved={approved} $rejected={rejected} - style={{ pointerEvents: isCreatingAnnotation ? "none" : "auto" }} + style={{ + pointerEvents: isCreatingAnnotation ? "none" : "auto", + cursor: clickThroughOnPlainClick ? "pointer" : undefined, + }} > {children || null} diff --git a/frontend/src/components/annotator/display/components/__tests__/SelectionBoundary.test.tsx b/frontend/src/components/annotator/display/components/__tests__/SelectionBoundary.test.tsx new file mode 100644 index 000000000..e567beae4 --- /dev/null +++ b/frontend/src/components/annotator/display/components/__tests__/SelectionBoundary.test.tsx @@ -0,0 +1,163 @@ +/** + * Unit tests for SelectionBoundary click semantics. + * + * Covers the ``clickThroughOnPlainClick`` prop introduced for OC_URL + * hyperlink annotations: plain (non-shift) clicks must fire ``onClick`` + * when the prop is on, but stay inert otherwise. Exercising both branches + * pins the behaviour that lets clickable-link annotations open without + * accidentally invading the normal selection-only semantics. + */ +import React from "react"; +import { render, fireEvent } from "@testing-library/react"; +import { describe, it, expect, vi } from "vitest"; + +import { SelectionBoundary } from "../SelectionBoundary"; + +vi.mock("../../../hooks/useAnnotationRefs", () => ({ + useAnnotationRefs: () => ({ + registerRef: vi.fn(), + unregisterRef: vi.fn(), + }), +})); + +vi.mock("jotai", async () => { + const actual = await vi.importActual("jotai"); + return { + ...actual, + // ``isCreatingAnnotation`` is read via ``useAtomValue`` — return + // ``false`` so the click path is not short-circuited by the + // "drawing a new selection" branch. + useAtomValue: vi.fn(() => false), + }; +}); + +const bounds = { left: 0, top: 0, right: 100, bottom: 50 }; + +// Required props that every test needs — extracted so the per-test JSX +// stays focused on the prop under test (clickThroughOnPlainClick, etc.). +const baseProps = { + hidden: false, + selected: false, +} as const; + +describe("SelectionBoundary click semantics", () => { + it("does NOT call onClick on a plain click by default", () => { + const onClick = vi.fn(); + const { container } = render( + + ); + const span = container.querySelector("span") as HTMLElement; + fireEvent.click(span); + expect(onClick).not.toHaveBeenCalled(); + }); + + it("calls onClick on a shift-click (default behaviour)", () => { + const onClick = vi.fn(); + const { container } = render( + + ); + const span = container.querySelector("span") as HTMLElement; + fireEvent.click(span, { shiftKey: true }); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it("calls onClick on a plain click when clickThroughOnPlainClick is true", () => { + // OC_URL hyperlink path: the surrounding renderer wires + // ``clickThroughOnPlainClick`` so a plain click opens the URL. + const onClick = vi.fn(); + const { container } = render( + + ); + const span = container.querySelector("span") as HTMLElement; + fireEvent.click(span); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it("shows pointer cursor when clickThroughOnPlainClick is true", () => { + // Visible affordance: hyperlink-style annotations must use a pointer + // cursor so the user knows the span is clickable. + const { container } = render( + + ); + const span = container.querySelector("span") as HTMLElement; + expect(span.style.cursor).toBe("pointer"); + }); + + it("does NOT set pointer cursor when clickThroughOnPlainClick is false", () => { + const { container } = render( + + ); + const span = container.querySelector("span") as HTMLElement; + expect(span.style.cursor).toBe(""); + }); + + it("noop on mouseDown without onClick prop", () => { + // ``handleMouseDown``'s early-return when ``!onClick`` is the + // codecov-flagged guard branch. Just rendering without onClick and + // firing the event must not throw. + const { container } = render( + + ); + const span = container.querySelector("span") as HTMLElement; + expect(() => fireEvent.mouseDown(span, { shiftKey: true })).not.toThrow(); + }); + + it("stops mouseDown propagation on shift+mousedown", () => { + // The boundary swallows shift+mousedown so the underlying selection + // layer doesn't restart a drag-selection over an existing annotation. + const onClick = vi.fn(); + const parentClick = vi.fn(); + const { container } = render( +
+ +
+ ); + const span = container.querySelector("span") as HTMLElement; + fireEvent.mouseDown(span, { shiftKey: true }); + // The parent handler must not see the event due to stopPropagation. + expect(parentClick).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/annotator/hooks/AnnotationHooks.tsx b/frontend/src/components/annotator/hooks/AnnotationHooks.tsx index fdea9a22a..3e8f641ff 100644 --- a/frontend/src/components/annotator/hooks/AnnotationHooks.tsx +++ b/frontend/src/components/annotator/hooks/AnnotationHooks.tsx @@ -4,6 +4,7 @@ import { useMutation } from "@apollo/client"; import { toast } from "react-toastify"; import { REQUEST_ADD_ANNOTATION, + REQUEST_ADD_URL_ANNOTATION, REQUEST_DELETE_ANNOTATION, REQUEST_UPDATE_ANNOTATION, REQUEST_ADD_DOC_TYPE_ANNOTATION, @@ -14,6 +15,8 @@ import { REJECT_ANNOTATION, NewAnnotationInputType, NewAnnotationOutputType, + NewUrlAnnotationInputType, + NewUrlAnnotationOutputType, RemoveAnnotationInputType, RemoveAnnotationOutputType, UpdateAnnotationInputType, @@ -281,7 +284,9 @@ export function useCreateAnnotation() { false, false, false, - createdAnnotationData.id + createdAnnotationData.id, + undefined, + createdAnnotationData.linkUrl ?? null ); } else { newAnnotation = new ServerTokenAnnotation( @@ -294,7 +299,9 @@ export function useCreateAnnotation() { false, false, false, - createdAnnotationData.id + createdAnnotationData.id, + undefined, + createdAnnotationData.linkUrl ?? null ); } @@ -322,6 +329,111 @@ export function useCreateAnnotation() { ]); } +/** + * Hook to create an OC_URL annotation: a clickable hyperlink anchored to + * the supplied selection. Internally calls the ``addUrlAnnotation`` mutation, + * which auto-ensures the OC_URL label exists in the corpus. + */ +export function useCreateUrlAnnotation() { + const { addMultipleAnnotations } = usePdfAnnotations(); + const selectedDocument = useAtomValue(selectedDocumentAtom); + const { selectedCorpus } = useCorpusState(); + + const [createUrlAnnotation] = useMutation< + NewUrlAnnotationOutputType, + NewUrlAnnotationInputType + >(REQUEST_ADD_URL_ANNOTATION); + + const handleCreateUrlAnnotation = useCallback( + async ( + annotation: ServerTokenAnnotation | ServerSpanAnnotation, + linkUrl: string + ) => { + if (!selectedCorpus || !selectedDocument) { + toast.warning("No corpus or document selected"); + return; + } + const trimmedUrl = linkUrl.trim(); + if (!trimmedUrl) { + toast.warning("URL is required for link annotation"); + return; + } + + try { + const result = await createUrlAnnotation({ + variables: { + json: annotation.json, + page: annotation.page, + rawText: annotation.rawText, + corpusId: selectedCorpus.id, + documentId: selectedDocument.id, + annotationType: + annotation instanceof ServerSpanAnnotation + ? LabelType.SpanLabel + : LabelType.TokenLabel, + linkUrl: trimmedUrl, + }, + }); + + const payload = result?.data?.addUrlAnnotation; + if (!payload?.ok || !payload.annotation) { + toast.error( + `Unable to create link annotation: ${ + payload?.message ?? "unknown error" + }` + ); + return; + } + + const created = payload.annotation; + const newAnnotation: ServerTokenAnnotation | ServerSpanAnnotation = + isSpanBasedFileType(selectedDocument.fileType) + ? new ServerSpanAnnotation( + created.page, + created.annotationLabel, + created.rawText, + false, + created.json as SpanAnnotationJson, + getPermissions(created.myPermissions || []), + false, + false, + false, + created.id, + undefined, + created.linkUrl + ) + : new ServerTokenAnnotation( + created.page, + created.annotationLabel, + created.rawText, + false, + created.json ?? {}, + getPermissions(created.myPermissions || []), + false, + false, + false, + created.id, + undefined, + created.linkUrl + ); + + addMultipleAnnotations([newAnnotation]); + toast.success("Link annotation created"); + } catch (error: unknown) { + toast.error(`Unable to create link annotation: ${error}`); + } + }, + [ + selectedDocument, + selectedCorpus, + createUrlAnnotation, + addMultipleAnnotations, + ] + ); + + return handleCreateUrlAnnotation; +} + /** * Hook to update an existing annotation. */ @@ -349,6 +461,9 @@ export function useUpdateAnnotation() { rawText: annotation.rawText, page: annotation.page, annotationLabel: annotation.annotationLabel.id, + // ``null`` is sent as-is and clears the column server-side; + // ``undefined`` is dropped by Apollo before serialising the request. + linkUrl: annotation.linkUrl ?? null, }, }); @@ -366,7 +481,9 @@ export function useUpdateAnnotation() { false, false, false, - annotation.id + annotation.id, + undefined, + annotation.linkUrl ?? null ); } else { updatedAnnotation = new ServerTokenAnnotation( @@ -379,7 +496,9 @@ export function useUpdateAnnotation() { false, false, false, - annotation.id + annotation.id, + undefined, + annotation.linkUrl ?? null ); } diff --git a/frontend/src/components/annotator/hooks/__tests__/AnnotationHooks.test.tsx b/frontend/src/components/annotator/hooks/__tests__/AnnotationHooks.test.tsx index e1d044ee6..72c02336d 100644 --- a/frontend/src/components/annotator/hooks/__tests__/AnnotationHooks.test.tsx +++ b/frontend/src/components/annotator/hooks/__tests__/AnnotationHooks.test.tsx @@ -29,6 +29,7 @@ import { useStructuralAnnotations, useInitialAnnotations, useCreateAnnotation, + useCreateUrlAnnotation, useUpdateAnnotation, useDeleteAnnotation, useApproveAnnotation, @@ -50,6 +51,7 @@ import { selectedDocumentAtom } from "../../context/DocumentAtom"; import { corpusStateAtom } from "../../context/CorpusAtom"; import { REQUEST_ADD_ANNOTATION, + REQUEST_ADD_URL_ANNOTATION, REQUEST_DELETE_ANNOTATION, REQUEST_UPDATE_ANNOTATION, REQUEST_ADD_DOC_TYPE_ANNOTATION, @@ -59,6 +61,7 @@ import { APPROVE_ANNOTATION, REJECT_ANNOTATION, } from "../../../../graphql/mutations"; +import { OC_URL_LABEL } from "../../../../assets/configurations/constants"; import { LabelType } from "../../types/enums"; import { PermissionTypes } from "../../../types"; import type { AnnotationLabelType } from "../../../../types/graphql-api"; @@ -144,6 +147,13 @@ interface WrapperOptions { mocks?: MockedResponse[]; withCorpus?: boolean; withDocument?: boolean; + /** + * Optional override for the mock document's ``fileType``. Defaults to + * ``application/txt`` (span-based). Use ``application/pdf`` to drive + * the Token-annotation branch in hooks that switch on file type + * (e.g. ``useCreateUrlAnnotation`` for PDFs). + */ + fileType?: string; initialAnnotations?: (ServerSpanAnnotation | ServerTokenAnnotation)[]; initialRelations?: RelationGroup[]; initialDocTypes?: DocTypeAnnotation[]; @@ -154,14 +164,21 @@ const buildWrapper = (options: WrapperOptions = {}) => { mocks = [], withCorpus = true, withDocument = true, + fileType, initialAnnotations = [], initialRelations = [], initialDocTypes = [], } = options; + const documentForAtom = withDocument + ? fileType + ? { ...mockDocument, fileType } + : mockDocument + : null; + const Hydrate = ({ children }: { children: ReactNode }) => { useHydrateAtoms([ - [selectedDocumentAtom, withDocument ? mockDocument : null], + [selectedDocumentAtom, documentForAtom], [ corpusStateAtom, { @@ -476,6 +493,316 @@ describe("AnnotationHooks", () => { }); }); + describe("useCreateUrlAnnotation", () => { + const ocUrlLabel: AnnotationLabelType = { + id: "label-url", + text: OC_URL_LABEL, + color: "#2563EB", + icon: "link", + description: "url", + labelType: LabelType.SpanLabel, + }; + + it("short-circuits (no mutation) when corpus is missing", async () => { + // The hook must guard against missing parents — otherwise Apollo + // would throw "No more mocked responses". + const ann = makeSpan("local"); + const { result } = renderHook( + () => ({ + create: useCreateUrlAnnotation(), + state: usePdfAnnotations(), + }), + { wrapper: buildWrapper({ withCorpus: false }) } + ); + + await act(async () => { + await result.current.create(ann, "https://example.com"); + }); + + expect(result.current.state.pdfAnnotations.annotations).toHaveLength(0); + }); + + it("short-circuits when linkUrl is blank/whitespace", async () => { + // A trimmed-empty URL is treated identically to an omitted URL: the + // mutation never fires, and toast.warning is shown to the user. + const ann = makeSpan("local"); + const { result } = renderHook( + () => ({ + create: useCreateUrlAnnotation(), + state: usePdfAnnotations(), + }), + { wrapper: buildWrapper() } + ); + + await act(async () => { + await result.current.create(ann, " "); + }); + + expect(result.current.state.pdfAnnotations.annotations).toHaveLength(0); + }); + + it("adds the server-returned URL annotation on success", async () => { + const localAnn = makeSpan("local-tmp-url", 0, 5, "hello"); + const serverId = "server-url-1"; + const linkUrl = "https://example.com/a"; + + const mocks: MockedResponse[] = [ + { + request: { + query: REQUEST_ADD_URL_ANNOTATION, + variables: { + json: localAnn.json, + documentId: mockDocument.id, + corpusId: mockCorpus.id, + rawText: localAnn.rawText, + page: localAnn.page, + annotationType: LabelType.SpanLabel, + linkUrl, + }, + }, + result: { + data: { + addUrlAnnotation: { + ok: true, + message: "URL annotation created", + annotation: { + id: serverId, + page: 0, + rawText: "hello", + json: { start: 0, end: 5 }, + linkUrl, + annotationType: LabelType.SpanLabel, + annotationLabel: ocUrlLabel, + myPermissions: ["CAN_READ", "CAN_UPDATE", "CAN_REMOVE"], + isPublic: false, + }, + }, + }, + }, + }, + ]; + + const { result } = renderHook( + () => ({ + create: useCreateUrlAnnotation(), + state: usePdfAnnotations(), + }), + { wrapper: buildWrapper({ mocks }) } + ); + + await act(async () => { + await result.current.create(localAnn, linkUrl); + }); + + // Key contract: the annotation stored in state carries the server + // id and the linkUrl returned by the server. + const stored = result.current.state.pdfAnnotations.annotations; + expect(stored).toHaveLength(1); + expect(stored[0].id).toBe(serverId); + expect(stored[0].linkUrl).toBe(linkUrl); + expect(stored[0].annotationLabel.text).toBe(OC_URL_LABEL); + }); + + it("falls back to toast.error when the mutation throws", async () => { + // Defence in depth: a network error or thrown Apollo error must NOT + // leave the user staring at an "unsaved changes" indicator with + // nothing in state. The hook swallows the error after toasting. + const localAnn = makeSpan("local-throws", 0, 5, "hello"); + const mocks: MockedResponse[] = [ + { + request: { + query: REQUEST_ADD_URL_ANNOTATION, + variables: { + json: localAnn.json, + documentId: mockDocument.id, + corpusId: mockCorpus.id, + rawText: localAnn.rawText, + page: localAnn.page, + annotationType: LabelType.SpanLabel, + linkUrl: "https://example.com", + }, + }, + error: new Error("network exploded"), + }, + ]; + + const { result } = renderHook( + () => ({ + create: useCreateUrlAnnotation(), + state: usePdfAnnotations(), + }), + { wrapper: buildWrapper({ mocks }) } + ); + + await act(async () => { + await result.current.create(localAnn, "https://example.com"); + }); + + // No annotation added on the throw path. + expect(result.current.state.pdfAnnotations.annotations).toHaveLength(0); + }); + + it("does not add to state when the server returns ok=false", async () => { + // Defence-in-depth: an unsafe URL slipping past client-side validation + // would be rejected by the server; the hook must NOT push anything + // into local state in that case. + const localAnn = makeSpan("local-tmp-url", 0, 5, "hello"); + const mocks: MockedResponse[] = [ + { + request: { + query: REQUEST_ADD_URL_ANNOTATION, + variables: { + json: localAnn.json, + documentId: mockDocument.id, + corpusId: mockCorpus.id, + rawText: localAnn.rawText, + page: localAnn.page, + annotationType: LabelType.SpanLabel, + linkUrl: "https://example.com", + }, + }, + result: { + data: { + addUrlAnnotation: { + ok: false, + message: "link_url must be an http(s):// URL", + annotation: null, + }, + }, + }, + }, + ]; + + const { result } = renderHook( + () => ({ + create: useCreateUrlAnnotation(), + state: usePdfAnnotations(), + }), + { wrapper: buildWrapper({ mocks }) } + ); + + await act(async () => { + await result.current.create(localAnn, "https://example.com"); + }); + + expect(result.current.state.pdfAnnotations.annotations).toHaveLength(0); + }); + + it("short-circuits without a selected document", async () => { + // The hook checks both ``selectedCorpus`` *and* ``selectedDocument``. + // The corpus-only short-circuit is covered above; this exercises the + // document branch (else-leg of the ``!selectedCorpus || !selectedDocument`` + // guard) so the no-document log/warning path is locked in. + const ann = makeSpan("local"); + const { result } = renderHook( + () => ({ + create: useCreateUrlAnnotation(), + state: usePdfAnnotations(), + }), + { wrapper: buildWrapper({ withDocument: false }) } + ); + + await act(async () => { + await result.current.create(ann, "https://example.com"); + }); + + expect(result.current.state.pdfAnnotations.annotations).toHaveLength(0); + }); + + it("creates a ServerTokenAnnotation for PDF documents on success", async () => { + // Companion to the Span-branch test above. PDFs use Token annotations, + // so the hook's ``isSpanBasedFileType(...) ? Span : Token`` ternary must + // produce a ``ServerTokenAnnotation`` with the server-returned linkUrl + // when the selected document is a PDF. Without this test, the Token + // branch (and ``created.linkUrl`` passthrough on the Token wrapper) + // is never exercised — exactly the 3 misses + 4 partials codecov + // flagged in ``AnnotationHooks.tsx`` for this PR. + const pdfTokenAnn = new ServerTokenAnnotation( + 0, + mockLabel, + "hello", + false, + { + 0: { bounds: {}, tokensJsons: [], rawText: "hello" }, + } as unknown as Record, + [ + PermissionTypes.CAN_READ, + PermissionTypes.CAN_UPDATE, + PermissionTypes.CAN_REMOVE, + ], + false, + false, + false, + "local-pdf-tmp" + ); + const serverId = "server-pdf-url"; + const linkUrl = "https://example.com/pdf"; + + const mocks: MockedResponse[] = [ + { + request: { + query: REQUEST_ADD_URL_ANNOTATION, + variables: { + json: pdfTokenAnn.json, + documentId: mockDocument.id, + corpusId: mockCorpus.id, + rawText: pdfTokenAnn.rawText, + page: pdfTokenAnn.page, + annotationType: LabelType.TokenLabel, + linkUrl, + }, + }, + result: { + data: { + addUrlAnnotation: { + ok: true, + message: "OK", + annotation: { + id: serverId, + page: 0, + rawText: "hello", + json: { + "0": { bounds: {}, tokensJsons: [], rawText: "hello" }, + }, + linkUrl, + annotationType: LabelType.TokenLabel, + annotationLabel: ocUrlLabel, + myPermissions: ["CAN_READ", "CAN_UPDATE", "CAN_REMOVE"], + isPublic: false, + }, + }, + }, + }, + }, + ]; + + const { result } = renderHook( + () => ({ + create: useCreateUrlAnnotation(), + state: usePdfAnnotations(), + }), + { + wrapper: buildWrapper({ + mocks, + fileType: "application/pdf", + }), + } + ); + + await act(async () => { + await result.current.create(pdfTokenAnn, linkUrl); + }); + + const stored = result.current.state.pdfAnnotations.annotations; + expect(stored).toHaveLength(1); + expect(stored[0].id).toBe(serverId); + expect(stored[0].linkUrl).toBe(linkUrl); + // Critical assertion: the hook must have produced a Token annotation, + // not a Span one, when the document is a PDF. + expect(stored[0]).toBeInstanceOf(ServerTokenAnnotation); + }); + }); + describe("useUpdateAnnotation", () => { it("replaces the annotation in state on a successful mutation", async () => { const existing = makeSpan("ann-1", 0, 5, "hello"); @@ -489,6 +816,10 @@ describe("AnnotationHooks", () => { rawText: existing.rawText, page: existing.page, annotationLabel: mockLabel.id, + // useUpdateAnnotation always sends linkUrl (null clears the + // column server-side; non-null sets it). The mock must match + // these variables exactly or Apollo never resolves. + linkUrl: null, }, }, result: { @@ -518,6 +849,75 @@ describe("AnnotationHooks", () => { expect(result.current.state.pdfAnnotations.unsavedChanges).toBe(true); }); + it("preserves linkUrl on the Token branch for PDF documents", async () => { + // The PDF (Token) branch in useUpdateAnnotation has the same + // ``annotation.linkUrl ?? null`` constructor parameter as the Span + // branch, but a separate code path. Without a PDF-typed wrapper, + // the Token-branch line (501 in AnnotationHooks.tsx) stays unhit + // and codecov flags it as a missed patch. + const existing = new ServerTokenAnnotation( + 0, + mockLabel, + "hello", + false, + { + 0: { bounds: {}, tokensJsons: [], rawText: "hello" }, + } as unknown as Record, + [ + PermissionTypes.CAN_READ, + PermissionTypes.CAN_UPDATE, + PermissionTypes.CAN_REMOVE, + ], + false, + false, + false, + "ann-pdf-1", + undefined, + "https://example.com/keep-on-update" + ); + const mocks: MockedResponse[] = [ + { + request: { + query: REQUEST_UPDATE_ANNOTATION, + variables: { + id: existing.id, + json: existing.json, + rawText: existing.rawText, + page: existing.page, + annotationLabel: mockLabel.id, + linkUrl: existing.linkUrl, + }, + }, + result: { + data: { updateAnnotation: { ok: true, message: "ok" } }, + }, + }, + ]; + + const { result } = renderHook( + () => ({ + update: useUpdateAnnotation(), + state: usePdfAnnotations(), + }), + { + wrapper: buildWrapper({ + mocks, + initialAnnotations: [existing], + fileType: "application/pdf", + }), + } + ); + + await act(async () => { + await result.current.update(existing); + }); + + const stored = result.current.state.pdfAnnotations.annotations; + expect(stored).toHaveLength(1); + expect(stored[0]).toBeInstanceOf(ServerTokenAnnotation); + expect(stored[0].linkUrl).toBe("https://example.com/keep-on-update"); + }); + it("updates one annotation in place without dropping siblings", async () => { // Regression: earlier versions called replaceAnnotations([updated]) // which collapsed the full list to the one updated annotation. diff --git a/frontend/src/components/annotator/renderers/pdf/PDF.tsx b/frontend/src/components/annotator/renderers/pdf/PDF.tsx index 708b55a14..a16ac892a 100644 --- a/frontend/src/components/annotator/renderers/pdf/PDF.tsx +++ b/frontend/src/components/annotator/renderers/pdf/PDF.tsx @@ -107,6 +107,15 @@ interface PDFProps { read_only: boolean; containerWidth?: number | null; createAnnotationHandler: (annotation: ServerTokenAnnotation) => Promise; + /** + * Optional handler that creates an OC_URL link annotation. Forwarded + * down to ``SelectionLayer`` so it can present an "Add link…" action in + * the selection menu. Omitted by callers that don't support links. + */ + createUrlAnnotationHandler?: ( + annotation: ServerTokenAnnotation, + url: string + ) => Promise; } // Shared render coordination state @@ -122,6 +131,7 @@ export const PDF: React.FC = ({ read_only, containerWidth, createAnnotationHandler, + createUrlAnnotationHandler, }) => { const { pages } = usePages(); const setViewStateError = useSetViewStateError(); @@ -563,6 +573,7 @@ export const PDF: React.FC = ({ onError={setViewStateError} containerWidth={containerWidth} createAnnotationHandler={createAnnotationHandler} + createUrlAnnotationHandler={createUrlAnnotationHandler} onZoomRenderRequest={requestPageRender} /> )} diff --git a/frontend/src/components/annotator/renderers/pdf/PDFPage.tsx b/frontend/src/components/annotator/renderers/pdf/PDFPage.tsx index 00e923e71..7bf892903 100644 --- a/frontend/src/components/annotator/renderers/pdf/PDFPage.tsx +++ b/frontend/src/components/annotator/renderers/pdf/PDFPage.tsx @@ -51,6 +51,11 @@ const CanvasWrapper = styled.div` interface PDFPageProps extends PageProps { containerWidth?: number | null; createAnnotationHandler: (annotation: ServerTokenAnnotation) => Promise; + /** Optional OC_URL link-annotation creator forwarded to SelectionLayer. */ + createUrlAnnotationHandler?: ( + annotation: ServerTokenAnnotation, + url: string + ) => Promise; onZoomRenderRequest?: ( pageNumber: number, renderer: any, @@ -73,6 +78,7 @@ export const PDFPage = ({ onError, containerWidth, createAnnotationHandler, + createUrlAnnotationHandler, onZoomRenderRequest, }: PDFPageProps) => { const canvasRef = useRef(null); @@ -568,6 +574,7 @@ export const PDFPage = ({ read_only={read_only} activeSpanLabel={activeSpanLabel ?? null} createAnnotation={createAnnotationHandler} + createUrlAnnotation={createUrlAnnotationHandler} pageNumber={pageInfo.page.pageNumber - 1} /> {pageAnnotationComponents} diff --git a/frontend/src/components/annotator/renderers/pdf/SelectionLayer.tsx b/frontend/src/components/annotator/renderers/pdf/SelectionLayer.tsx index 5536b5455..b89e2704d 100644 --- a/frontend/src/components/annotator/renderers/pdf/SelectionLayer.tsx +++ b/frontend/src/components/annotator/renderers/pdf/SelectionLayer.tsx @@ -17,7 +17,9 @@ import { useCorpusState } from "../../context/CorpusAtom"; import { useAnnotationSelection } from "../../context/UISettingsAtom"; import { useAtom, useAtomValue } from "jotai"; import { isCreatingAnnotationAtom } from "../../context/UISettingsAtom"; -import { Copy, Tag, X, AlertCircle, Settings, Link } from "lucide-react"; +import { Copy, Tag, X, AlertCircle, Settings, Link, Link2 } from "lucide-react"; +import { CreateUrlAnnotationModal } from "../../components/modals/CreateUrlAnnotationModal"; +import { LabelType } from "../../types/enums"; import { SelectionActionMenu, ActionMenuItem, @@ -34,6 +36,9 @@ import { } from "../../../../utils/textBlockEncoding"; import { clampMenuPosition } from "../../../../utils/layout"; import { + OC_URL_LABEL, + OC_URL_LABEL_COLOR, + PENDING_OC_URL_LABEL_ID, SELECTION_MENU_COOLDOWN_MS, Z_INDEX, } from "../../../../assets/configurations/constants"; @@ -43,6 +48,16 @@ interface SelectionLayerProps { read_only: boolean; activeSpanLabel: AnnotationLabelType | null; createAnnotation: (annotation: ServerTokenAnnotation) => void; + /** + * Optional handler that creates an OC_URL link annotation. When provided + * the action menu renders an "Add link…" item; otherwise the link UI is + * hidden. Lifted as a prop (rather than calling the hook directly) so + * tests can render SelectionLayer without an Apollo provider. + */ + createUrlAnnotation?: ( + annotation: ServerTokenAnnotation, + url: string + ) => Promise; pageNumber: number; } @@ -51,6 +66,7 @@ const SelectionLayer = ({ read_only, activeSpanLabel, createAnnotation, + createUrlAnnotation, pageNumber, }: SelectionLayerProps) => { const location = useLocation(); @@ -66,6 +82,10 @@ const SelectionLayer = ({ const { setSelectedAnnotations } = useAnnotationSelection(); const [, setIsCreatingAnnotation] = useAtom(isCreatingAnnotationAtom); + const [urlModalOpen, setUrlModalOpen] = useState(false); + const [urlPendingSelections, setUrlPendingSelections] = useState<{ + [key: number]: BoundingBox[]; + } | null>(null); const [localPageSelection, setLocalPageSelection] = useState< { pageNumber: number; bounds: BoundingBox } | undefined >(); @@ -254,6 +274,90 @@ const SelectionLayer = ({ setPendingSelections({}); }, [activeSpanLabel, pendingSelections, handleCreateMultiPageAnnotation]); + /** + * Opens the URL prompt modal, capturing the current pending selection so + * the user can finish typing a URL without losing their bounds while the + * action menu closes. + */ + const handleStartCreateLink = useCallback(() => { + setUrlPendingSelections(pendingSelections); + lastMenuInteractionTime.current = Date.now(); + setShowActionMenu(false); + setUrlModalOpen(true); + }, [pendingSelections]); + + /** + * Builds a ServerTokenAnnotation from the captured selections and calls + * ``addUrlAnnotation``. Mirrors ``handleCreateMultiPageAnnotation`` for + * the non-URL flow. + */ + const handleConfirmCreateLink = useCallback( + async (url: string) => { + const selections = urlPendingSelections; + if ( + !createUrlAnnotation || + !selections || + Object.keys(selections).length === 0 + ) { + setUrlModalOpen(false); + return; + } + + const pages = Object.keys(selections).map(Number); + const annotations: Record = {}; + let combinedRawText = ""; + for (const pageNum of pages) { + const pageAnnotation = pageInfo.getPageAnnotationJson( + selections[pageNum] + ); + if (pageAnnotation) { + annotations[pageNum] = pageAnnotation; + combinedRawText += " " + pageAnnotation.rawText; + } + } + + // ``createUrlAnnotation`` only consumes ``page``, ``json``, ``rawText`` + // and the label-type marker, so a synthetic OC_URL label placeholder + // is sufficient — the backend resolves/creates the real label. + // Cast is required because ``AnnotationLabelType`` declares optional + // ``icon``/``description`` fields that we intentionally omit on this + // transient client-side placeholder (never persisted as-is). + const placeholder: AnnotationLabelType = { + id: PENDING_OC_URL_LABEL_ID, + text: OC_URL_LABEL, + color: OC_URL_LABEL_COLOR, + labelType: LabelType.TokenLabel, + } as AnnotationLabelType; + + const annotation = new ServerTokenAnnotation( + pages[0], + placeholder, + combinedRawText.trim(), + false, + annotations, + [], + false, + false, + false + ); + + await createUrlAnnotation(annotation, url); + + setUrlModalOpen(false); + setUrlPendingSelections(null); + setMultiSelections({}); + setPendingSelections({}); + }, + [urlPendingSelections, pageInfo, createUrlAnnotation] + ); + + const handleCancelCreateLink = useCallback(() => { + setUrlModalOpen(false); + setUrlPendingSelections(null); + setMultiSelections({}); + setPendingSelections({}); + }, []); + /** * Handles canceling the selection without any action. */ @@ -891,6 +995,22 @@ const SelectionLayer = ({ {!read_only && canUpdateCorpus && ( <> + {createUrlAnnotation && ( + { + e.stopPropagation(); + handleStartCreateLink(); + }} + onTouchStart={(e) => { + e.stopPropagation(); + lastMenuInteractionTime.current = Date.now(); + }} + data-testid="create-link-button" + > + + Add link… + + )} {activeSpanLabel ? ( { @@ -978,6 +1098,29 @@ const SelectionLayer = ({ , document.body )} + + {urlModalOpen && + createPortal( + + pageInfo.getPageAnnotationJson(urlPendingSelections[p]) + ?.rawText ?? "" + ) + .join(" ") + .trim() + : "" + } + onCancel={handleCancelCreateLink} + onConfirm={handleConfirmCreateLink} + />, + document.body + )} ); }; diff --git a/frontend/src/components/annotator/renderers/pdf/__tests__/SelectionLayer.urlFlow.test.tsx b/frontend/src/components/annotator/renderers/pdf/__tests__/SelectionLayer.urlFlow.test.tsx new file mode 100644 index 000000000..d8a932352 --- /dev/null +++ b/frontend/src/components/annotator/renderers/pdf/__tests__/SelectionLayer.urlFlow.test.tsx @@ -0,0 +1,247 @@ +/** + * Unit-level coverage for SelectionLayer's URL-annotation flow. + * + * Exercises ``handleStartCreateLink`` → ``CreateUrlAnnotationModal`` → + * ``handleConfirmCreateLink`` / ``handleCancelCreateLink`` so the + * URL creation path is locked in without needing a full Playwright + * component test. These callbacks are entirely unhit by the existing + * jsdom test suite (which only drives the basic selection lifecycle), + * so codecov flags them as missed patches. + */ +import React from "react"; +import { render, fireEvent, act, screen } from "@testing-library/react"; +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { Provider as JotaiProvider } from "jotai"; +import { MemoryRouter } from "react-router-dom"; + +import SelectionLayer from "../SelectionLayer"; +import { PDFPageInfo } from "../../../types/pdf"; +import { + AnnotationLabelType, + LabelType, +} from "../../../../../types/graphql-api"; +import { PermissionTypes } from "../../../../types"; + +vi.mock("../../../context/CorpusAtom", () => ({ + useCorpusState: vi.fn(), +})); + +vi.mock("../../../hooks/useAnnotationSelection", () => ({ + useAnnotationSelection: () => ({ + setSelectedAnnotations: vi.fn(), + }), +})); + +// ``useAnnotationRefs`` returns a refs bundle expected to have a +// ``annotationElementRefs.current`` shape; SelectionBoundary (used by +// pending-selection bounds) accesses it during render. Stub it with a +// minimal shape so the SelectionLayer tree renders inside jsdom. +vi.mock("../../../hooks/useAnnotationRefs", () => ({ + useAnnotationRefs: () => ({ + annotationElementRefs: { current: {} }, + textSearchElementRefs: { current: {} }, + chatSourceElementRefs: { current: {} }, + PDFPageCanvasRef: { current: null }, + PDFPageRendererRef: { current: null }, + PDFPageContainerRefs: { current: {} }, + scrollContainerRef: { current: null }, + registerRef: vi.fn(), + unregisterRef: vi.fn(), + }), +})); + +// SelectionLayer reads ``isCreatingAnnotationAtom`` via ``useAtom`` and +// ``scrollContainerRefAtom`` via ``useAtomValue``. Mock both with safe +// defaults so the component renders without a Jotai store. +vi.mock("jotai", async () => { + const actual = await vi.importActual("jotai"); + return { + ...actual, + useAtom: vi.fn(() => [false, vi.fn()]), + useAtomValue: vi.fn(() => null), + }; +}); + +import { useCorpusState } from "../../../context/CorpusAtom"; + +const activeLabel: AnnotationLabelType = { + id: "label-1", + text: "Test Label", + color: "#0066cc", + description: "Test label", + labelType: LabelType.SpanLabel, + icon: "tag", + readonly: false, +}; + +/** + * Mock PDFPageInfo that yields a deterministic annotation payload — + * required because the URL-confirm handler iterates over the captured + * pending selections and asks for their PAWLs-format JSON. + */ +function buildPageInfo() { + return { + page: { pageNumber: 1 }, + getPageAnnotationJson: vi.fn(() => ({ + bounds: { left: 0, top: 0, right: 10, bottom: 10 }, + rawText: "linked text", + tokensJsons: [{ pageIndex: 0, tokenIndex: 0 }], + })), + getAnnotationForBounds: vi.fn(() => null), + } as unknown as PDFPageInfo; +} + +function mountLayer(opts: { + createUrlAnnotation?: ReturnType; + createAnnotation?: ReturnType; +}) { + const corpusMock = vi.mocked(useCorpusState); + corpusMock.mockReturnValue({ + canUpdateCorpus: true, + myPermissions: [PermissionTypes.CAN_UPDATE], + selectedCorpus: { id: "corpus-1" }, + humanSpanLabels: [activeLabel], + humanTokenLabels: [activeLabel], + relationLabels: [], + } as unknown as ReturnType); + + const utils = render( + + + + + + ); + + // Stub the canvas previousSibling so SelectionLayer's mousedown can + // read a bounding rect from inside jsdom. + const layer = utils.container.querySelector( + "#selection-layer" + ) as HTMLElement; + const fakeCanvas = document.createElement("canvas"); + Object.defineProperty(fakeCanvas, "getBoundingClientRect", { + value: () => ({ + left: 0, + top: 0, + right: 200, + bottom: 200, + width: 200, + height: 200, + x: 0, + y: 0, + toJSON: () => ({}), + }), + configurable: true, + }); + layer.parentElement?.insertBefore(fakeCanvas, layer); + + // Run a synthetic selection so ``pendingSelections`` is non-empty and + // the action menu (which hosts the "Add link…" button) is rendered. + // Wrap each lifecycle phase in its own act() so the doc-level listeners + // (attached only while ``localPageSelection`` is set, via a useEffect + // dependency) are bound before the mouseup arrives. + act(() => { + fireEvent.mouseDown(layer, { clientX: 5, clientY: 5, buttons: 1 }); + }); + act(() => { + fireEvent.mouseMove(document, { clientX: 100, clientY: 100, buttons: 1 }); + }); + act(() => { + fireEvent.mouseUp(document, { clientX: 100, clientY: 100 }); + }); + + return { ...utils, layer }; +} + +describe("SelectionLayer URL-annotation flow", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("clicking 'Add link…' opens the URL modal", async () => { + const createUrl = vi.fn(async () => undefined); + mountLayer({ createUrlAnnotation: createUrl }); + + // The action menu portals to document.body once the selection lands. + const linkBtn = await screen.findByTestId("create-link-button"); + expect(linkBtn).toBeInTheDocument(); + + act(() => { + fireEvent.click(linkBtn); + }); + + // After the click the menu collapses and the URL modal mounts. + // ``CreateUrlAnnotationModal`` renders an input the user can type into. + const urlInput = await screen.findByPlaceholderText(/https:\/\//); + expect(urlInput).toBeInTheDocument(); + }); + + it("confirming the URL modal invokes createUrlAnnotation with the typed URL", async () => { + const createUrl = vi.fn(async () => undefined); + mountLayer({ createUrlAnnotation: createUrl }); + + const linkBtn = await screen.findByTestId("create-link-button"); + act(() => { + fireEvent.click(linkBtn); + }); + + const urlInput = (await screen.findByPlaceholderText( + /https:\/\// + )) as HTMLInputElement; + + act(() => { + fireEvent.change(urlInput, { + target: { value: "https://example.com/from-test" }, + }); + }); + + // ``CreateUrlAnnotationModal`` renders the confirm button with text + // "Create link" when no initialUrl is supplied — find it by role/name. + const confirmBtn = await screen.findByRole("button", { + name: /create link/i, + }); + await act(async () => { + fireEvent.click(confirmBtn); + }); + + expect(createUrl).toHaveBeenCalledTimes(1); + const callArgs = createUrl.mock.calls[0] as unknown as [unknown, string]; + expect(callArgs[1]).toBe("https://example.com/from-test"); + }); + + it("cancelling the URL modal does NOT invoke createUrlAnnotation", async () => { + const createUrl = vi.fn(async () => undefined); + mountLayer({ createUrlAnnotation: createUrl }); + + const linkBtn = await screen.findByTestId("create-link-button"); + act(() => { + fireEvent.click(linkBtn); + }); + + await screen.findByPlaceholderText(/https:\/\//); + + // Modal's cancel button is rendered with "Cancel" label. + const cancelBtn = await screen.findByRole("button", { name: /^cancel$/i }); + act(() => { + fireEvent.click(cancelBtn); + }); + + expect(createUrl).not.toHaveBeenCalled(); + }); + + it("does NOT render the 'Add link…' button when createUrlAnnotation is missing", async () => { + // The button is gated on ``createUrlAnnotation && ...`` so omitting the + // prop must hide the entry point entirely. Without this gate, users + // could click into a no-op modal. + mountLayer({ createUrlAnnotation: undefined }); + + expect(screen.queryByTestId("create-link-button")).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/components/annotator/renderers/txt/TxtAnnotator.tsx b/frontend/src/components/annotator/renderers/txt/TxtAnnotator.tsx index cc3d60e43..a54bc1426 100644 --- a/frontend/src/components/annotator/renderers/txt/TxtAnnotator.tsx +++ b/frontend/src/components/annotator/renderers/txt/TxtAnnotator.tsx @@ -33,8 +33,18 @@ import { PermissionTypes, TextSearchSpanResult } from "../../../types"; import { Label, LabelContainer, PaperContainer } from "./StyledComponents"; import RadialButtonCloud, { CloudButtonItem } from "./RadialButtonCloud"; import { hexToRgba } from "./utils"; -import { useLocation } from "react-router-dom"; -import { Copy, Link, Tag, X, AlertCircle } from "lucide-react"; +import { useLocation, useNavigate } from "react-router-dom"; +import { + Copy, + ExternalLink, + Link, + Link2, + Tag, + X, + AlertCircle, +} from "lucide-react"; +import { isUrlAnnotation, openAnnotationUrl } from "../../utils/urlAnnotation"; +import { CreateUrlAnnotationModal } from "../../components/modals/CreateUrlAnnotationModal"; import { encodeTextBlock, textBlockFromSpan, @@ -48,7 +58,10 @@ import { HelpText, } from "../../components/SelectionActionMenu"; import { clampMenuPosition } from "../../../../utils/layout"; -import { Z_INDEX } from "../../../../assets/configurations/constants"; +import { + OC_URL_LABEL, + Z_INDEX, +} from "../../../../assets/configurations/constants"; import { OS_LEGAL_COLORS, chatSourceBlueAlpha, @@ -110,6 +123,16 @@ interface TxtAnnotatorProps { allowInput: boolean; /** Creates a new annotation in upstream data. */ createAnnotation: (added_annotation_obj: ServerSpanAnnotation) => void; + /** + * Optional handler that creates an OC_URL link annotation. When provided, + * an "Add link…" item appears in the selection menu; otherwise the link + * UI is hidden. Passed in (rather than via a hook) so the renderer + * stays decoupled from Apollo for unit tests. + */ + createUrlAnnotation?: ( + annotation: ServerSpanAnnotation, + url: string + ) => Promise; /** Updates an existing annotation in upstream data. */ updateAnnotation: (updated_annotation: ServerSpanAnnotation) => void; /** Approves an annotation if permitted. */ @@ -351,6 +374,7 @@ const TxtAnnotator: React.FC = ({ read_only, allowInput, createAnnotation, + createUrlAnnotation, updateAnnotation, approveAnnotation, rejectAnnotation, @@ -366,6 +390,7 @@ const TxtAnnotator: React.FC = ({ onAnnotationRefChange, }) => { const location = useLocation(); + const navigate = useNavigate(); const [hoveredSpanIndex, setHoveredSpanIndex] = useState(null); const [editModalOpen, setEditModalOpen] = useState(false); @@ -495,9 +520,22 @@ const TxtAnnotator: React.FC = ({ /** * Handle clicking on a label - toggle selected annotation or clear it. + * + * For OC_URL link annotations, a plain click opens the target URL instead + * of toggling selection. Holding Shift / Ctrl / Cmd falls back to the + * normal toggle behaviour so authors can still pick the link to edit it. */ const handleLabelClick = useCallback( - (annotation: ServerSpanAnnotation) => { + (annotation: ServerSpanAnnotation, event?: React.MouseEvent) => { + if ( + isUrlAnnotation(annotation) && + !event?.shiftKey && + !event?.metaKey && + !event?.ctrlKey + ) { + // Pass ``navigate`` so site-relative paths stay in the SPA. + if (openAnnotationUrl(annotation, navigate)) return; + } if (selectedAnnotations.includes(annotation.id)) { setSelectedAnnotations([]); } else { @@ -874,6 +912,56 @@ const TxtAnnotator: React.FC = ({ dismissMenu(); }, [pendingSelection, getSpan, createAnnotation, dismissMenu]); + // URL-link creation flow: capture the active selection, hide the action + // menu, and pop the URL prompt modal. On confirm we hand off to the + // injected ``createUrlAnnotation`` prop (wired to the corpus-scoped + // ``useCreateUrlAnnotation`` hook by the wrapper). + const [urlModalOpen, setUrlModalOpen] = useState(false); + const [urlPendingSelection, setUrlPendingSelection] = useState<{ + start: number; + end: number; + text: string; + } | null>(null); + + const handleTxtStartCreateLink = useCallback(() => { + if (pendingSelection) { + setUrlPendingSelection(pendingSelection); + dismissMenu(); + setUrlModalOpen(true); + } + }, [pendingSelection, dismissMenu]); + + const handleTxtConfirmCreateLink = useCallback( + async (url: string) => { + if (urlPendingSelection && createUrlAnnotation) { + const newAnnotation = getSpan(urlPendingSelection); + await createUrlAnnotation(newAnnotation, url); + } + setUrlModalOpen(false); + setUrlPendingSelection(null); + }, + [urlPendingSelection, getSpan, createUrlAnnotation] + ); + + const handleTxtCancelCreateLink = useCallback(() => { + setUrlModalOpen(false); + setUrlPendingSelection(null); + }, []); + + // Editing the URL on an existing OC_URL annotation. + const [urlEditAnnotation, setUrlEditAnnotation] = + useState(null); + + const handleTxtConfirmEditLink = useCallback( + (url: string) => { + if (urlEditAnnotation) { + updateAnnotation(urlEditAnnotation.update({ linkUrl: url })); + } + setUrlEditAnnotation(null); + }, + [urlEditAnnotation, updateAnnotation] + ); + // Handle clicks outside the action menu and keyboard shortcuts useEffect(() => { if (!showActionMenu) return; @@ -1024,6 +1112,20 @@ const TxtAnnotator: React.FC = ({ ? finalAnnotations[0].annotationLabel.color || "#cccccc" : undefined; + // First OC_URL annotation overlapping this span — used to make + // the span behave (and look) like a hyperlink. + const linkAnnotation = finalAnnotations.find(isUrlAnnotation); + + const handleSpanClick = linkAnnotation + ? (event: React.MouseEvent) => { + if (event.shiftKey || event.metaKey || event.ctrlKey) return; + // Pass ``navigate`` so site-relative paths stay in the SPA. + if (openAnnotationUrl(linkAnnotation, navigate)) { + event.stopPropagation(); + } + } + : undefined; + return ( = ({ } onMouseEnter={() => handleMouseEnter(index)} onMouseLeave={handleMouseLeave} + onClick={handleSpanClick} approved={approved} rejected={rejected} hasBorder={hasBorder} @@ -1045,6 +1148,9 @@ const TxtAnnotator: React.FC = ({ ...backgroundStyle, position: "relative", paddingLeft: isChatSource ? "4px" : undefined, + cursor: linkAnnotation ? "pointer" : undefined, + textDecoration: linkAnnotation ? "underline" : undefined, + textUnderlineOffset: linkAnnotation ? "2px" : undefined, }} > {isChatSource && ( @@ -1068,13 +1174,27 @@ const TxtAnnotator: React.FC = ({ !annotation.structural && annotation.myPermissions.includes(PermissionTypes.CAN_UPDATE) ) { + // Use the label text (NOT ``isUrlAnnotation``) so the + // URL editor opens even when ``link_url`` is null/empty. + // ``isUrlAnnotation`` requires both the OC_URL label AND a + // non-empty linkUrl — if an OC_URL annotation has no URL yet + // (e.g. created via the generic ``addAnnotation``), the + // author must still be able to attach one. + const isOcUrlAnnotation = + annotation.annotationLabel?.text === OC_URL_LABEL; actions.push({ name: "edit", color: "#a3a3a3", - tooltip: "Edit Annotation", + tooltip: isOcUrlAnnotation + ? "Edit Link URL" + : "Edit Annotation", onClick: () => { - setAnnotationToEdit(annotation); - setEditModalOpen(true); + if (isOcUrlAnnotation) { + setUrlEditAnnotation(annotation); + } else { + setAnnotationToEdit(annotation); + setEditModalOpen(true); + } }, }); } @@ -1139,9 +1259,20 @@ const TxtAnnotator: React.FC = ({ $index={labelIndex} onClick={(e: React.MouseEvent) => { e.stopPropagation(); - handleLabelClick(annotation); + handleLabelClick(annotation, e); + }} + style={{ + display: "inline-flex", + alignItems: "center", + gap: "4px", }} > + {isUrlAnnotation(annotation) && ( + + )} {annotation.annotationLabel.text} = ({ {allowInput && !read_only && ( <> + {createUrlAnnotation && ( + { + e.stopPropagation(); + handleTxtStartCreateLink(); + }} + data-testid="txt-create-link-button" + > + + Add link… + + )} { e.stopPropagation(); @@ -1245,6 +1388,29 @@ const TxtAnnotator: React.FC = ({ document.body )} + {urlModalOpen && + createPortal( + , + document.body + )} + + {urlEditAnnotation && + createPortal( + setUrlEditAnnotation(null)} + onConfirm={handleTxtConfirmEditLink} + />, + document.body + )} + + new ServerSpanAnnotation( + 0, + mockLabel, + "hello", + false, + { start: sel.start, end: sel.end }, + [PermissionTypes.CAN_READ, PermissionTypes.CAN_UPDATE], + false, + false, + false, + "local-tmp-id" + ) + ), + visibleLabels: null, + availableLabels: [mockLabel], + selectedLabelTypeId: mockLabel.id, + // allowInput=true + read_only=false unlocks the menu's annotation-creation + // entry points (Apply Label / Add link…), matching the live UX gate. + read_only: false, + allowInput: true, + zoom_level: 1, + createAnnotation: vi.fn(), + updateAnnotation: vi.fn(), + deleteAnnotation: vi.fn(), + selectedAnnotations: [] as string[], + setSelectedAnnotations: vi.fn(), + showStructuralAnnotations: true, + chatSources: EMPTY, +}; + +/** + * Stub ``document.getSelection`` so ``handleMouseUp`` reads a deterministic + * text range and pushes ``pendingSelection`` into state. The values mirror + * what jsdom would return after a real ``span.click``+drag, but without + * depending on jsdom's incomplete layout engine. + */ +function mockSelection(opts: { + text: string; + anchorNode: Node; + anchorOffset: number; + focusNode: Node; + focusOffset: number; +}) { + // ``TxtAnnotator.dismissMenu`` calls ``Selection.removeAllRanges`` after + // a menu interaction; jsdom's getSelection() shim implements it but the + // mocked instance does not by default. Provide the full surface area + // the component touches so the dismissal path doesn't throw. + const sel = { + toString: () => opts.text, + anchorNode: opts.anchorNode, + focusNode: opts.focusNode, + anchorOffset: opts.anchorOffset, + focusOffset: opts.focusOffset, + removeAllRanges: vi.fn(), + rangeCount: 1, + } as unknown as Selection; + vi.spyOn(document, "getSelection").mockReturnValue(sel); +} + +describe("TxtAnnotator URL-annotation flow", () => { + beforeEach(() => { + vi.clearAllMocks(); + // Reset menu cooldown by faking the timestamp far enough in the past. + vi.useFakeTimers({ now: Date.now() + 10_000 }); + vi.useRealTimers(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("'Add link…' button is only rendered when createUrlAnnotation is provided", async () => { + // Without ``createUrlAnnotation`` the menu must NOT advertise the + // link-creation entry point — clicking it would be a no-op confusion. + const { container, rerender } = render( + + + + ); + + const wrapper = container.querySelector("div[id]") as HTMLDivElement; + expect(wrapper).toBeTruthy(); + + // Stub selection covering "hello". + const textNode = wrapper.querySelector("span")?.firstChild ?? wrapper; + mockSelection({ + text: "hello", + anchorNode: textNode, + anchorOffset: 0, + focusNode: textNode, + focusOffset: 5, + }); + + act(() => { + fireEvent.mouseUp(wrapper, { clientX: 50, clientY: 50 }); + }); + + // The menu may not render the link button without createUrlAnnotation + expect(screen.queryByText(/add link/i)).not.toBeInTheDocument(); + + // Now re-render with the prop; the button must appear. + const createUrl = vi.fn(async () => undefined); + rerender( + + + + ); + + const wrapper2 = container.querySelector("div[id]") as HTMLDivElement; + const textNode2 = wrapper2.querySelector("span")?.firstChild ?? wrapper2; + mockSelection({ + text: "hello", + anchorNode: textNode2, + anchorOffset: 0, + focusNode: textNode2, + focusOffset: 5, + }); + + act(() => { + fireEvent.mouseUp(wrapper2, { clientX: 50, clientY: 50 }); + }); + + // "Add link…" should now appear in the menu. + expect(await screen.findByText(/add link/i)).toBeInTheDocument(); + }); + + it("opens the URL modal when 'Add link…' is clicked, then awaits createUrlAnnotation on confirm", async () => { + const createUrl = vi.fn(async () => undefined); + + const { container } = render( + + + + ); + + const wrapper = container.querySelector("div[id]") as HTMLDivElement; + const textNode = wrapper.querySelector("span")?.firstChild ?? wrapper; + mockSelection({ + text: "hello", + anchorNode: textNode, + anchorOffset: 0, + focusNode: textNode, + focusOffset: 5, + }); + + act(() => { + fireEvent.mouseUp(wrapper, { clientX: 50, clientY: 50 }); + }); + + const linkBtn = await screen.findByText(/add link/i); + act(() => { + fireEvent.click(linkBtn); + }); + + // CreateUrlAnnotationModal must mount with the placeholder input. + const urlInput = (await screen.findByPlaceholderText( + /https:\/\// + )) as HTMLInputElement; + act(() => { + fireEvent.change(urlInput, { + target: { value: "https://example.com/txt" }, + }); + }); + + const confirmBtn = await screen.findByRole("button", { + name: /create link/i, + }); + await act(async () => { + fireEvent.click(confirmBtn); + }); + + expect(createUrl).toHaveBeenCalledTimes(1); + const callArgs = createUrl.mock.calls[0] as unknown as [unknown, string]; + expect(callArgs[1]).toBe("https://example.com/txt"); + }); + + it("renders existing OC_URL annotations as hyperlinks (underline + pointer)", async () => { + // The renderer's hyperlink-styling block (cursor: pointer + underline) + // only fires when ``isUrlAnnotation`` matches — i.e. an annotation + // carries the OC_URL label AND a non-empty linkUrl. Without an OC_URL + // fixture, those style branches stay unhit. + const linkAnn = new ServerSpanAnnotation( + 0, + ocUrlLabel, + "hello", + false, + { start: 0, end: 5 }, + [PermissionTypes.CAN_READ, PermissionTypes.CAN_UPDATE], + false, + false, + false, + "ann-link-1", + undefined, + "https://example.com/anchor" + ); + + const { container } = render( + + + + ); + + // The annotated span (covering "hello") must be present with the + // hyperlink styling derived from ``isUrlAnnotation`` matching. + const annotatedSpan = container.querySelector( + '[data-testid^="annotated-span-"]' + ); + expect(annotatedSpan).toBeTruthy(); + const style = (annotatedSpan as HTMLElement).getAttribute("style") || ""; + // Pointer cursor + underline are the two visible hyperlink signals. + expect(style.toLowerCase()).toContain("cursor: pointer"); + expect(style.toLowerCase()).toContain("text-decoration: underline"); + }); + + it("clicking a hyperlink annotation opens the URL", async () => { + // Companion to the styling test above: a plain (no shift/meta/ctrl) + // click on a hyperlink span must route through ``openAnnotationUrl`` + // — which we observe via the mocked ``window.open`` for absolute URLs. + const openSpy = vi + .spyOn(window, "open") + .mockReturnValue(null as unknown as Window); + + const linkAnn = new ServerSpanAnnotation( + 0, + ocUrlLabel, + "hello", + false, + { start: 0, end: 5 }, + [PermissionTypes.CAN_READ], + false, + false, + false, + "ann-link-2", + undefined, + "https://example.com/click-target" + ); + + const { container } = render( + + + + ); + + const annotatedSpan = container.querySelector( + '[data-testid^="annotated-span-"]' + ) as HTMLElement; + expect(annotatedSpan).toBeTruthy(); + + act(() => { + fireEvent.click(annotatedSpan); + }); + + expect(openSpy).toHaveBeenCalledWith( + "https://example.com/click-target", + "_blank", + "noopener,noreferrer" + ); + openSpy.mockRestore(); + }); + + it("cancelling the URL modal does NOT invoke createUrlAnnotation", async () => { + const createUrl = vi.fn(async () => undefined); + + const { container } = render( + + + + ); + + const wrapper = container.querySelector("div[id]") as HTMLDivElement; + const textNode = wrapper.querySelector("span")?.firstChild ?? wrapper; + mockSelection({ + text: "hello", + anchorNode: textNode, + anchorOffset: 0, + focusNode: textNode, + focusOffset: 5, + }); + + act(() => { + fireEvent.mouseUp(wrapper, { clientX: 50, clientY: 50 }); + }); + + const linkBtn = await screen.findByText(/add link/i); + act(() => { + fireEvent.click(linkBtn); + }); + + await screen.findByPlaceholderText(/https:\/\//); + + const cancelBtn = await screen.findByRole("button", { name: /^cancel$/i }); + act(() => { + fireEvent.click(cancelBtn); + }); + + expect(createUrl).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/annotator/types/annotations.ts b/frontend/src/components/annotator/types/annotations.ts index e5a7b3640..424ce9a0c 100644 --- a/frontend/src/components/annotator/types/annotations.ts +++ b/frontend/src/components/annotator/types/annotations.ts @@ -84,7 +84,8 @@ export class ServerSpanAnnotation { public readonly rejected: boolean, public readonly canComment: boolean = false, id: string | undefined = undefined, - public readonly contentModalities?: string[] + public readonly contentModalities?: string[], + public readonly linkUrl?: string | null ) { this.id = id || uuidv4(); } @@ -105,7 +106,8 @@ export class ServerSpanAnnotation { delta.rejected ?? this.rejected, delta.canComment ?? this.canComment, this.id, - delta.contentModalities ?? this.contentModalities + delta.contentModalities ?? this.contentModalities, + delta.linkUrl ?? this.linkUrl ); } @@ -121,7 +123,8 @@ export class ServerSpanAnnotation { obj.rejected, obj.canComment, obj.id, - obj.contentModalities + obj.contentModalities, + obj.linkUrl ); } } @@ -144,7 +147,8 @@ export class ServerTokenAnnotation { public readonly rejected: boolean, public readonly canComment: boolean = false, id: string | undefined = undefined, - public readonly contentModalities?: string[] + public readonly contentModalities?: string[], + public readonly linkUrl?: string | null ) { this.id = id || uuidv4(); // Normalize any format (v1 or v2) to v1 for the rendering layer. @@ -180,7 +184,8 @@ export class ServerTokenAnnotation { delta.rejected ?? this.rejected, delta.canComment ?? this.canComment, this.id, - delta.contentModalities ?? this.contentModalities + delta.contentModalities ?? this.contentModalities, + delta.linkUrl ?? this.linkUrl ); } @@ -196,7 +201,8 @@ export class ServerTokenAnnotation { obj.rejected, obj.canComment, obj.id, - obj.contentModalities + obj.contentModalities, + obj.linkUrl ); } } diff --git a/frontend/src/components/annotator/utils/__tests__/urlAnnotation.test.ts b/frontend/src/components/annotator/utils/__tests__/urlAnnotation.test.ts new file mode 100644 index 000000000..c9e575e41 --- /dev/null +++ b/frontend/src/components/annotator/utils/__tests__/urlAnnotation.test.ts @@ -0,0 +1,283 @@ +/** + * Unit tests for ``urlAnnotation.ts``: + * + * - ``isUrlAnnotation`` returns true only when the annotation carries the + * ``OC_URL`` label AND a non-empty ``linkUrl``. + * - ``openAnnotationUrl`` opens http(s) URLs via ``window.open`` with + * noopener/noreferrer, navigates site-relative paths in the current tab, + * and refuses dangerous schemes (``javascript:``, ``data:``) even when + * the model layer would normally have stripped them. + * + * These tests pin the click-time defence: the renderer must never invoke + * ``window.open`` with attacker-controlled schemes, even if a stale cached + * annotation slipped through the model-level allow-list. + */ +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; + +import { OC_URL_LABEL } from "../../../../assets/configurations/constants"; +import { LabelType } from "../../types/enums"; +import { PermissionTypes } from "../../../types"; +import { + ServerSpanAnnotation, + ServerTokenAnnotation, +} from "../../types/annotations"; +import { + isSafeUrl, + isUrlAnnotation, + openAnnotationUrl, +} from "../urlAnnotation"; +import type { AnnotationLabelType } from "../../../../types/graphql-api"; + +// SemanticICONS unions are unwieldy in tests; cast via ``unknown`` once +// at the constant boundary so the rest of the file stays well-typed. +const ocUrlLabel: AnnotationLabelType = { + id: "label-url", + text: OC_URL_LABEL, + color: "#2563EB", + description: "url", + labelType: LabelType.SpanLabel, + icon: "link" as unknown as AnnotationLabelType["icon"], +}; + +const otherLabel: AnnotationLabelType = { + id: "label-other", + text: "Other", + color: "#333333", + description: "", + labelType: LabelType.SpanLabel, + icon: "tag" as unknown as AnnotationLabelType["icon"], +}; + +function makeSpan( + label: AnnotationLabelType, + linkUrl: string | null | undefined +): ServerSpanAnnotation { + return new ServerSpanAnnotation( + 0, + label, + "hello", + false, + { start: 0, end: 5 }, + [PermissionTypes.CAN_READ], + false, + false, + false, + "ann-1", + undefined, + linkUrl + ); +} + +function makeToken( + label: AnnotationLabelType, + linkUrl: string | null | undefined +): ServerTokenAnnotation { + return new ServerTokenAnnotation( + 0, + label, + "hello", + false, + { + 0: { bounds: {}, rawText: "hello", tokensJsons: [] }, + } as unknown as Record, + [PermissionTypes.CAN_READ], + false, + false, + false, + "ann-1", + undefined, + linkUrl + ); +} + +describe("isUrlAnnotation", () => { + it("returns true when label is OC_URL and linkUrl is non-empty", () => { + expect(isUrlAnnotation(makeSpan(ocUrlLabel, "https://example.com"))).toBe( + true + ); + expect(isUrlAnnotation(makeToken(ocUrlLabel, "https://example.com"))).toBe( + true + ); + }); + + it("returns false when label is OC_URL but linkUrl is missing", () => { + // Common while the author is editing — the annotation is not yet + // clickable so click handlers must keep selection behaviour. + expect(isUrlAnnotation(makeSpan(ocUrlLabel, null))).toBe(false); + expect(isUrlAnnotation(makeSpan(ocUrlLabel, undefined))).toBe(false); + expect(isUrlAnnotation(makeSpan(ocUrlLabel, ""))).toBe(false); + expect(isUrlAnnotation(makeSpan(ocUrlLabel, " "))).toBe(false); + }); + + it("returns false when linkUrl is present but label is not OC_URL", () => { + // Defence in depth: the existence of a linkUrl alone does NOT make an + // annotation clickable; the label must opt-in. + expect(isUrlAnnotation(makeSpan(otherLabel, "https://example.com"))).toBe( + false + ); + }); +}); + +describe("openAnnotationUrl", () => { + let originalOpen: typeof window.open; + let originalLocation: Location; + let openSpy: ReturnType; + let assignSpy: ReturnType; + + beforeEach(() => { + originalOpen = window.open; + openSpy = vi.fn(); + window.open = openSpy as unknown as typeof window.open; + + // jsdom's ``window.location`` is non-configurable per-property. Replace + // the whole object via Object.defineProperty on window — that descriptor + // IS configurable — so we can inject a recording stub for ``assign``. + originalLocation = window.location; + assignSpy = vi.fn(); + Object.defineProperty(window, "location", { + configurable: true, + writable: true, + value: { + ...originalLocation, + assign: assignSpy, + }, + }); + }); + + afterEach(() => { + window.open = originalOpen; + Object.defineProperty(window, "location", { + configurable: true, + writable: true, + value: originalLocation, + }); + }); + + it("opens https URLs in a new tab with noopener,noreferrer", () => { + const ok = openAnnotationUrl(makeSpan(ocUrlLabel, "https://example.com")); + expect(ok).toBe(true); + expect(openSpy).toHaveBeenCalledWith( + "https://example.com", + "_blank", + "noopener,noreferrer" + ); + expect(assignSpy).not.toHaveBeenCalled(); + }); + + it("opens http URLs in a new tab with noopener,noreferrer", () => { + const ok = openAnnotationUrl(makeSpan(ocUrlLabel, "http://example.com")); + expect(ok).toBe(true); + expect(openSpy).toHaveBeenCalledTimes(1); + }); + + it("navigates site-relative paths via window.location.assign as fallback", () => { + // When no ``navigate`` callback is supplied, the helper falls back to + // a hard navigation. This keeps the API safe for non-router contexts. + const ok = openAnnotationUrl(makeSpan(ocUrlLabel, "/corpus/foo")); + expect(ok).toBe(true); + expect(assignSpy).toHaveBeenCalledWith("/corpus/foo"); + expect(openSpy).not.toHaveBeenCalled(); + }); + + it("uses the navigate callback when supplied for site-relative paths", () => { + // Preferred path: pass a ``useNavigate()`` callback from react-router-dom + // so the SPA router resolves the URL in place, preserving Apollo cache + // and component state. The hard ``window.location.assign`` fallback + // must NOT fire. + const navigateSpy = vi.fn(); + const ok = openAnnotationUrl( + makeSpan(ocUrlLabel, "/corpus/foo"), + navigateSpy + ); + expect(ok).toBe(true); + expect(navigateSpy).toHaveBeenCalledWith("/corpus/foo"); + expect(assignSpy).not.toHaveBeenCalled(); + expect(openSpy).not.toHaveBeenCalled(); + }); + + it("refuses to open javascript: URLs", () => { + // The model layer would already strip these, but the renderer is the + // last line of defence — never reflect attacker-controlled schemes. + const ok = openAnnotationUrl(makeSpan(ocUrlLabel, "javascript:alert(1)")); + expect(ok).toBe(false); + expect(openSpy).not.toHaveBeenCalled(); + expect(assignSpy).not.toHaveBeenCalled(); + }); + + it("refuses to open data: URLs", () => { + const ok = openAnnotationUrl( + makeSpan(ocUrlLabel, "data:text/html,") + ); + expect(ok).toBe(false); + expect(openSpy).not.toHaveBeenCalled(); + }); + + it("refuses to open protocol-relative URLs (open-redirect guard)", () => { + // ``//evil.com`` starts with ``/`` but the browser would resolve it + // as ``https://evil.com`` — the site-relative branch must reject it + // so this open-redirect vector closes here at the renderer layer. + const ok = openAnnotationUrl(makeSpan(ocUrlLabel, "//evil.com")); + expect(ok).toBe(false); + expect(openSpy).not.toHaveBeenCalled(); + expect(assignSpy).not.toHaveBeenCalled(); + }); + + it("refuses to open empty/missing URLs", () => { + expect(openAnnotationUrl(makeSpan(ocUrlLabel, ""))).toBe(false); + expect(openAnnotationUrl(makeSpan(ocUrlLabel, undefined))).toBe(false); + expect(openAnnotationUrl(makeSpan(ocUrlLabel, null))).toBe(false); + expect(openSpy).not.toHaveBeenCalled(); + }); + + it("trims whitespace from valid URLs before opening", () => { + const ok = openAnnotationUrl( + makeSpan(ocUrlLabel, " https://example.com ") + ); + expect(ok).toBe(true); + expect(openSpy).toHaveBeenCalledWith( + "https://example.com", + "_blank", + "noopener,noreferrer" + ); + }); +}); + +describe("isSafeUrl", () => { + // Direct coverage of the exported helper used by authoring UIs + // (``CreateUrlAnnotationModal`` shares it via import) so the + // empty-string branch is exercised independently of + // ``openAnnotationUrl`` (which short-circuits earlier on ``!url``). + it("returns false for an empty string", () => { + expect(isSafeUrl("")).toBe(false); + }); + + it("returns false for a whitespace-only string", () => { + // ``isSafeUrl`` trims internally and then checks the *normalised* + // length, so leading/trailing whitespace must collapse to false. + expect(isSafeUrl(" ")).toBe(false); + expect(isSafeUrl("\t\n ")).toBe(false); + }); + + it("returns true for absolute http(s) URLs", () => { + expect(isSafeUrl("http://example.com")).toBe(true); + expect(isSafeUrl("https://example.com/path")).toBe(true); + // Case-insensitive scheme. + expect(isSafeUrl("HTTPS://EXAMPLE.COM")).toBe(true); + }); + + it("returns true for site-relative paths", () => { + expect(isSafeUrl("/corpus/foo")).toBe(true); + }); + + it("rejects protocol-relative URLs (open-redirect guard)", () => { + expect(isSafeUrl("//evil.com")).toBe(false); + expect(isSafeUrl("//evil.com/path?x=1")).toBe(false); + }); + + it("rejects dangerous schemes", () => { + expect(isSafeUrl("javascript:alert(1)")).toBe(false); + expect(isSafeUrl("data:text/html,") + + def test_file_scheme_is_rejected(self): + # file:// references would let an attacker probe local resources. + with self.assertRaises(ValidationError): + validate_link_url("file:///etc/passwd") + + def test_ftp_scheme_is_rejected(self): + # Only http(s) + site-relative are in the allow-list — ftp is out. + with self.assertRaises(ValidationError): + validate_link_url("ftp://example.com/file") + + def test_case_insensitive_scheme(self): + # Schemes are compared lowercased, so casing must not bypass the check. + validate_link_url("HTTPS://example.com") + with self.assertRaises(ValidationError): + validate_link_url("JavaScript:alert(1)") + + def test_whitespace_prefix_does_not_bypass(self): + # ``" javascript:..."`` could trick a naive startswith check if we + # did not strip; the regex must still reject after normalisation. + with self.assertRaises(ValidationError): + validate_link_url(" javascript:alert(1)") + + def test_protocol_relative_url_is_rejected(self): + # ``//evil.com`` starts with ``/`` but browsers resolve it as + # ``https://evil.com``. The site-relative branch of the allow-list + # must not let it through — otherwise we ship an open redirect. + with self.assertRaises(ValidationError): + validate_link_url("//evil.com") + with self.assertRaises(ValidationError): + validate_link_url("//evil.com/path?x=1") + # Whitespace-prefixed protocol-relative also rejected (post-strip + # the leading ``//`` is preserved, so the rejection still fires). + with self.assertRaises(ValidationError): + validate_link_url(" //evil.com") + + def test_annotation_clean_rejects_unsafe_link_url(self): + # The model's ``clean()`` must invoke ``validate_link_url`` so + # callers that go through full_clean() are protected. + user = User.objects.create_user(username="u1", password="x") + doc = Document.objects.create( + title="doc", creator=user, is_public=False, backend_lock=False + ) + label = AnnotationLabel.objects.create( + text="L", label_type=TOKEN_LABEL, creator=user + ) + ann = Annotation( + page=0, + raw_text="hello", + document=doc, + annotation_label=label, + creator=user, + annotation_type=TOKEN_LABEL, + link_url="javascript:alert(1)", + json={"0": {"bounds": {}, "rawText": "hello", "tokensJsons": []}}, + ) + with self.assertRaises(ValidationError): + ann.clean() + + def test_annotation_save_rejects_unsafe_link_url(self): + # The override on ``save()`` runs even when the JSON-validation flag + # is disabled — this is the last line of defence before persistence. + user = User.objects.create_user(username="u2", password="x") + doc = Document.objects.create( + title="doc", creator=user, is_public=False, backend_lock=False + ) + label = AnnotationLabel.objects.create( + text="L", label_type=TOKEN_LABEL, creator=user + ) + ann = Annotation( + page=0, + raw_text="hello", + document=doc, + annotation_label=label, + creator=user, + annotation_type=TOKEN_LABEL, + link_url="javascript:alert(1)", + json={"0": {"bounds": {}, "rawText": "hello", "tokensJsons": []}}, + ) + with self.assertRaises(ValidationError): + ann.save() + + def test_whitespace_only_link_url_collapses_to_none(self): + # ``" "`` is truthy as a string. Without explicit normalisation + # ``save()`` would persist the whitespace verbatim, leaving the + # column with garbage. Both ``clean()`` and ``save()`` collapse + # whitespace-only to None so the column stays NULL. + user = User.objects.create_user(username="u3", password="x") + doc = Document.objects.create( + title="doc", creator=user, is_public=False, backend_lock=False + ) + label = AnnotationLabel.objects.create( + text="L", label_type=TOKEN_LABEL, creator=user + ) + ann = Annotation( + page=0, + raw_text="hello", + document=doc, + annotation_label=label, + creator=user, + annotation_type=TOKEN_LABEL, + link_url=" ", + json={"0": {"bounds": {}, "rawText": "hello", "tokensJsons": []}}, + ) + ann.save() + ann.refresh_from_db() + self.assertIsNone(ann.link_url) + + def test_save_validates_link_url_when_json_validation_disabled(self): + # ``save()`` has an ``elif`` branch that runs when + # ``VALIDATE_ANNOTATION_JSON`` is False: ``clean()`` was skipped + # so the override must validate ``link_url`` itself, otherwise an + # unsafe scheme would slip past the last line of defence whenever + # JSON validation is disabled in production. + from django.test import override_settings + + user = User.objects.create_user(username="u4", password="x") + doc = Document.objects.create( + title="doc", creator=user, is_public=False, backend_lock=False + ) + label = AnnotationLabel.objects.create( + text="L", label_type=TOKEN_LABEL, creator=user + ) + ann = Annotation( + page=0, + raw_text="hello", + document=doc, + annotation_label=label, + creator=user, + annotation_type=TOKEN_LABEL, + link_url="javascript:alert(1)", + json={"0": {"bounds": {}, "rawText": "hello", "tokensJsons": []}}, + ) + # DEBUG=False + VALIDATE_ANNOTATION_JSON not set -> skips clean(), + # forcing the save-side validation branch to fire. + with override_settings(DEBUG=False, VALIDATE_ANNOTATION_JSON=False): + with self.assertRaises(ValidationError): + ann.save() + + def test_save_normalises_link_url_whitespace_when_json_validation_disabled(self): + # Companion to the test above: the save-side ``elif`` branch must + # also strip whitespace and collapse whitespace-only to ``None`` + # so the column stays canonical regardless of which path + # persisted it. + from django.test import override_settings + + user = User.objects.create_user(username="u5", password="x") + doc = Document.objects.create( + title="doc", creator=user, is_public=False, backend_lock=False + ) + label = AnnotationLabel.objects.create( + text="L", label_type=TOKEN_LABEL, creator=user + ) + ann = Annotation( + page=0, + raw_text="hello", + document=doc, + annotation_label=label, + creator=user, + annotation_type=TOKEN_LABEL, + link_url=" ", + json={"0": {"bounds": {}, "rawText": "hello", "tokensJsons": []}}, + ) + with override_settings(DEBUG=False, VALIDATE_ANNOTATION_JSON=False): + ann.save() + ann.refresh_from_db() + self.assertIsNone(ann.link_url) + + +class AddUrlAnnotationMutationTests(TestCase): + """Coverage of the ``addUrlAnnotation`` GraphQL mutation.""" + + def setUp(self): + self.owner = User.objects.create_user(username="owner", password="x") + self.outsider = User.objects.create_user(username="outsider", password="x") + + original_doc = Document.objects.create( + title="Owner Doc", + creator=self.owner, + is_public=False, + backend_lock=False, + ) + self.corpus = Corpus.objects.create( + title="Owner Corpus", creator=self.owner, is_public=False + ) + # add_document returns the corpus-scoped copy that the frontend + # actually annotates against. + self.document, _, _ = self.corpus.add_document( + document=original_doc, user=self.owner + ) + + set_permissions_for_obj_to_user( + self.owner, self.document, [PermissionTypes.CRUD] + ) + set_permissions_for_obj_to_user(self.owner, self.corpus, [PermissionTypes.CRUD]) + + self.client = Client(schema) + + def _execute(self, *, user, link_url, raw_text="link text"): + return self.client.execute( + ADD_URL_ANNOTATION_MUTATION, + variables={ + "corpusId": to_global_id("CorpusType", self.corpus.pk), + "documentId": to_global_id("DocumentType", self.document.pk), + "page": 0, + "rawText": raw_text, + "json": { + "0": { + "bounds": {}, + "rawText": raw_text, + "tokensJsons": [], + } + }, + "annotationType": "TOKEN_LABEL", + "linkUrl": link_url, + }, + context_value=_MutationContext(user), + ) + + def test_owner_creates_url_annotation_and_label(self): + # Happy path: owner creates a URL annotation. The OC_URL label is + # created on first use and the resulting annotation carries the + # supplied link_url. + before_labels = AnnotationLabel.objects.filter(text=OC_URL_LABEL).count() + result = self._execute(user=self.owner, link_url="https://example.com/a") + self.assertNotIn("errors", result, msg=result.get("errors")) + + payload = result["data"]["addUrlAnnotation"] + self.assertTrue(payload["ok"], msg=payload.get("message")) + self.assertIsNotNone(payload["annotation"]) + self.assertEqual(payload["annotation"]["linkUrl"], "https://example.com/a") + self.assertEqual(payload["annotation"]["annotationLabel"]["text"], OC_URL_LABEL) + + # The OC_URL label exists exactly once — the mutation is idempotent + # at the label level so repeated calls reuse the same label row. + self.assertEqual( + AnnotationLabel.objects.filter(text=OC_URL_LABEL).count(), + before_labels + 1, + ) + + def test_second_url_annotation_reuses_oc_url_label(self): + # Idempotency: creating a second URL annotation must NOT create a + # second OC_URL label — ensure_label_and_labelset is idempotent. + self._execute(user=self.owner, link_url="https://example.com/a") + self._execute(user=self.owner, link_url="https://example.com/b") + self.assertEqual(AnnotationLabel.objects.filter(text=OC_URL_LABEL).count(), 1) + + def test_rejects_javascript_scheme(self): + # Defence in depth: the GraphQL layer must refuse unsafe schemes + # before persistence (the model layer is the last line of defence). + before = Annotation.objects.count() + result = self._execute(user=self.owner, link_url="javascript:alert(1)") + payload = result["data"]["addUrlAnnotation"] + self.assertFalse(payload["ok"]) + self.assertIsNone(payload["annotation"]) + # No row written. + self.assertEqual(Annotation.objects.count(), before) + + def test_rejects_data_scheme(self): + result = self._execute( + user=self.owner, link_url="data:text/html," + ) + self.assertFalse(result["data"]["addUrlAnnotation"]["ok"]) + + def test_outsider_cannot_create_url_annotation(self): + # IDOR coverage: an authenticated user with no permissions on + # the parent corpus/document gets the uniform permission error + # and no annotation is written. + before = Annotation.objects.count() + result = self._execute(user=self.outsider, link_url="https://example.com") + payload = result["data"]["addUrlAnnotation"] + self.assertFalse(payload["ok"]) + self.assertIsNone(payload["annotation"]) + self.assertEqual(Annotation.objects.count(), before) + + def test_site_relative_url_accepted(self): + # Confirms the allow-list lets through internal SPA links. + result = self._execute(user=self.owner, link_url="/corpus/foo/doc/bar") + payload = result["data"]["addUrlAnnotation"] + self.assertTrue(payload["ok"]) + self.assertEqual(payload["annotation"]["linkUrl"], "/corpus/foo/doc/bar") + + +class AddAnnotationLinkUrlTests(TestCase): + """Coverage of the optional ``link_url`` argument on ``addAnnotation``.""" + + def setUp(self): + self.owner = User.objects.create_user(username="owner", password="x") + original_doc = Document.objects.create( + title="Owner Doc", + creator=self.owner, + is_public=False, + backend_lock=False, + ) + self.corpus = Corpus.objects.create( + title="Owner Corpus", creator=self.owner, is_public=False + ) + self.document, _, _ = self.corpus.add_document( + document=original_doc, user=self.owner + ) + self.label = AnnotationLabel.objects.create( + text="Custom", label_type=TOKEN_LABEL, creator=self.owner + ) + set_permissions_for_obj_to_user( + self.owner, self.document, [PermissionTypes.CRUD] + ) + set_permissions_for_obj_to_user(self.owner, self.corpus, [PermissionTypes.CRUD]) + self.client = Client(schema) + + def _execute(self, *, link_url, user=None): + return self.client.execute( + ADD_ANNOTATION_WITH_LINK_URL_MUTATION, + variables={ + "corpusId": to_global_id("CorpusType", self.corpus.pk), + "documentId": to_global_id("DocumentType", self.document.pk), + "annotationLabelId": to_global_id("AnnotationLabelType", self.label.pk), + "page": 0, + "rawText": "anchor", + "json": {"0": {"bounds": {}, "rawText": "anchor", "tokensJsons": []}}, + "annotationType": "TOKEN_LABEL", + "linkUrl": link_url, + }, + context_value=_MutationContext(user or self.owner), + ) + + def test_add_annotation_persists_link_url(self): + result = self._execute(link_url="https://example.com") + payload = result["data"]["addAnnotation"] + self.assertTrue(payload["ok"], msg=payload.get("message")) + self.assertEqual(payload["annotation"]["linkUrl"], "https://example.com") + + def test_add_annotation_rejects_unsafe_link_url(self): + # Validation happens BEFORE the parents are resolved; no DB write. + before = Annotation.objects.count() + result = self._execute(link_url="javascript:alert(1)") + payload = result["data"]["addAnnotation"] + self.assertFalse(payload["ok"]) + self.assertIsNone(payload["annotation"]) + self.assertEqual(Annotation.objects.count(), before) + + def test_add_annotation_without_link_url_is_ok(self): + # Backward compatibility: omitting link_url must still create an + # annotation with link_url=NULL. + result = self._execute(link_url=None) + payload = result["data"]["addAnnotation"] + self.assertTrue(payload["ok"], msg=payload.get("message")) + self.assertIsNone(payload["annotation"]["linkUrl"]) + + +class UpdateAnnotationLinkUrlTests(TestCase): + """Coverage of ``link_url`` handling in ``updateAnnotation``.""" + + def setUp(self): + self.owner = User.objects.create_user(username="owner", password="x") + original_doc = Document.objects.create( + title="Owner Doc", + creator=self.owner, + is_public=False, + backend_lock=False, + ) + self.corpus = Corpus.objects.create( + title="Owner Corpus", creator=self.owner, is_public=False + ) + self.document, _, _ = self.corpus.add_document( + document=original_doc, user=self.owner + ) + self.label = AnnotationLabel.objects.create( + text="Custom", label_type=TOKEN_LABEL, creator=self.owner + ) + set_permissions_for_obj_to_user( + self.owner, self.document, [PermissionTypes.CRUD] + ) + set_permissions_for_obj_to_user(self.owner, self.corpus, [PermissionTypes.CRUD]) + + self.annotation = Annotation.objects.create( + page=0, + raw_text="anchor", + document=self.document, + corpus=self.corpus, + annotation_label=self.label, + creator=self.owner, + annotation_type=TOKEN_LABEL, + link_url="https://example.com/old", + json={"0": {"bounds": {}, "rawText": "anchor", "tokensJsons": []}}, + ) + set_permissions_for_obj_to_user( + self.owner, self.annotation, [PermissionTypes.CRUD] + ) + + self.client = Client(schema) + + def _execute(self, *, link_url): + return self.client.execute( + UPDATE_ANNOTATION_MUTATION, + variables={ + "id": to_global_id("AnnotationType", self.annotation.pk), + "linkUrl": link_url, + }, + context_value=_MutationContext(self.owner), + ) + + def test_update_sets_new_link_url(self): + result = self._execute(link_url="https://example.com/new") + self.assertNotIn("errors", result, msg=result.get("errors")) + self.annotation.refresh_from_db() + self.assertEqual(self.annotation.link_url, "https://example.com/new") + + def test_update_with_empty_string_clears_link_url(self): + # The serializer normalises "" → None so the column ends up NULL. + result = self._execute(link_url="") + self.assertNotIn("errors", result, msg=result.get("errors")) + self.annotation.refresh_from_db() + self.assertIsNone(self.annotation.link_url) + + def test_update_rejects_unsafe_link_url(self): + # serializer.validate_link_url calls validate_link_url which raises + # ValidationError; the original value must remain. + before = self.annotation.link_url + result = self._execute(link_url="javascript:alert(1)") + + # The row must NOT have been updated regardless of how the rejection + # surfaced (DRFMutation ok=False vs GraphQL-level error). + self.annotation.refresh_from_db() + self.assertEqual(self.annotation.link_url, before) + + # And the mutation must NOT report ok=True. Default to True in the + # ``.get`` so a missing/absent ``updateAnnotation`` payload still + # fails the assertion — silent absence is not success. + payload = (result.get("data") or {}).get("updateAnnotation") or {} + self.assertFalse(payload.get("ok", True)) + + +class LinkUrlExporterTests(TestCase): + """Coverage of ``link_url`` passthrough in the ETL exporters. + + Two independent code paths emit ``link_url`` into the export schema: + * ``utils.etl.build_document_export`` (per-document V1 export, fork) + * ``utils.export_v2.package_structural_annotation_set`` (structural + sets in V2 export, indirectly via test_corpus_export_import_v2) + + Both branches are guarded by ``if annot.link_url:``. Without a test + that uses an annotation with ``link_url`` set, the *inside* of the + ``if`` block stays unhit and codecov flags those lines as missed + patches. + """ + + def setUp(self): + self.user = User.objects.create_user(username="exporter", password="x") + self.label = AnnotationLabel.objects.create( + text="Anchor", label_type=TOKEN_LABEL, creator=self.user + ) + self.corpus = Corpus.objects.create( + title="Exporter Corpus", creator=self.user, is_public=False + ) + self.document = Document.objects.create( + title="Exporter Doc", + creator=self.user, + is_public=False, + backend_lock=False, + page_count=1, + file_type="text/plain", + ) + + def test_build_document_export_emits_link_url(self): + """``etl.build_document_export`` must propagate link_url. + + The branch ``if annot.link_url: annot_export["link_url"] = ...`` is + the contract for V1 export / corpus fork. Without it, OC_URL + annotations silently lose their click targets on round-trip. + """ + from opencontractserver.types.enums import AnnotationFilterMode + from opencontractserver.utils.etl import ( + build_document_export, + build_label_lookups, + ) + + Annotation.objects.create( + page=0, + raw_text="click here", + document=self.document, + corpus=self.corpus, + annotation_label=self.label, + creator=self.user, + annotation_type=TOKEN_LABEL, + link_url="https://example.com/exported", + json={"0": {"bounds": {}, "rawText": "click here", "tokensJsons": []}}, + ) + + # Pre-build label lookups (mirrors what export_tasks does before + # invoking build_document_export per doc). + lookups = build_label_lookups( + corpus_id=self.corpus.id, + analysis_ids=None, + annotation_filter_mode=AnnotationFilterMode.CORPUS_LABELSET_PLUS_ANALYSES, + ) + # Ensure our text label is wired into lookups so the annotation + # passes the filter in build_document_export. build_label_lookups + # pulls labels via the corpus labelset + referenced labels, so add + # the label explicitly to the labelset for completeness. + from opencontractserver.annotations.models import LabelSet + + labelset = LabelSet.objects.create(title="LS", creator=self.user) + labelset.annotation_labels.add(self.label) + self.corpus.label_set = labelset + self.corpus.save() + lookups = build_label_lookups( + corpus_id=self.corpus.id, + analysis_ids=None, + annotation_filter_mode=AnnotationFilterMode.CORPUS_LABELSET_PLUS_ANALYSES, + ) + + ( + doc_name, + base64_file, + doc_export_data, + text_lbls, + doc_lbls, + ) = build_document_export( + label_lookups=lookups, + doc_id=self.document.id, + corpus_id=self.corpus.id, + analysis_ids=None, + annotation_filter_mode=AnnotationFilterMode.CORPUS_LABELSET_PLUS_ANALYSES, + ) + + # Find the annotation we wrote — it must carry the link_url in + # the export payload. + link_urls = [ + a.get("link_url") + for a in doc_export_data.get("labelled_text", []) + if a.get("link_url") + ] + self.assertIn("https://example.com/exported", link_urls) diff --git a/opencontractserver/tests/test_users_tasks_auth0.py b/opencontractserver/tests/test_users_tasks_auth0.py index b9cb8527d..816734940 100644 --- a/opencontractserver/tests/test_users_tasks_auth0.py +++ b/opencontractserver/tests/test_users_tasks_auth0.py @@ -102,3 +102,25 @@ def test_non_200_response_returns_none_without_persisting(self, mock_post): self.assertIsNone(token) self.assertEqual(Auth0APIToken.objects.count(), token_count_before) + + @patch("opencontractserver.users.tasks.requests.get") + def test_get_user_details_async_returns_parsed_response(self, mock_get): + # ``get_user_details_async`` is defined inside the same + # ``if settings.USE_AUTH0:`` block as ``get_new_auth0_token``, so + # it only exists with USE_AUTH0=True. Exercise the header + # construction + Auth0 GET so the patch-coverage line that + # assembles the bearer-token headers is hit. + mock_response = MagicMock() + mock_response.text = json.dumps({"user_id": "auth0|abc", "email": "x@y.z"}) + mock_get.return_value = mock_response + + details = self.users_tasks.get_user_details_async( + "bearer-token-xyz", "auth0|abc" + ) + + self.assertEqual(details["user_id"], "auth0|abc") + mock_get.assert_called_once() + # The Authorization header must use the supplied bearer token. + _, kwargs = mock_get.call_args + headers = kwargs.get("headers", {}) + self.assertEqual(headers.get("Authorization"), "Bearer bearer-token-xyz") diff --git a/opencontractserver/tests/test_web_search_tool.py b/opencontractserver/tests/test_web_search_tool.py index c0ceacde2..429a0713a 100644 --- a/opencontractserver/tests/test_web_search_tool.py +++ b/opencontractserver/tests/test_web_search_tool.py @@ -543,11 +543,26 @@ async def _run(): class TestPipelineSettingsToolSecrets(TestCase): """Test PipelineSettings tool secret helpers.""" + def setUp(self): + """Invalidate the singleton cache so each test sees fresh DB state. + + Without this, parallel xdist workers can hand back a cached instance + whose ``modified_by`` FK references a User row deleted by a sibling + test, causing ``SET CONSTRAINTS ALL IMMEDIATE`` to fail at teardown. + """ + from opencontractserver.documents.models import PipelineSettings + + PipelineSettings._invalidate_cache() + def tearDown(self): """Clean up tool settings to avoid leaking state to other tests.""" from opencontractserver.documents.models import PipelineSettings - ps = PipelineSettings.get_instance() + # Bypass cache to read live DB state; null out any stale modified_by + # FK so the post-test ``SET CONSTRAINTS`` check doesn't fail when a + # previously-deleted user remains referenced. + ps = PipelineSettings.get_instance(use_cache=False) + ps.modified_by = None ps.delete_tool_settings(WEB_SEARCH_SETTINGS_KEY) ps.save() diff --git a/opencontractserver/types/dicts.py b/opencontractserver/types/dicts.py index 3a8812ca3..243cc9ec8 100644 --- a/opencontractserver/types/dicts.py +++ b/opencontractserver/types/dicts.py @@ -241,6 +241,10 @@ class OpenContractsAnnotationPythonType(TypedDict): list[str] ] # ["TEXT"], ["IMAGE"], or ["TEXT", "IMAGE"] long_description: NotRequired[Optional[str]] + # Target URL for OC_URL clickable hyperlink annotations. Persisted on + # round-trip so a corpus full of link annotations doesn't silently lose + # all its targets through fork / V2-export / V2-import. + link_url: NotRequired[Optional[str]] class SpanAnnotation(TypedDict): diff --git a/opencontractserver/users/tasks.py b/opencontractserver/users/tasks.py index 113b7a9c8..b23221732 100644 --- a/opencontractserver/users/tasks.py +++ b/opencontractserver/users/tasks.py @@ -33,7 +33,7 @@ def get_new_auth0_token() -> Optional[str]: url = f"https://{auth0_settings.AUTH0_DOMAIN}/oauth/token" - headers: dict[str, str] = {"content-type": "application/json"} + headers: dict[str, str | bytes] = {"content-type": "application/json"} request_data: dict[str, Any] = { "grant_type": auth0_settings.AUTH0_M2M_MANAGEMENT_GRANT_TYPE, @@ -161,7 +161,7 @@ def ensure_valid_auth0_token() -> Optional[str]: @celery_app.task def get_user_details_async(token: str, auth0_Id: str) -> dict[str, Any]: - headers: dict[str, str] = { + headers: dict[str, str | bytes] = { "Authorization": f"Bearer {token}", } url = f"https://{auth0_settings.AUTH0_DOMAIN}/api/v2/users/{auth0_Id}" diff --git a/opencontractserver/utils/cloud.py b/opencontractserver/utils/cloud.py index 13b472fe7..93cbde9d1 100644 --- a/opencontractserver/utils/cloud.py +++ b/opencontractserver/utils/cloud.py @@ -7,8 +7,8 @@ def maybe_add_cloud_run_auth( - url: str, headers: dict[str, str], force: bool = False -) -> dict[str, str]: + url: str, headers: dict[str, str | bytes], force: bool = False +) -> dict[str, str | bytes]: """ Attach an Authorization bearer with a Google Cloud Run identity token when applicable. Args: diff --git a/opencontractserver/utils/etl.py b/opencontractserver/utils/etl.py index 964308d16..793bd03fb 100644 --- a/opencontractserver/utils/etl.py +++ b/opencontractserver/utils/etl.py @@ -412,6 +412,8 @@ def build_document_export( annot_export["content_modalities"] = annot.content_modalities if annot.long_description is not None: annot_export["long_description"] = annot.long_description + if annot.link_url: + annot_export["link_url"] = annot.link_url labelled_text.append(annot_export) # Span annotations ({start, end}) don't have page-keyed structure diff --git a/opencontractserver/utils/export_v2.py b/opencontractserver/utils/export_v2.py index 92b1479fc..95db431ac 100644 --- a/opencontractserver/utils/export_v2.py +++ b/opencontractserver/utils/export_v2.py @@ -107,6 +107,10 @@ def package_structural_annotation_set( } if annot.long_description is not None: annot_data["long_description"] = annot.long_description + # Carry the OC_URL ``link_url`` through round-trip so forked / + # re-imported corpora keep their clickable targets. + if annot.link_url: + annot_data["link_url"] = annot.link_url structural_annotations.append(annot_data) # Get structural relationships diff --git a/opencontractserver/utils/import_v2.py b/opencontractserver/utils/import_v2.py index a961499ab..6aa2039a4 100644 --- a/opencontractserver/utils/import_v2.py +++ b/opencontractserver/utils/import_v2.py @@ -140,6 +140,9 @@ def import_structural_annotation_set( page=annot_data.get("page", 0), json=annot_data.get("annotation_json", {}), annotation_type=annot_data.get("annotation_type", ""), + # Restore OC_URL ``link_url`` if it was exported. Falsy / + # missing values stay NULL on the column. + link_url=annot_data.get("link_url") or None, structural=True, creator=user_obj, ) diff --git a/opencontractserver/utils/importing.py b/opencontractserver/utils/importing.py index 7293d284d..069d28e7b 100644 --- a/opencontractserver/utils/importing.py +++ b/opencontractserver/utils/importing.py @@ -150,6 +150,10 @@ def import_annotations( annotation_type=final_annotation_type, structural=annotation_data.get("structural", False), content_modalities=annotation_data.get("content_modalities", []), + # OC_URL annotations carry a click-through ``link_url`` that + # must survive round-trip through bulk import. Falsy / + # missing values stay NULL on the column. + link_url=annotation_data.get("link_url") or None, ) ) parallel_old_ids.append(annotation_data.get("id"))