diff --git a/apps/obsidian/src/utils/__tests__/noteFileName.test.ts b/apps/obsidian/src/utils/__tests__/noteFileName.test.ts new file mode 100644 index 0000000000..9e7db80b5c --- /dev/null +++ b/apps/obsidian/src/utils/__tests__/noteFileName.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import { noteFileNameFromTitle } from "~/utils/noteFileName"; + +describe("noteFileNameFromTitle", () => { + it("keeps a plain title", () => { + expect(noteFileNameFromTitle("CLM - sleep improves memory")).toBe( + "CLM - sleep improves memory", + ); + }); + + it("unwraps Roam page references in the title", () => { + expect( + noteFileNameFromTitle( + "[[EVD]] - REM sleep aids recall - [[@Smith 2020]]", + ), + ).toBe("EVD - REM sleep aids recall - @Smith 2020"); + }); + + it("drops the characters Obsidian rejects in file names", () => { + expect(noteFileNameFromTitle('ac:d"e/f\\g|h?i*j#k^l[m]n')).toBe( + "abcdefghijklmn", + ); + }); + + it("drops a stray bracket that is not a page reference", () => { + expect(noteFileNameFromTitle("[[unclosed - [tag] - x")).toBe( + "unclosed - tag - x", + ); + }); + + it("drops a slash rather than making a folder", () => { + expect(noteFileNameFromTitle("Projects/Alpha")).toBe("ProjectsAlpha"); + }); + + it("collapses whitespace left behind", () => { + expect(noteFileNameFromTitle(" #tag - x ")).toBe("tag - x"); + }); + + it("returns an empty string when nothing survives", () => { + expect(noteFileNameFromTitle("[[]]")).toBe(""); + expect(noteFileNameFromTitle("#")).toBe(""); + }); + + it("unwraps nested and adjacent references", () => { + expect(noteFileNameFromTitle("[[a [[b]] c]]")).toBe("a b c"); + expect(noteFileNameFromTitle("[[EVD]][[x]]")).toBe("EVDx"); + }); + + it("does not treat a pipe inside a reference as an alias", () => { + expect(noteFileNameFromTitle("[[Page|alias]]")).toBe("Pagealias"); + }); + + it("passes non-ASCII titles through", () => { + expect( + noteFileNameFromTitle("[[EVD]] - Schlaf verbessert Gedächtnis"), + ).toBe("EVD - Schlaf verbessert Gedächtnis"); + }); +}); diff --git a/apps/obsidian/src/utils/importNodes.ts b/apps/obsidian/src/utils/importNodes.ts index 11225dd16e..c0d472f70c 100644 --- a/apps/obsidian/src/utils/importNodes.ts +++ b/apps/obsidian/src/utils/importNodes.ts @@ -37,6 +37,7 @@ import { } from "./importedNodeContent"; import { decorateTitle } from "@repo/database/lib/decorateTitle"; import { buildSchemaRid, findLocalNodeTypeMatch } from "./schemaMatching"; +import { noteFileNameFromTitle } from "./noteFileName"; type PublishedNode = { source_local_id: string; @@ -1725,7 +1726,9 @@ const importNodes = async ({ coreTitle !== undefined && localNodeType ? decorateTitle(localNodeType.format, coreTitle) : null; - const sanitizedFileName = sanitizeFileName(decoratedTitle ?? fileName); + const sanitizedFileName = + noteFileNameFromTitle(decoratedTitle ?? fileName) || + node.nodeInstanceId; let finalFilePath: string; if (existingFile) { diff --git a/apps/obsidian/src/utils/noteFileName.ts b/apps/obsidian/src/utils/noteFileName.ts new file mode 100644 index 0000000000..817db29395 --- /dev/null +++ b/apps/obsidian/src/utils/noteFileName.ts @@ -0,0 +1,17 @@ +// Titles arrive from other apps in their own syntax: Roam node titles carry page +// references ([[EVD]] - x - [[@Smith 2020]]) and tags. Obsidian creates a file with +// those characters in its name but cannot link to it, so references are unwrapped +// to keep the page name and the rest of the link characters (#^[]|, the set +// checkInvalidChars enforces on node type formats) are dropped with the OS set. +// A slash is dropped rather than made a folder: a Roam namespace and an Obsidian +// folder are not the same thing. Returns "" when nothing survives; the caller +// picks the fallback name. +const PAGE_REFERENCE = /\[\[([^\]]*)\]\]/g; +const REJECTED_IN_FILE_NAMES = /[<>:"/\\|?*#^[\]]/g; + +export const noteFileNameFromTitle = (title: string): string => + title + .replace(PAGE_REFERENCE, "$1") + .replace(REJECTED_IN_FILE_NAMES, "") + .replace(/\s+/g, " ") + .trim(); diff --git a/apps/roam/src/components/CreateRelationDialog.tsx b/apps/roam/src/components/CreateRelationDialog.tsx index c1d08b7012..fa4b335f0d 100644 --- a/apps/roam/src/components/CreateRelationDialog.tsx +++ b/apps/roam/src/components/CreateRelationDialog.tsx @@ -1,3 +1,4 @@ +import { useRelationSchemaRevision } from "~/utils/relationSchemaChanges"; import React, { useState, useMemo } from "react"; import { Dialog, Classes, Label, Button, Callout } from "@blueprintjs/core"; import renderOverlay from "roamjs-components/util/renderOverlay"; @@ -8,6 +9,7 @@ import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageU import getDiscourseRelations, { type DiscourseRelation, } from "~/utils/getDiscourseRelations"; +import { excludeProvisionalRelationSchemas } from "~/utils/relationSchemaAcceptance"; import { createReifiedRelation } from "~/utils/createReifiedBlock"; import { getStoredRelationsEnabled } from "~/utils/storedRelations"; import findDiscourseNode from "~/utils/findDiscourseNode"; @@ -291,7 +293,7 @@ const prepareRelData = ( ): RelWithDirection[] => { nodeTitle = nodeTitle || getPageTitleByPageUid(targetNodeUid).trim(); const discourseNodeSchemas = getDiscourseNodes(); - const relations = getDiscourseRelations(); + const relations = excludeProvisionalRelationSchemas(getDiscourseRelations()); const nodeSchema = findDiscourseNode({ uid: targetNodeUid, title: nodeTitle, @@ -387,6 +389,7 @@ export const renderCreateRelationDialog = ( export const CreateRelationButton = ( props: CreateRelationDialogProps & { fill?: boolean }, ): React.JSX.Element | null => { + useRelationSchemaRevision(); const { fill = false, ...relationProps } = props; const storedRelationsEnabled = getStoredRelationsEnabled(); if (!storedRelationsEnabled) return null; @@ -405,7 +408,8 @@ export const CreateRelationButton = ( text="Add relation" disabled={extProps === null} onClick={() => { - renderCreateRelationDialog(extProps); + // A schema may have been accepted since this button last rendered. + renderCreateRelationDialog(relationProps); }} /> ); diff --git a/apps/roam/src/components/DiscoverSharedNodesDialog.tsx b/apps/roam/src/components/DiscoverSharedNodesDialog.tsx index ba53226c46..5b73099d56 100644 --- a/apps/roam/src/components/DiscoverSharedNodesDialog.tsx +++ b/apps/roam/src/components/DiscoverSharedNodesDialog.tsx @@ -5,17 +5,21 @@ import { Classes, Dialog, HTMLTable, + Icon, InputGroup, Intent, NonIdealState, + ProgressBar, Spinner, - Tag, Tooltip, } from "@blueprintjs/core"; import React, { useCallback, useEffect, useMemo, useState } from "react"; +import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageUid"; import createOverlayRender from "roamjs-components/util/createOverlayRender"; +import openBlockInSidebar from "roamjs-components/writes/openBlockInSidebar"; import type { SharedNode } from "@repo/database/lib/sharedNodes"; import { discoverSharedNodes } from "~/utils/discoverSharedNodes"; +import { getErrorMessage } from "~/utils/getErrorMessage"; import { importSharedNodes, isFailedSharedNodeImport, @@ -23,6 +27,13 @@ import { } from "~/utils/importSharedNodes"; import { importSharedRelations } from "~/utils/importSharedRelations"; import internalError from "~/utils/internalError"; +import { + DEFAULT_SHARED_NODE_SORT, + getNextSharedNodeSort, + sortSharedNodes, + type SharedNodeSort, + type SharedNodeSortColumn, +} from "~/utils/sortSharedNodes"; import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext"; const IMPORT_ERROR_TYPE = "Shared node import failed"; @@ -33,13 +44,11 @@ const formatModifiedAt = (modifiedAt: string): string => const SharedNodeRow = ({ node, - alreadyImported, selected, selectionDisabled, onToggleSelected, }: { node: SharedNode; - alreadyImported: boolean; selected: boolean; selectionDisabled: boolean; onToggleSelected: () => void; @@ -54,9 +63,6 @@ const SharedNodeRow = ({ onChange={onToggleSelected} /> - - {node.platform} -
{node.spaceName} @@ -77,39 +83,72 @@ const SharedNodeRow = ({ {node.title}
- - {node.sourceLocalId ? ( -
- {node.sourceLocalId} -
- ) : ( - Not provided - )} - {formatModifiedAt(node.lastModified)} - - {alreadyImported ? ( - - Imported - - ) : ( - Available - )} - ); +const SortableHeader = ({ + label, + column, + sort, + onSort, +}: { + label: string; + column: SharedNodeSortColumn; + sort: SharedNodeSort; + onSort: (column: SharedNodeSortColumn) => void; +}): React.ReactElement => { + const isActive = sort.column === column; + return ( + + + + ); +}; + +const ImportedNodeLink = ({ + pageUid, + onOpenInMainWindow, +}: { + pageUid: string; + onOpenInMainWindow: () => void; +}): React.ReactElement => ( + { + if (event.shiftKey) { + void openBlockInSidebar(pageUid); + return; + } + void window.roamAlphaAPI.ui.mainWindow.openPage({ + page: { uid: pageUid }, + }); + onOpenInMainWindow(); + }} + > + {getPageTitleByPageUid(pageUid)} + +); + const ImportResultsSummary = ({ results, + onOpenInMainWindow, }: { results: SharedNodeImportItem[]; + onOpenInMainWindow: () => void; }) => { const importedCount = results.filter( (item) => item.status === "imported", @@ -118,30 +157,39 @@ const ImportResultsSummary = ({ (item) => item.status === "skipped", ).length; const failedImports = results.filter(isFailedSharedNodeImport); - const warnings = results.flatMap((item) => - item.status !== "failed" && item.warning - ? [{ sharedNode: item.sharedNode, message: item.warning }] - : [], + const completedImports = results.flatMap((item) => + item.status === "failed" ? [] : [item], ); - const importNotices = [...failedImports, ...warnings]; + const warningCount = completedImports.filter((item) => item.warning).length; return ( 0 ? Intent.WARNING : Intent.SUCCESS} - title={`${importedCount} imported, ${skippedCount} skipped, ${failedImports.length} failed${warnings.length > 0 ? `, ${warnings.length} with warnings` : ""}`} + intent={ + failedImports.length > 0 || warningCount > 0 + ? Intent.WARNING + : Intent.SUCCESS + } + title={`${importedCount} imported, ${skippedCount} skipped, ${failedImports.length} failed${warningCount > 0 ? `, ${warningCount} with warnings` : ""}`} > {skippedCount > 0 && (
Skipped nodes were already up to date in this graph.
)} - {importNotices.length > 0 && ( - - )} + ); }; @@ -152,6 +200,7 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { const [loading, setLoading] = useState(true); const [error, setError] = useState(""); const [searchTerm, setSearchTerm] = useState(""); + const [sort, setSort] = useState(DEFAULT_SHARED_NODE_SORT); const [selectedRids, setSelectedRids] = useState>(new Set()); const [spaceId, setSpaceId] = useState(0); const [importProgress, setImportProgress] = useState<{ @@ -201,19 +250,24 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { void loadNodes(); }, [loadNodes]); + const availableNodes = useMemo( + () => + sortSharedNodes({ + nodes: nodes.filter((node) => !importedRids.has(node.rid)), + sort, + }), + [importedRids, nodes, sort], + ); + const visibleNodes = useMemo(() => { const normalizedSearch = searchTerm.trim().toLocaleLowerCase(); - if (!normalizedSearch) return nodes; - return nodes.filter((node) => - [ - node.platform, - node.spaceName, - node.spaceUri, - node.title, - node.sourceLocalId, - ].some((value) => value.toLocaleLowerCase().includes(normalizedSearch)), + if (!normalizedSearch) return availableNodes; + return availableNodes.filter((node) => + [node.spaceName, node.spaceUri, node.title].some((value) => + value.toLocaleLowerCase().includes(normalizedSearch), + ), ); - }, [nodes, searchTerm]); + }, [availableNodes, searchTerm]); const visibleRids = visibleNodes.map((node) => node.rid); const allVisibleSelected = @@ -238,6 +292,10 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { }); }; + const handleSort = (column: SharedNodeSortColumn): void => { + setSort((currentSort) => getNextSharedNodeSort({ currentSort, column })); + }; + const importSelectedNodes = async (): Promise => { const selectedNodes = nodes.filter((node) => selectedRids.has(node.rid)); @@ -260,7 +318,6 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { newlyImportedRids.forEach((rid) => next.add(rid)); return next; }); - await importSharedRelations(client, spaceId, [...importedRids]); setImportResults(results); const failedImports = results.filter(isFailedSharedNodeImport); setSelectedRids( @@ -279,6 +336,16 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { sendEmail: false, }); } + await importSharedRelations(client, spaceId, [...importedRids]).catch( + (relationsError: unknown) => + internalError({ + error: relationsError, + type: IMPORT_ERROR_TYPE, + context: { operation: IMPORT_ERROR_OPERATION }, + sendEmail: false, + userMessage: `The nodes were imported, but their relations were not: ${getErrorMessage(relationsError)}`, + }), + ); } catch (importError) { internalError({ error: importError, @@ -302,15 +369,19 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { canOutsideClickClose={!importing} enforceFocus={false} isCloseButtonShown={!importing} - style={{ width: "min(68rem, calc(100vw - 2rem))" }} + style={{ + width: "min(68rem, calc(100vw - 2rem))", + height: "min(48rem, calc(100vh - 4rem))", + }} isOpen onClose={onClose} - title="Discover shared nodes" + title="Import shared nodes" >
void }) => {
- {importResults && } + {importProgress ? ( + + + + ) : ( + importResults && ( + + ) + )} {loading ? (
@@ -351,7 +438,9 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
@@ -370,12 +459,24 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { onChange={toggleAllVisibleSelected} /> - Source app - Source space - Title - Source ID - Modified - Status + + + @@ -383,7 +484,6 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { toggleNodeSelected(node.rid)} selected={selectedRids.has(node.rid)} selectionDisabled={importing} @@ -399,7 +499,7 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => { {loading || error ? "" - : `${visibleNodes.length} of ${nodes.length} nodes`} + : `${visibleNodes.length} of ${availableNodes.length} nodes`}
diff --git a/apps/roam/src/components/Export.tsx b/apps/roam/src/components/Export.tsx index daa5a1040d..65b5a31bff 100644 --- a/apps/roam/src/components/Export.tsx +++ b/apps/roam/src/components/Export.tsx @@ -927,7 +927,7 @@ const ExportDialog: ExportDialogComponent = ({ messages.push( `${assets.failed.length} file${ assets.failed.length === 1 ? "" : "s" - } could not be copied.`, + } could not be copied. Publish again to retry.`, ); renderToast({ content: messages.join(" "), diff --git a/apps/roam/src/components/SuggestionsBody.tsx b/apps/roam/src/components/SuggestionsBody.tsx index c81d4dc00d..ff055b7034 100644 --- a/apps/roam/src/components/SuggestionsBody.tsx +++ b/apps/roam/src/components/SuggestionsBody.tsx @@ -1,3 +1,4 @@ +import { useRelationSchemaRevision } from "~/utils/relationSchemaChanges"; import React, { useMemo, useState, useEffect, useCallback } from "react"; import { Button, @@ -21,6 +22,7 @@ import getDiscourseContextResults from "~/utils/getDiscourseContextResults"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import findDiscourseNode from "~/utils/findDiscourseNode"; import getDiscourseRelations from "~/utils/getDiscourseRelations"; +import { excludeProvisionalRelationSchemas } from "~/utils/relationSchemaAcceptance"; import getDiscourseNodes from "~/utils/getDiscourseNodes"; import normalizePageTitle from "roamjs-components/queries/normalizePageTitle"; import { type RelationDetails } from "~/utils/hyde"; @@ -232,7 +234,13 @@ const SuggestionsBody = ({ () => findDiscourseNode({ uid: tagUid }), [tagUid], ); - const allRelations = useMemo(() => getDiscourseRelations(), []); + const relationSchemaRevision = useRelationSchemaRevision(); + const allRelations = useMemo( + () => excludeProvisionalRelationSchemas(getDiscourseRelations()), + // Acceptance and deletion invalidate the relation data stored outside React. + // eslint-disable-next-line react-hooks/exhaustive-deps + [relationSchemaRevision], + ); const allNodes = useMemo(() => getDiscourseNodes(), []); const validRelations = useMemo(() => { diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx index 3669a62022..50069d1dbf 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx @@ -9,6 +9,7 @@ import { getRelationColor, } from "./DiscourseRelationUtil"; import { discourseContext } from "~/components/canvas/Tldraw"; +import { isAcceptedRelationSchema } from "~/utils/relationSchemaAcceptance"; import { dispatchToastEvent } from "~/components/canvas/ToastListener"; import { isRelationComplete } from "~/utils/isRelationComplete"; import { @@ -350,7 +351,9 @@ export const createAllRelationShapeTools = ( override onEnter = () => { this.didTimeout = false; - const selectedRelations = discourseContext.relations[name] || []; + const selectedRelations = ( + discourseContext.relations[name] || [] + ).filter(isAcceptedRelationSchema); const hasIncompleteSelectedRelation = selectedRelations.some( (relation) => !isRelationComplete(relation), ); @@ -384,7 +387,7 @@ export const createAllRelationShapeTools = ( target && isDiscourseNodeShape(target) ? getDiscourseNodeTypeId({ shape: target }) : undefined; - const relation = discourseContext.relations[name].find( + const relation = selectedRelations.find( (r) => r.source === targetNodeTypeId || r.destination === targetNodeTypeId, @@ -392,7 +395,7 @@ export const createAllRelationShapeTools = ( if (relation) { this.shapeType = relation.id; } else { - const acceptableTypes = discourseContext.relations[name] + const acceptableTypes = selectedRelations .flatMap((r) => [ discourseContext.nodes[r.source]?.text, discourseContext.nodes[r.destination]?.text, diff --git a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx index 9af71910d3..9ee4f3c4e6 100644 --- a/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx +++ b/apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationUtil.tsx @@ -67,6 +67,7 @@ import { createReifiedRelation } from "~/utils/createReifiedBlock"; import { getStoredRelationsEnabled } from "~/utils/storedRelations"; import type { DiscourseRelation } from "~/utils/getDiscourseRelations"; import { discourseContext, isPageUid } from "~/components/canvas/Tldraw"; +import { isAcceptedRelationSchema } from "~/utils/relationSchemaAcceptance"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; /** @@ -663,11 +664,11 @@ export const createAllRelationShapeUtils = ( isDirect, isReverse, matchingRelation: foundRelation, - } = this.checkConnectionTypeAcrossLabel( - relation.label, + } = this.checkConnectionTypeAcrossLabel({ + label: relation.label, sourceNodeType, targetNodeType, - ); + }); const matchingRelation = foundRelation ?? relation; if (!isDirect && !isReverse) { @@ -1043,11 +1044,11 @@ export const createAllRelationShapeUtils = ( const endNodeType = getDiscourseNodeTypeId({ shape: endNode }); const { isReverse, matchingRelation } = - this.checkConnectionTypeAcrossLabel( - relation.label, - startNodeType, - endNodeType, - ); + this.checkConnectionTypeAcrossLabel({ + label: relation.label, + sourceNodeType: startNodeType, + targetNodeType: endNodeType, + }); const effectiveRelation = matchingRelation ?? relation; @@ -1765,16 +1766,24 @@ export class BaseDiscourseRelationUtil extends ShapeUtil return checkConnectionType(relation, sourceNodeType, targetNodeType); } - checkConnectionTypeAcrossLabel( - label: string, - sourceNodeType: string, - targetNodeType: string, - ): { + checkConnectionTypeAcrossLabel({ + label, + sourceNodeType, + targetNodeType, + includeProvisional, + }: { + label: string; + sourceNodeType: string; + targetNodeType: string; + includeProvisional?: boolean; + }): { isDirect: boolean; isReverse: boolean; matchingRelation: DiscourseRelation | null; } { - const relationsWithLabel = discourseContext.relations[label]; + const relationsWithLabel = includeProvisional + ? discourseContext.relations[label] + : discourseContext.relations[label]?.filter(isAcceptedRelationSchema); if (!relationsWithLabel) { return { isDirect: false, isReverse: false, matchingRelation: null }; } @@ -1794,7 +1803,9 @@ export class BaseDiscourseRelationUtil extends ShapeUtil } getValidTargetTypes(label: string, sourceNodeType: string): string[] { - const relationsWithLabel = discourseContext.relations[label]; + const relationsWithLabel = discourseContext.relations[label]?.filter( + isAcceptedRelationSchema, + ); if (!relationsWithLabel) return []; const targets = new Set(); @@ -1814,11 +1825,14 @@ export class BaseDiscourseRelationUtil extends ShapeUtil const relation = relations.find((r) => r.id === relationId); if (!relation) return false; - const { isDirect, isReverse } = this.checkConnectionTypeAcrossLabel( - relation.label, + // Validates handle drags of arrows that already exist, so provisional + // relations stay re-bindable; only creation paths filter them out. + const { isDirect, isReverse } = this.checkConnectionTypeAcrossLabel({ + label: relation.label, sourceNodeType, targetNodeType, - ); + includeProvisional: true, + }); return isDirect || isReverse; } diff --git a/apps/roam/src/components/canvas/DiscourseToolPanel.tsx b/apps/roam/src/components/canvas/DiscourseToolPanel.tsx index e9fda93f8a..e1ee82a1cf 100644 --- a/apps/roam/src/components/canvas/DiscourseToolPanel.tsx +++ b/apps/roam/src/components/canvas/DiscourseToolPanel.tsx @@ -15,6 +15,8 @@ import { useAtom } from "@tldraw/state-react"; import { TOOL_ARROW_ICON_SVG, NODE_COLOR_ICON_SVG } from "~/icons"; import { getDiscourseNodeColors } from "~/utils/getDiscourseNodeColors"; import { DEFAULT_WIDTH, DEFAULT_HEIGHT } from "./Tldraw"; +import { hasAcceptedRelationSchema } from "./canvasUtils"; +import { useRelationSchemaRevision } from "~/utils/relationSchemaChanges"; import { DEFAULT_STYLE_PROPS, DISCOURSE_NODE_SHAPE_TYPE, @@ -76,7 +78,12 @@ const DiscourseGraphPanel = ({ [editor], ); - const uniqueRelations = useMemo(() => [...new Set(relations)], [relations]); + const relationSchemaRevision = useRelationSchemaRevision(); + const uniqueRelations = useMemo( + () => [...new Set(relations)].filter(hasAcceptedRelationSchema), + // eslint-disable-next-line react-hooks/exhaustive-deps + [relations, relationSchemaRevision], + ); const currentNodeTool = nodes.find((node) => node.type === currentToolId); const currentRelationTool = uniqueRelations.find( diff --git a/apps/roam/src/components/canvas/Tldraw.tsx b/apps/roam/src/components/canvas/Tldraw.tsx index 505833d02b..235d06ef6e 100644 --- a/apps/roam/src/components/canvas/Tldraw.tsx +++ b/apps/roam/src/components/canvas/Tldraw.tsx @@ -1,3 +1,4 @@ +import { subscribeToRelationSchemaChanges } from "~/utils/relationSchemaChanges"; import React, { useState, useRef, @@ -54,7 +55,7 @@ import { import "tldraw/tldraw.css"; import tldrawStyles from "./tldrawStyles"; import { DragHandleOverlay } from "./overlays/DragHandleOverlay"; -import { isDiscourseNodeShape } from "./canvasUtils"; +import { hasAcceptedRelationSchema, isDiscourseNodeShape } from "./canvasUtils"; import getDiscourseNodes, { DiscourseNode } from "~/utils/getDiscourseNodes"; import getDiscourseRelations, { DiscourseRelation, @@ -766,6 +767,7 @@ const TldrawCanvasShared = ({ }, {} as Record, ); + return relations; }, []); const allRelationsById = useMemo(() => { @@ -777,9 +779,29 @@ const TldrawCanvasShared = ({ const allRelationIds = useMemo(() => { return Object.keys(allRelationsById); }, [allRelationsById]); - const allRelationNames = useMemo(() => { - return Object.keys(discourseContext.relations); - }, []); + const registeredRelationNames = useMemo( + () => [...new Set(allRelations.map((relation) => relation.label))], + [allRelations], + ); + const allRelationNames = useMemo( + () => registeredRelationNames.filter(hasAcceptedRelationSchema), + [registeredRelationNames], + ); + useEffect( + () => + subscribeToRelationSchemaChanges(() => { + const editor = appRef.current; + if (!editor) return; + const tool = editor.getCurrentToolId(); + if ( + registeredRelationNames.includes(tool) && + !hasAcceptedRelationSchema(tool) + ) { + editor.setCurrentTool("select"); + } + }), + [registeredRelationNames], + ); const allNodes = useMemo(() => { const allNodes = getDiscourseNodes(); discourseContext.nodes = Object.fromEntries( @@ -1025,7 +1047,9 @@ const TldrawCanvasShared = ({ static override isLockable = true; }; const discourseNodeTools = createNodeShapeTools(allNodes); - const discourseRelationTools = createAllRelationShapeTools(allRelationNames); + const discourseRelationTools = createAllRelationShapeTools( + registeredRelationNames, + ); const referencedNodeTools = createAllReferencedNodeTools( allAddReferencedNodeByAction, ); diff --git a/apps/roam/src/components/canvas/canvasUtils.ts b/apps/roam/src/components/canvas/canvasUtils.ts index 5bb71dbbc4..714e65fc8e 100644 --- a/apps/roam/src/components/canvas/canvasUtils.ts +++ b/apps/roam/src/components/canvas/canvasUtils.ts @@ -4,6 +4,7 @@ import { DiscourseNodeShape, } from "~/components/canvas/DiscourseNodeUtil"; import { discourseContext } from "~/components/canvas/Tldraw"; +import { isAcceptedRelationSchema } from "~/utils/relationSchemaAcceptance"; export const isDiscourseNodeShape = ( editor: Editor, @@ -19,6 +20,12 @@ export const isDiscourseNodeShape = ( export const getAllRelations = () => Object.values(discourseContext.relations).flat(); +export const getCreatableRelations = () => + getAllRelations().filter(isAcceptedRelationSchema); + +export const hasAcceptedRelationSchema = (relationLabel: string): boolean => + !!discourseContext.relations[relationLabel]?.some(isAcceptedRelationSchema); + export const checkConnectionType = ( relation: { source: string; destination: string }, sourceNodeType: string, @@ -36,7 +43,7 @@ export const hasValidRelationTypes = ( sourceNodeType: string, targetNodeType: string, ): boolean => - getAllRelations().some( + getCreatableRelations().some( (r) => (r.source === sourceNodeType && r.destination === targetNodeType) || (r.source === targetNodeType && r.destination === sourceNodeType), diff --git a/apps/roam/src/components/canvas/overlays/relationCreation.ts b/apps/roam/src/components/canvas/overlays/relationCreation.ts index ede2fc6aff..0daeefd060 100644 --- a/apps/roam/src/components/canvas/overlays/relationCreation.ts +++ b/apps/roam/src/components/canvas/overlays/relationCreation.ts @@ -13,7 +13,7 @@ import { createOrUpdateArrowBinding } from "~/components/canvas/DiscourseRelatio import { getDiscourseNodeTypeId } from "~/components/canvas/DiscourseNodeUtil"; import { checkConnectionType, - getAllRelations, + getCreatableRelations, isDiscourseNodeShape, } from "~/components/canvas/canvasUtils"; import type { DiscourseRelation } from "~/utils/getDiscourseRelations"; @@ -93,7 +93,7 @@ export const getValidRelationTypesBetween = ( const validTypes: RelationTypeOption[] = []; const seenLabels = new Set(); - for (const relation of getAllRelations()) { + for (const relation of getCreatableRelations()) { if (!isRelationComplete(relation)) continue; const { isDirect, isReverse } = checkConnectionType( relation, @@ -129,7 +129,9 @@ export const createDefaultRelationBetweenNodes = async ({ sourceId: TLShapeId; targetId: TLShapeId; }): Promise => { - const selectedRelation = getAllRelations().find((r) => r.id === relationId); + const selectedRelation = getCreatableRelations().find( + (r) => r.id === relationId, + ); if (!selectedRelation) return null; const sourceNode = editor.getShape(sourceId); diff --git a/apps/roam/src/components/canvas/uiOverrides.tsx b/apps/roam/src/components/canvas/uiOverrides.tsx index d3d8cffd2d..cd03d2c6e6 100644 --- a/apps/roam/src/components/canvas/uiOverrides.tsx +++ b/apps/roam/src/components/canvas/uiOverrides.tsx @@ -66,7 +66,7 @@ import { getValidRelationTypesBetween, persistRelationArrow, } from "./overlays/relationCreation"; -import { getAllRelations } from "./canvasUtils"; +import { getCreatableRelations } from "./canvasUtils"; import { createOrUpdateArrowBinding } from "./DiscourseRelationShape/helpers"; import DiscourseGraphPanel from "./DiscourseToolPanel"; import type { CanvasNodeShortcuts } from "~/components/settings/utils/zodSchema"; @@ -300,7 +300,9 @@ const convertArrowToRelation = async ({ const boundNodes = getArrowBoundNodeInfo(editor, arrow); if (!boundNodes) return null; - const selectedRelation = getAllRelations().find((r) => r.id === relationId); + const selectedRelation = getCreatableRelations().find( + (r) => r.id === relationId, + ); if (!selectedRelation) return null; const sourceNode = editor.getShape(boundNodes.startId); diff --git a/apps/roam/src/components/settings/DiscourseRelationConfigPanel.tsx b/apps/roam/src/components/settings/DiscourseRelationConfigPanel.tsx index 3f85a04ba0..73f6bf5883 100644 --- a/apps/roam/src/components/settings/DiscourseRelationConfigPanel.tsx +++ b/apps/roam/src/components/settings/DiscourseRelationConfigPanel.tsx @@ -10,6 +10,7 @@ import { SpinnerSize, Tab, Tabs, + Tag, Tooltip, HTMLTable, ControlGroup, @@ -21,6 +22,7 @@ import React, { useCallback, useEffect, useMemo, + useReducer, useRef, useState, } from "react"; @@ -65,6 +67,16 @@ import { type RelationSort, type RelationSortColumn, } from "~/utils/sortRelations"; +import { + acceptImportedRelationSchema, + readRelationSchemaImportMeta, + type RelationSchemaImportMeta, +} from "~/utils/relationSchemaAcceptance"; +import { getReifiedRelations } from "~/utils/createReifiedBlock"; +import { ridToSpaceUriAndLocalId } from "@repo/database/lib/rid"; +import { ROAM_URL_PREFIX } from "~/utils/canonicalRoamUrl"; +import { deleteRelationSchema } from "~/utils/deleteRelationSchema"; +import internalError from "~/utils/internalError"; const DEFAULT_SELECTED_RELATION = { display: "none", @@ -976,6 +988,14 @@ type Relation = { source: string | undefined; destination: string | undefined; }; +type ImportedRelation = Relation & { importMeta: RelationSchemaImportMeta }; + +const formatImportedSource = (sourceNodeRid: string): string => { + const { spaceUri } = ridToSpaceUriAndLocalId(sourceNodeRid); + return spaceUri.startsWith(ROAM_URL_PREFIX) + ? spaceUri.slice(ROAM_URL_PREFIX.length) + : spaceUri; +}; const DiscourseRelationConfigPanel = ({ uid, parentUid, @@ -1037,6 +1057,17 @@ const DiscourseRelationConfigPanel = ({ : visibleRelations, [nodes, sort, visibleRelations], ); + // Acceptance lives in block props, not in the relations state, so the split + // below is recomputed on every render and accepting bumps this reducer to + // trigger one. + const [, refreshImportMeta] = useReducer((version: number) => version + 1, 0); + const localRelations: Relation[] = []; + const importedRelations: ImportedRelation[] = []; + for (const rel of sortedRelations) { + const importMeta = readRelationSchemaImportMeta(rel.uid); + if (importMeta) importedRelations.push({ ...rel, importMeta }); + else localRelations.push(rel); + } const editingRelationInfo = useMemo( () => editingRelation ? getFullTreeByParentUid(editingRelation) : undefined, @@ -1069,15 +1100,52 @@ const DiscourseRelationConfigPanel = ({ setEditingRelation(rel.uid); }; - const handleDelete = (rel: Relation) => { - void deleteBlock(rel.uid).then(() => { - const { [rel.uid]: _, ...remaining } = getGlobalSettings().Relations; - setGlobalSetting([GLOBAL_KEYS.relations], remaining); - setTimeout(() => { - refreshConfigTree(); - setRelations(refreshRelations()); - }, 50); - }); + const handleDelete = async (rel: Relation): Promise => { + await deleteRelationSchema(rel.uid); + setRelations(refreshRelations()); + }; + const handleAcceptImported = (rel: Relation) => { + void acceptImportedRelationSchema(rel.uid) + .then(() => { + posthog.capture("Discourse Relation: Accepted", { + relationUid: rel.uid, + }); + refreshImportMeta(); + }) + .catch((error: unknown) => { + internalError({ + error, + type: "Discourse Relation: Accept failed", + userMessage: "Could not accept the imported relation.", + }); + }); + }; + const handleDeleteImported = (rel: Relation) => { + void getReifiedRelations() + .then((reifiedRelations) => { + const inUseCount = reifiedRelations.filter( + (r) => r.hasSchema === rel.uid, + ).length; + if (inUseCount > 0) { + renderToast({ + id: "discourse-relation-delete-blocked", + intent: Intent.WARNING, + content: `Cannot delete this imported relation: ${inUseCount} relation ${ + inUseCount === 1 ? "instance uses" : "instances use" + } it in this graph.`, + }); + setDeleteConfirmation(null); + return; + } + return handleDelete(rel); + }) + .catch((error: unknown) => { + internalError({ + error, + type: "Discourse Relation: Delete imported failed", + userMessage: "Could not delete the imported relation.", + }); + }); }; const handleDuplicate = (rel: Relation) => { const text = rel.text; @@ -1183,7 +1251,7 @@ const DiscourseRelationConfigPanel = ({ - {sortedRelations.map((rel) => ( + {localRelations.map((rel) => ( handleEdit(rel)}> {nodes[rel.source || ""]?.label} @@ -1222,7 +1290,13 @@ const DiscourseRelationConfigPanel = ({ intent={Intent.DANGER} onClick={(e) => { e.stopPropagation(); - handleDelete(rel); + void handleDelete(rel).catch((error: unknown) => { + internalError({ + error, + type: "Discourse Relation: Delete failed", + userMessage: "Could not delete the relation.", + }); + }); }} className={`mx-1 ${ deleteConfirmation !== rel.uid ? "opacity-0" : "" @@ -1243,6 +1317,104 @@ const DiscourseRelationConfigPanel = ({ ))} + {importedRelations.length > 0 && ( + <> +

Imported relations

+

+ Imported relations are read-only. Accept a relation to enable it for + local use. +

+ + + + {renderSortableHeader("Source", "source")} + {renderSortableHeader("Relation", "relation")} + {renderSortableHeader("Destination", "destination")} + From + Status + Actions + + + + {importedRelations.map((rel) => { + const isProvisional = rel.importMeta.status === "provisional"; + return ( + + + {nodes[rel.source || ""]?.label} + + {rel.text} + + {nodes[rel.destination || ""]?.label} + + + {formatImportedSource( + rel.importMeta.importedFrom.sourceNodeRid, + )} + + + {isProvisional ? ( + + Provisional + + ) : ( + + Accepted + + )} + + +
+ {deleteConfirmation === rel.uid ? ( + <> + + + + ) : ( + <> + {isProvisional && ( + +
+ + + ); + })} + + + + )} ); }; diff --git a/apps/roam/src/utils/__tests__/conceptConversion.test.ts b/apps/roam/src/utils/__tests__/conceptConversion.test.ts index 67252fc56f..c73c7c836f 100644 --- a/apps/roam/src/utils/__tests__/conceptConversion.test.ts +++ b/apps/roam/src/utils/__tests__/conceptConversion.test.ts @@ -108,23 +108,50 @@ describe("discourseNodeSchemaToLocalConcept source slot", () => { }); }); + // Not covered: an upsert whose author_local_id doesn't resolve nulls author_id. it("carries the type author as author_local_id", () => { const concept = discourseNodeSchemaToLocalConcept(CONTEXT, nodeType({})); expect(concept.author_local_id).toBe("author-1"); }); +}); - it("keeps the label and template it already carried", () => { +describe("discourseNodeSchemaToLocalConcept label and template", () => { + it("writes the template body to template_content, with no template title", () => { const concept = discourseNodeSchemaToLocalConcept( CONTEXT, - nodeType({ template: [{ text: "Question:" }] }), + nodeType({ + template: [{ text: "Question:", children: [{ text: "Answer" }] }], + }), ); expect(concept.literal_content).toEqual({ label: "Evidence", format: "[[EVD]] - {content} - {Source}", - template: "* Question:\n", + template_content: "* Question:\n * Answer\n \n", roles: ["sourceDocument"], }); }); + + it.each([ + ["no template", undefined], + ["an empty template", []], + ["a template of only components", [{ text: "{{query block}}" }]], + ])("omits template_content for %s", (_label, template) => { + const concept = discourseNodeSchemaToLocalConcept( + CONTEXT, + nodeType({ template }), + ); + expect(concept.literal_content).not.toHaveProperty("template_content"); + expect(concept.literal_content).not.toHaveProperty("template"); + }); + + it("keeps a slash-separated name whole in label and name", () => { + const concept = discourseNodeSchemaToLocalConcept( + CONTEXT, + nodeType({ text: "Evidence/Figure" }), + ); + expect(concept.name).toBe("Evidence/Figure"); + expect(concept.literal_content).toMatchObject({ label: "Evidence/Figure" }); + }); }); describe("discourseNodeBlockToLocalConcept core title", () => { diff --git a/apps/roam/src/utils/__tests__/deleteRelationSchema.test.ts b/apps/roam/src/utils/__tests__/deleteRelationSchema.test.ts new file mode 100644 index 0000000000..9af73b4b27 --- /dev/null +++ b/apps/roam/src/utils/__tests__/deleteRelationSchema.test.ts @@ -0,0 +1,79 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { deleteRelationSchema } from "~/utils/deleteRelationSchema"; +import { + isRelationSchemaDeleted, + subscribeToRelationSchemaChanges, +} from "~/utils/relationSchemaChanges"; + +const mocks = vi.hoisted(() => ({ + deleteBlock: vi.fn(), + setSetting: vi.fn(), + refresh: vi.fn(), +})); +vi.mock("roamjs-components/writes/deleteBlock", () => ({ + default: mocks.deleteBlock, +})); +vi.mock("~/components/settings/utils/accessors", () => ({ + getGlobalSettings: () => ({ + Relations: { deleted: { label: "supports" }, kept: { label: "opposes" } }, + }), + setGlobalSetting: mocks.setSetting, +})); +vi.mock("~/utils/refreshConfigTree", () => ({ default: mocks.refresh })); +beforeEach(() => { + vi.resetAllMocks(); + vi.useFakeTimers(); +}); +afterEach(() => { + vi.useRealTimers(); +}); + +it("blocks creation immediately after deletion and waits for configuration refresh", async () => { + const listener = vi.fn(); + const unsubscribe = subscribeToRelationSchemaChanges(listener); + try { + const finished = vi.fn(); + const pending = deleteRelationSchema("deleted").then(finished); + await Promise.resolve(); + expect(isRelationSchemaDeleted("deleted")).toBe(true); + expect(listener).toHaveBeenCalledOnce(); + expect(mocks.setSetting).toHaveBeenCalledWith(["Relations"], { + kept: { label: "opposes" }, + }); + expect(finished).not.toHaveBeenCalled(); + await vi.runAllTimersAsync(); + await pending; + expect(mocks.refresh).toHaveBeenCalledOnce(); + expect(finished).toHaveBeenCalledOnce(); + } finally { + unsubscribe(); + } +}); +it("propagates a failed block deletion without disabling the schema", async () => { + mocks.deleteBlock.mockRejectedValueOnce(new Error("delete failed")); + await expect(deleteRelationSchema("failed-delete")).rejects.toThrow( + "delete failed", + ); + expect(isRelationSchemaDeleted("failed-delete")).toBe(false); + expect(mocks.setSetting).not.toHaveBeenCalled(); +}); +it("propagates setting failures while keeping the deleted schema unavailable", async () => { + mocks.setSetting.mockImplementationOnce(() => { + throw new Error("settings failed"); + }); + await expect(deleteRelationSchema("failed-settings")).rejects.toThrow( + "settings failed", + ); + expect(isRelationSchemaDeleted("failed-settings")).toBe(true); +}); +it("propagates configuration refresh failures to the caller", async () => { + mocks.refresh.mockImplementationOnce(() => { + throw new Error("refresh failed"); + }); + const pending = expect( + deleteRelationSchema("failed-refresh"), + ).rejects.toThrow("refresh failed"); + await vi.runAllTimersAsync(); + await pending; + expect(isRelationSchemaDeleted("failed-refresh")).toBe(true); +}); diff --git a/apps/roam/src/utils/__tests__/discourseGraphPanel.test.ts b/apps/roam/src/utils/__tests__/discourseGraphPanel.test.ts new file mode 100644 index 0000000000..b32714b8ed --- /dev/null +++ b/apps/roam/src/utils/__tests__/discourseGraphPanel.test.ts @@ -0,0 +1,144 @@ +// @vitest-environment jsdom +import React from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { act } from "react-dom/test-utils"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import DiscourseGraphPanel from "~/components/canvas/DiscourseToolPanel"; +import { + DISCOURSE_GRAPH_PROP_NAME, + IMPORTED_FROM_PROP_KEY, +} from "~/utils/createReifiedBlock"; +import { acceptImportedRelationSchema } from "~/utils/relationSchemaAcceptance"; +import { markRelationSchemaDeleted } from "~/utils/relationSchemaChanges"; +import type { json } from "~/utils/getBlockProps"; + +const mocks = vi.hoisted(() => ({ + props: new Map>(), + discourseContext: { + nodes: {}, + relations: {}, + }, +})); +vi.mock("tldraw", () => { + const editor = { + getCurrentToolId: () => "discourse-tool", + getZoomLevel: () => 1, + setCurrentTool: vi.fn(), + }; + return { + useEditor: () => editor, + useValue: (_name: string, compute: () => unknown) => compute(), + useQuickReactor: vi.fn(), + createShapeId: vi.fn(), + Vec: class {}, + Box: class {}, + FONT_FAMILIES: { sans: "sans" }, + }; +}); +vi.mock("@tldraw/state-react", () => ({ + useAtom: (_name: string, initial: () => unknown) => { + const value = initial(); + return { get: () => value, set: vi.fn() }; + }, +})); +vi.mock("~/components/canvas/Tldraw", () => ({ + DEFAULT_WIDTH: 160, + DEFAULT_HEIGHT: 64, + discourseContext: mocks.discourseContext, +})); +vi.mock("~/components/canvas/DiscourseNodeUtil", () => ({ + DiscourseNodeUtil: class {}, + DEFAULT_STYLE_PROPS: {}, + DISCOURSE_NODE_SHAPE_TYPE: "discourse-node", + FONT_SIZES: { s: 12 }, +})); +vi.mock( + "~/components/canvas/DiscourseRelationShape/DiscourseRelationUtil", + () => ({ getRelationColor: () => "black" }), +); +vi.mock("~/components/settings/DiscourseNodeCanvasSettings", () => ({ + formatHexColor: () => "", +})); +vi.mock("~/utils/getDiscourseNodeColors", () => ({ + getDiscourseNodeColors: () => ({ backgroundColor: "", textColor: "" }), +})); +vi.mock("~/icons", () => ({ + TOOL_ARROW_ICON_SVG: "", + NODE_COLOR_ICON_SVG: "", +})); +vi.mock("~/utils/internalError", () => ({ default: vi.fn() })); + +const importedFrom = { + [IMPORTED_FROM_PROP_KEY]: { + sourceNodeRid: "orn:obsidian.schema:vault/relation", + sourceModifiedAt: "2026-08-01T00:00:00.000Z", + }, +}; + +let container: HTMLDivElement; +let root: Root; +beforeEach(() => { + mocks.props.clear(); + mocks.props.set("imported-supports", { + [DISCOURSE_GRAPH_PROP_NAME]: importedFrom, + }); + mocks.discourseContext.relations = { + opposes: [{ id: "local-opposes" }], + supports: [{ id: "imported-supports" }], + }; + Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + Object.assign(window, { + roamAlphaAPI: { + pull: (_pattern: string, [, uid]: [string, string]) => ({ + ":block/props": mocks.props.get(uid) ?? {}, + }), + data: { + block: { + update: ({ + block, + }: { + block: { uid: string; props: Record }; + }) => { + mocks.props.set(block.uid, block.props); + return Promise.resolve(); + }, + }, + }, + }, + }); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); +afterEach(() => { + act(() => root.unmount()); + container.remove(); +}); + +const listedRelations = (): string[] => + [...container.querySelectorAll("[data-drag_item_index] > span:last-child")] + .map((span) => span.textContent ?? "") + .filter((text) => text in mocks.discourseContext.relations); + +it("refilters relation tools itself when a schema is accepted or deleted", async () => { + await act(() => { + root.render( + React.createElement(DiscourseGraphPanel, { + nodes: [], + relations: ["opposes", "supports"], + }), + ); + return Promise.resolve(); + }); + expect(listedRelations()).toEqual(["opposes"]); + + await act(async () => { + await acceptImportedRelationSchema("imported-supports"); + }); + expect(listedRelations()).toEqual(["opposes", "supports"]); + + act(() => { + markRelationSchemaDeleted("imported-supports"); + }); + expect(listedRelations()).toEqual(["opposes"]); +}); diff --git a/apps/roam/src/utils/__tests__/importSharedNodes.test.ts b/apps/roam/src/utils/__tests__/importSharedNodes.test.ts index 50a0f01b57..e0f75c6ce7 100644 --- a/apps/roam/src/utils/__tests__/importSharedNodes.test.ts +++ b/apps/roam/src/utils/__tests__/importSharedNodes.test.ts @@ -109,9 +109,17 @@ describe("importSharedNodes", () => { }); expect(items).toEqual([ - { sharedNode: sharedNodes[0], status: "imported" }, - { sharedNode: sharedNodes[1], status: "imported" }, - { sharedNode: sharedNodes[2], status: "skipped" }, + { + sharedNode: sharedNodes[0], + status: "imported", + pageUid: "page-node-1", + }, + { + sharedNode: sharedNodes[1], + status: "imported", + pageUid: "page-node-2", + }, + { sharedNode: sharedNodes[2], status: "skipped", pageUid: "page-node-3" }, { sharedNode: sharedNodes[3], status: "failed", @@ -270,6 +278,7 @@ describe("importSharedNodes", () => { { sharedNode: sharedNodes[0], status: "imported", + pageUid: "page-node-1", warning: "No source was published with this node.", }, ]); @@ -294,7 +303,11 @@ describe("importSharedNodes", () => { status: "failed", message: "roam api unavailable", }, - { sharedNode: sharedNodes[1], status: "imported" }, + { + sharedNode: sharedNodes[1], + status: "imported", + pageUid: "page-node-2", + }, ]); }); it("imports a discovered source this graph lacks before the node that names it", async () => { @@ -331,8 +344,8 @@ describe("importSharedNodes", () => { mockedMaterializeSharedNode.mock.calls.map(([args]) => args.sharedNode), ).toEqual([source, evidence]); expect(items).toEqual([ - { sharedNode: source, status: "imported" }, - { sharedNode: evidence, status: "imported" }, + { sharedNode: source, status: "imported", pageUid: "page-source" }, + { sharedNode: evidence, status: "imported", pageUid: "page-evidence" }, ]); expect(onProgress.mock.calls).toEqual([ [1, 2], @@ -361,7 +374,9 @@ describe("importSharedNodes", () => { expect(mockedFindImportedNodeUidBySourceRid).toHaveBeenCalledWith( source.rid, ); - expect(items).toEqual([{ sharedNode: evidence, status: "imported" }]); + expect(items).toEqual([ + { sharedNode: evidence, status: "imported", pageUid: "page-evidence" }, + ]); }); it("does not add a source that is selected", async () => { @@ -403,7 +418,9 @@ describe("importSharedNodes", () => { expect(mockedFindImportedNodeUidBySourceRid).not.toHaveBeenCalled(); expect(mockedMaterializeSharedNode).toHaveBeenCalledTimes(1); - expect(items).toEqual([{ sharedNode: evidence, status: "imported" }]); + expect(items).toEqual([ + { sharedNode: evidence, status: "imported", pageUid: "page-evidence" }, + ]); }); it("adds a source once when several nodes name it by a bare id", async () => { diff --git a/apps/roam/src/utils/__tests__/importSharedRelations.test.ts b/apps/roam/src/utils/__tests__/importSharedRelations.test.ts index 80e4d95c8b..50b8725e4f 100644 --- a/apps/roam/src/utils/__tests__/importSharedRelations.test.ts +++ b/apps/roam/src/utils/__tests__/importSharedRelations.test.ts @@ -2,12 +2,15 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { DGSupabaseClient } from "@repo/database/lib/client"; import type { DiscourseRelation } from "~/utils/getDiscourseRelations"; import { importSharedRelations } from "~/utils/importSharedRelations"; +import refreshConfigTree from "~/utils/refreshConfigTree"; +import { writeImportedSourceIdentity } from "~/utils/importedSourceIdentity"; import getDiscourseRelations from "~/utils/getDiscourseRelations"; import { createRelationSchema } from "~/utils/createRelationSchema"; vi.hoisted(() => { vi.stubGlobal("window", { roamAlphaAPI: { graph: { name: "local" } } }); }); +vi.mock("~/utils/refreshConfigTree", () => ({ default: vi.fn() })); vi.mock("~/utils/getDiscourseRelations", () => ({ default: vi.fn() })); vi.mock("~/utils/getDiscourseNodes", () => ({ default: () => [{ type: "local-claim", text: "Claim" }], @@ -70,6 +73,23 @@ const client = {} as DGSupabaseClient; beforeEach(() => vi.clearAllMocks()); describe("importSharedRelations schema matching", () => { + it("refreshes the grammar after storing a new schema and its provenance", async () => { + vi.mocked(getDiscourseRelations).mockReturnValue([]); + vi.mocked(createRelationSchema).mockResolvedValue("imported-supports"); + await importSharedRelations(client, 7); + expect(writeImportedSourceIdentity).toHaveBeenCalledWith( + expect.objectContaining({ + pageUid: "imported-supports", + sourceNodeRid: "orn:obsidian.schema:remote/supports", + }), + ); + expect(refreshConfigTree).toHaveBeenCalledOnce(); + expect( + vi.mocked(refreshConfigTree).mock.invocationCallOrder[0], + ).toBeGreaterThan( + vi.mocked(writeImportedSourceIdentity).mock.invocationCallOrder[0], + ); + }); it("reuses one schema when its query patterns produce multiple matches", async () => { vi.mocked(getDiscourseRelations).mockReturnValue([ { diff --git a/apps/roam/src/utils/__tests__/relationSchemaAcceptance.test.ts b/apps/roam/src/utils/__tests__/relationSchemaAcceptance.test.ts new file mode 100644 index 0000000000..ac2b0ca78e --- /dev/null +++ b/apps/roam/src/utils/__tests__/relationSchemaAcceptance.test.ts @@ -0,0 +1,154 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + DISCOURSE_GRAPH_PROP_NAME, + IMPORTED_FROM_PROP_KEY, +} from "~/utils/createReifiedBlock"; +import { + acceptImportedRelationSchema, + excludeProvisionalRelationSchemas, + isAcceptedRelationSchema, + isProvisionalRelationSchema, + readRelationSchemaImportMeta, + RELATION_SCHEMA_STATUS_PROP_KEY, +} from "~/utils/relationSchemaAcceptance"; +import { markRelationSchemaDeleted } from "~/utils/relationSchemaChanges"; +import type { json } from "~/utils/getBlockProps"; + +vi.mock("~/utils/internalError", () => ({ default: vi.fn() })); + +const SOURCE_NODE_RID = "orn:obsidian.schema:vault-a/relation-type-1"; +const SOURCE_MODIFIED_AT = "2026-08-01T12:00:00.000Z"; +const SCHEMA_UID = "relation-schema-uid"; + +const importedFromProps = { + [IMPORTED_FROM_PROP_KEY]: { + sourceModifiedAt: SOURCE_MODIFIED_AT, + sourceNodeRid: SOURCE_NODE_RID, + }, +}; + +const propsByUid = new Map>(); + +const setRoamAlphaApi = (): void => { + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + data: { + block: { + update: vi.fn( + ({ + block, + }: { + block: { props: Record; uid: string }; + }) => { + propsByUid.set(block.uid, block.props); + return Promise.resolve(); + }, + ), + }, + }, + pull: (_pattern: string, [, uid]: [string, string]) => ({ + ":block/props": propsByUid.get(uid) ?? {}, + }), + }, + }; +}; + +beforeEach(() => { + propsByUid.clear(); + setRoamAlphaApi(); +}); + +describe("relation schema import meta", () => { + it("returns undefined for local schemas without imported provenance", () => { + expect(readRelationSchemaImportMeta(SCHEMA_UID)).toBeUndefined(); + expect(isProvisionalRelationSchema(SCHEMA_UID)).toBe(false); + }); + + it("treats imported schemas without a status as provisional", () => { + propsByUid.set(SCHEMA_UID, { + [DISCOURSE_GRAPH_PROP_NAME]: importedFromProps, + }); + + expect(readRelationSchemaImportMeta(SCHEMA_UID)).toEqual({ + importedFrom: { + sourceModifiedAt: SOURCE_MODIFIED_AT, + sourceNodeRid: SOURCE_NODE_RID, + }, + status: "provisional", + }); + expect(isProvisionalRelationSchema(SCHEMA_UID)).toBe(true); + }); + + it("treats imported schemas with an accepted status as accepted", () => { + propsByUid.set(SCHEMA_UID, { + [DISCOURSE_GRAPH_PROP_NAME]: { + ...importedFromProps, + [RELATION_SCHEMA_STATUS_PROP_KEY]: "accepted", + }, + }); + + expect(readRelationSchemaImportMeta(SCHEMA_UID)?.status).toBe("accepted"); + expect(isProvisionalRelationSchema(SCHEMA_UID)).toBe(false); + }); +}); + +describe("acceptImportedRelationSchema", () => { + it("marks the schema accepted while preserving imported provenance", async () => { + propsByUid.set(SCHEMA_UID, { + [DISCOURSE_GRAPH_PROP_NAME]: importedFromProps, + "other-extension": { enabled: true }, + }); + + await acceptImportedRelationSchema(SCHEMA_UID); + + expect(propsByUid.get(SCHEMA_UID)).toEqual({ + [DISCOURSE_GRAPH_PROP_NAME]: { + ...importedFromProps, + [RELATION_SCHEMA_STATUS_PROP_KEY]: "accepted", + }, + "other-extension": { enabled: true }, + }); + expect(readRelationSchemaImportMeta(SCHEMA_UID)?.status).toBe("accepted"); + }); +}); + +describe("excludeProvisionalRelationSchemas", () => { + it("filters provisional schemas but keeps local and accepted ones", () => { + propsByUid.set("provisional-uid", { + [DISCOURSE_GRAPH_PROP_NAME]: importedFromProps, + }); + propsByUid.set("accepted-uid", { + [DISCOURSE_GRAPH_PROP_NAME]: { + ...importedFromProps, + [RELATION_SCHEMA_STATUS_PROP_KEY]: "accepted", + }, + }); + + const relations = [ + { id: "local-uid" }, + { id: "provisional-uid" }, + { id: "accepted-uid" }, + ]; + + expect(excludeProvisionalRelationSchemas(relations)).toEqual([ + { id: "local-uid" }, + { id: "accepted-uid" }, + ]); + }); +}); + +describe("isAcceptedRelationSchema", () => { + it("reads acceptance and deletion live, without a refresh step", async () => { + const schema = { id: "live-schema-uid" }; + propsByUid.set(schema.id, { + [DISCOURSE_GRAPH_PROP_NAME]: importedFromProps, + }); + expect(isAcceptedRelationSchema(schema)).toBe(false); + + await acceptImportedRelationSchema(schema.id); + expect(isAcceptedRelationSchema(schema)).toBe(true); + + markRelationSchemaDeleted(schema.id); + expect(isAcceptedRelationSchema(schema)).toBe(false); + }); +}); diff --git a/apps/roam/src/utils/__tests__/relationSchemaCreationRefresh.test.ts b/apps/roam/src/utils/__tests__/relationSchemaCreationRefresh.test.ts new file mode 100644 index 0000000000..5830f8051a --- /dev/null +++ b/apps/roam/src/utils/__tests__/relationSchemaCreationRefresh.test.ts @@ -0,0 +1,172 @@ +// @vitest-environment jsdom +import React from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { act } from "react-dom/test-utils"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { CreateRelationButton } from "~/components/CreateRelationDialog"; +import SuggestionsBody from "~/components/SuggestionsBody"; +import { acceptImportedRelationSchema } from "~/utils/relationSchemaAcceptance"; +import { markRelationSchemaDeleted } from "~/utils/relationSchemaChanges"; +import { + DISCOURSE_GRAPH_PROP_NAME, + IMPORTED_FROM_PROP_KEY, +} from "~/utils/createReifiedBlock"; +import type { json } from "~/utils/getBlockProps"; + +const mocks = vi.hoisted(() => ({ + search: vi.fn().mockResolvedValue([]), + props: new Map>(), +})); +vi.mock("~/utils/hyde", () => ({ performHydeSearch: mocks.search })); +vi.mock("~/utils/getDiscourseNodes", () => ({ + default: () => [ + { type: "source", text: "Source", format: "{content}" }, + { type: "target", text: "Target", format: "{content}" }, + ], +})); +vi.mock("~/utils/findDiscourseNode", () => ({ + default: () => ({ type: "source" }), +})); +vi.mock("~/utils/getDiscourseRelations", () => ({ + default: () => [ + { + id: "mounted-schema", + source: "source", + destination: "target", + label: "supports", + complement: "supported by", + triples: [], + }, + ], +})); +vi.mock("~/utils/getDiscourseContextResults", () => ({ + default: vi.fn().mockResolvedValue([]), +})); +vi.mock("~/utils/storedRelations", () => ({ + getStoredRelationsEnabled: () => true, +})); +vi.mock("~/components/settings/utils/accessors", () => ({ + getGlobalSetting: () => [], +})); +vi.mock("~/utils/internalError", () => ({ default: vi.fn() })); +vi.mock("~/utils/notifySuggestiveModeAdoption", () => ({ + notifyBlockSuggestionAdded: vi.fn(), + notifyRelationSuggestionAdded: vi.fn(), +})); +vi.mock("~/utils/discourseContextMutationRefresh", () => ({ + refreshDiscourseContextsForMutatedUids: vi.fn(), +})); +vi.mock("roamjs-components/queries/getAllPageNames", () => ({ + default: () => [], +})); +vi.mock("roamjs-components/queries/getPageTitleByPageUid", () => ({ + default: () => "Source page", +})); +vi.mock("roamjs-components/queries/getPageUidByPageTitle", () => ({ + default: () => "source-page", +})); +vi.mock("roamjs-components/components/AutocompleteInput", () => ({ + default: () => null, +})); +vi.mock("roamjs-components/components/MenuItemSelect", () => ({ + default: () => null, +})); +vi.mock("roamjs-components/util/renderOverlay", () => ({ default: vi.fn() })); +vi.mock("roamjs-components/components/Toast", () => ({ render: vi.fn() })); +vi.mock("posthog-js", () => ({ default: { capture: vi.fn() } })); + +let container: HTMLDivElement; +let root: Root; +beforeEach(() => { + mocks.props.set("mounted-schema", { + [DISCOURSE_GRAPH_PROP_NAME]: { + [IMPORTED_FROM_PROP_KEY]: { + sourceNodeRid: "orn:obsidian.schema:vault/relation", + sourceModifiedAt: "2026-08-01T00:00:00.000Z", + }, + }, + }); + Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + Object.assign(window, { + roamAlphaAPI: { + pull: (_pattern: string, [, uid]: [string, string]) => ({ + ":block/props": mocks.props.get(uid) ?? {}, + }), + data: { + block: { + update: vi.fn( + ({ + block, + }: { + block: { uid: string; props: Record }; + }) => { + mocks.props.set(block.uid, block.props); + return Promise.resolve(); + }, + ), + }, + backend: { q: vi.fn().mockResolvedValue([]) }, + }, + }, + }); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); +afterEach(() => { + act(() => root.unmount()); + container.remove(); + vi.clearAllMocks(); +}); +const clickAllPages = async (): Promise => { + const button = [...container.querySelectorAll("button")].find( + (b) => b.textContent === "All Pages", + ); + expect(button).toBeDefined(); + await act(() => { + button?.click(); + return Promise.resolve(); + }); +}; +it("refreshes both mounted creation interfaces after acceptance and deletion", async () => { + await act(() => { + root.render( + React.createElement( + React.Fragment, + null, + React.createElement(CreateRelationButton, { + sourceNodeUid: "source-page", + }), + React.createElement(SuggestionsBody, { + tag: "Source page", + blockUid: "suggestions-block", + }), + ), + ); + return Promise.resolve(); + }); + const addButton = [...container.querySelectorAll("button")].find( + (b) => b.textContent === "Add relation", + ); + expect(addButton?.disabled).toBe(true); + await clickAllPages(); + expect(mocks.search).toHaveBeenLastCalledWith( + expect.objectContaining({ validTypes: [], uniqueRelationTypeTriplets: [] }), + ); + await act(async () => { + await acceptImportedRelationSchema("mounted-schema"); + }); + expect(addButton?.disabled).toBe(false); + await clickAllPages(); + expect(mocks.search).toHaveBeenLastCalledWith( + expect.objectContaining({ validTypes: ["target"] }), + ); + act(() => { + markRelationSchemaDeleted("mounted-schema"); + }); + expect(addButton?.disabled).toBe(true); + await clickAllPages(); + expect(mocks.search).toHaveBeenLastCalledWith( + expect.objectContaining({ validTypes: [], uniqueRelationTypeTriplets: [] }), + ); +}); diff --git a/apps/roam/src/utils/__tests__/requestAssetRetry.test.ts b/apps/roam/src/utils/__tests__/requestAssetRetry.test.ts new file mode 100644 index 0000000000..2fdf6dc23c --- /dev/null +++ b/apps/roam/src/utils/__tests__/requestAssetRetry.test.ts @@ -0,0 +1,199 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { DISCOURSE_GRAPH_PROP_NAME } from "~/utils/createReifiedBlock"; +import type { json } from "~/utils/getBlockProps"; +import internalError from "~/utils/internalError"; +import type { NodeAssetResult } from "~/utils/publishNodeAssets"; +import { + NEEDS_REFRESH_PROP_KEY, + requestAssetRetries, +} from "~/utils/requestAssetRetry"; + +vi.mock("~/utils/internalError", () => ({ default: vi.fn() })); + +const NODE_UID = "tgWb6JozF"; +const OTHER_NODE_UID = "Xk3pQr8sT"; +const IMAGE = + "https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2FlqP2ioVNC3.png?alt=media&token=5e6f7a8b"; +const SECOND_IMAGE = + "https://firebasestorage.googleapis.com/v0/b/firescript-577a2.appspot.com/o/imgs%2Fapp%2FMAPLab%2FsecondImage.png?alt=media&token=1a2b3c4d"; + +type Block = { page: string; text: string }; + +const blocks = new Map(); +const propsByUid = new Map>(); +const update = vi.fn( + ({ block }: { block: { uid: string; props: Record } }) => { + propsByUid.set(block.uid, block.props); + return Promise.resolve(); + }, +); + +const query = vi.fn((_query: string, pageUid: string, url: string) => + Promise.resolve( + [...blocks.entries()] + .filter(([, block]) => block.page === pageUid && block.text.includes(url)) + .map(([uid]) => uid), + ), +); + +const failed = (sourceLocalId: string, sourceRef: string): NodeAssetResult => ({ + status: "failed", + sourceLocalId, + sourceRef, + error: "descriptor 500", +}); + +const copied = (sourceLocalId: string, sourceRef: string): NodeAssetResult => + ({ + status: "copied", + sourceLocalId, + sourceRef, + contentHash: "abc", + }) as NodeAssetResult; + +beforeEach(() => { + blocks.clear(); + propsByUid.clear(); + update.mockClear(); + query.mockClear(); + vi.mocked(internalError).mockClear(); + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + data: { async: { q: query }, block: { update } }, + pull: (_pattern: string, [, uid]: [string, string]) => ({ + ":block/props": propsByUid.get(uid) ?? {}, + }), + }, + }; +}); + +describe("requestAssetRetries", () => { + it("marks the block holding a failed asset", async () => { + blocks.set("imgBlock", { + page: NODE_UID, + text: `![](${IMAGE})`, + }); + + const retried = await requestAssetRetries([failed(NODE_UID, IMAGE)]); + + expect(retried).toEqual(new Set([NODE_UID])); + expect(propsByUid.get("imgBlock")).toEqual({ + [DISCOURSE_GRAPH_PROP_NAME]: { [NEEDS_REFRESH_PROP_KEY]: true }, + }); + }); + + it("writes nothing when every asset copied", async () => { + blocks.set("imgBlock", { + page: NODE_UID, + text: `![](${IMAGE})`, + }); + + const retried = await requestAssetRetries([copied(NODE_UID, IMAGE)]); + + expect(retried.size).toBe(0); + expect(update).not.toHaveBeenCalled(); + }); + + it("keeps the block's existing props, including other discourse-graph data", async () => { + blocks.set("imgBlock", { + page: NODE_UID, + text: `![](${IMAGE})`, + }); + propsByUid.set("imgBlock", { + other: "kept", + [DISCOURSE_GRAPH_PROP_NAME]: { importedFrom: { sourceNodeRid: "rid" } }, + }); + + await requestAssetRetries([failed(NODE_UID, IMAGE)]); + + expect(propsByUid.get("imgBlock")).toEqual({ + other: "kept", + [DISCOURSE_GRAPH_PROP_NAME]: { + importedFrom: { sourceNodeRid: "rid" }, + [NEEDS_REFRESH_PROP_KEY]: true, + }, + }); + }); + + it("writes once per node, however many of its assets failed", async () => { + blocks.set("imgBlock", { + page: NODE_UID, + text: `![](${IMAGE})`, + }); + blocks.set("secondBlock", { + page: NODE_UID, + text: `![](${SECOND_IMAGE})`, + }); + + await requestAssetRetries([ + failed(NODE_UID, IMAGE), + failed(NODE_UID, SECOND_IMAGE), + ]); + + expect(update).toHaveBeenCalledTimes(1); + }); + + it("does not retry an asset that only reaches the node from another page", async () => { + blocks.set("refSource", { + page: OTHER_NODE_UID, + text: `![](${IMAGE})`, + }); + + const retried = await requestAssetRetries([failed(NODE_UID, IMAGE)]); + + expect(retried.size).toBe(0); + expect(update).not.toHaveBeenCalled(); + }); + + it("uses another failed asset's block when the first has none on the page", async () => { + blocks.set("secondBlock", { + page: NODE_UID, + text: `![](${SECOND_IMAGE})`, + }); + + const retried = await requestAssetRetries([ + failed(NODE_UID, IMAGE), + failed(NODE_UID, SECOND_IMAGE), + ]); + + expect(retried).toEqual(new Set([NODE_UID])); + expect(propsByUid.has("secondBlock")).toBe(true); + }); + + it("writes props only, so the block's last editor is kept", async () => { + blocks.set("imgBlock", { + page: NODE_UID, + text: `![](${IMAGE})`, + }); + + await requestAssetRetries([failed(NODE_UID, IMAGE)]); + + const [[{ block }]] = update.mock.calls; + expect(Object.keys(block).sort()).toEqual(["props", "uid"]); + }); + + it("reports a failed write and still marks the other nodes", async () => { + blocks.set("imgBlock", { + page: NODE_UID, + text: `![](${IMAGE})`, + }); + blocks.set("otherBlock", { + page: OTHER_NODE_UID, + text: `![](${IMAGE})`, + }); + update.mockRejectedValueOnce(new Error("write refused")); + + const retried = await requestAssetRetries([ + failed(NODE_UID, IMAGE), + failed(OTHER_NODE_UID, IMAGE), + ]); + + expect(retried).toEqual(new Set([OTHER_NODE_UID])); + expect(internalError).toHaveBeenCalledWith( + expect.objectContaining({ + type: "Asset retry failed", + context: { sourceLocalId: NODE_UID }, + }), + ); + }); +}); diff --git a/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts b/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts index a050e04cfa..74992d828d 100644 --- a/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts +++ b/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts @@ -221,6 +221,23 @@ describe("nodeSchemaToCrossApp format", () => { }); }); +describe("nodeSchemaToCrossApp template", () => { + it("carries the template body, with no template title", () => { + const schema = convertSchema( + nodeSchema({ template: [{ text: "Question:" }] }), + ); + expect(schema?.template).toBe("* Question:\n"); + expect(schema?.templateTitle).toBeUndefined(); + }); + + it.each([ + ["no template", undefined], + ["an empty template", []], + ])("leaves the template out for %s", (_label, template) => { + expect(convertSchema(nodeSchema({ template }))?.template).toBeUndefined(); + }); +}); + describe("nodeSchemaToCrossApp source slot", () => { it("adds a sourceDocument slot definition pointing at the Source node type", () => { mockedGetDiscourseNodes.mockReturnValue([ diff --git a/apps/roam/src/utils/__tests__/schemaMetadataParity.test.ts b/apps/roam/src/utils/__tests__/schemaMetadataParity.test.ts new file mode 100644 index 0000000000..b9aa2d1029 --- /dev/null +++ b/apps/roam/src/utils/__tests__/schemaMetadataParity.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, vi } from "vitest"; +import type { LocalConceptDataInput } from "@repo/database/inputTypes"; +import type { DiscourseNode } from "~/utils/getDiscourseNodes"; + +vi.mock("roamjs-components/queries/getFullTreeByParentUid", () => ({ + default: () => ({ children: [] }), +})); +vi.mock("roamjs-components/queries/getPageViewType", () => ({ + default: () => "bullet", +})); +vi.mock("roamjs-components/queries/getPageTitleByPageUid", () => ({ + default: () => "", +})); +vi.mock("~/utils/pageToMarkdown", () => ({ toMarkdown: () => "" })); +vi.mock("~/utils/getDiscourseRelations", () => ({ default: () => [] })); +vi.mock("~/utils/getDiscourseNodes", () => ({ + default: () => [SOURCE_TYPE], +})); + +// Sync reads the author through q, publish through pull; both see one creator. +vi.hoisted(() => { + (globalThis as { window?: unknown }).window = { + roamAlphaAPI: { + util: { generateUID: () => "someUid" }, + q: () => [["author-1", "page-1", 1000, 2000]], + pull: () => ({ + ":create/time": 1000, + ":page/edit-time": 2000, + ":create/user": { ":user/uid": "author-1" }, + }), + }, + }; +}); + +import { crossAppNodeSchemaToDbConcept } from "@repo/database/lib/crossAppConverters"; +import { discourseNodeSchemaToLocalConcept } from "~/utils/conceptConversion"; +import { nodeSchemaToCrossApp } from "~/utils/roamToCrossAppConverters"; + +const CONTEXT = { spaceId: 1, userId: 2 } as never; + +const nodeType = (overrides: Partial): DiscourseNode => ({ + text: "Evidence", + type: "_EVD-node", + shortcut: "e", + format: "[[EVD]] - {content} - {Source}", + specification: [], + backedBy: "user", + canvasSettings: {}, + ...overrides, +}); + +const SOURCE_TYPE = nodeType({ + text: "Source", + type: "src-node", + format: "@{content}", +}); + +// Timestamps and space fields are transport details that differ by path. +const schemaMetadata = (concept: LocalConceptDataInput) => ({ + name: concept.name, + source_local_id: concept.source_local_id, + author_local_id: concept.author_local_id, + literal_content: concept.literal_content, + local_reference_content: concept.local_reference_content, +}); + +const viaSync = (node: DiscourseNode) => + schemaMetadata(discourseNodeSchemaToLocalConcept(CONTEXT, node)); + +const viaPublish = (node: DiscourseNode) => { + const schema = nodeSchemaToCrossApp(node); + if (!schema) throw new Error("publish produced no schema"); + return schemaMetadata(crossAppNodeSchemaToDbConcept(schema)); +}; + +describe("node schema metadata from sync and publish", () => { + it.each([ + [ + "a template and a source slot", + nodeType({ + template: [{ text: "Question:", children: [{ text: "Answer" }] }], + }), + ], + ["a source slot and no template", nodeType({})], + [ + "a template and no source slot", + nodeType({ + text: "Claim", + type: "clm", + format: "[[CLM]] - {content}", + template: [{ text: "Grounds:" }], + }), + ], + [ + "neither a template nor a source slot", + nodeType({ text: "Claim", type: "clm", format: "[[CLM]] - {content}" }), + ], + [ + "a template of only components", + nodeType({ template: [{ text: "{{x}}" }] }), + ], + ["a slash-separated name", nodeType({ text: "Evidence/Figure" })], + ])("match for a schema with %s", (_label, node) => { + expect(viaPublish(node)).toEqual(viaSync(node)); + }); + + it("both carry the creator as author_local_id", () => { + expect(viaSync(nodeType({})).author_local_id).toBe("author-1"); + expect(viaPublish(nodeType({})).author_local_id).toBe("author-1"); + }); +}); diff --git a/apps/roam/src/utils/__tests__/sortSharedNodes.test.ts b/apps/roam/src/utils/__tests__/sortSharedNodes.test.ts new file mode 100644 index 0000000000..10e55de7bc --- /dev/null +++ b/apps/roam/src/utils/__tests__/sortSharedNodes.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "vitest"; +import { + DEFAULT_SHARED_NODE_SORT, + getNextSharedNodeSort, + sortSharedNodes, +} from "~/utils/sortSharedNodes"; + +const older = { + spaceName: "beta vault", + title: "Claim 10", + lastModified: "2026-06-14T09:00:00.000Z", +}; +const newer = { + spaceName: "Alpha graph", + title: "Claim 9", + lastModified: "2026-06-14T15:00:00.000Z", +}; +const newest = { + spaceName: "Gamma vault", + title: "evidence", + lastModified: "2026-06-14T14:00:00.000-02:00", +}; + +describe("getNextSharedNodeSort", () => { + it("sorts a newly clicked column descending", () => { + expect( + getNextSharedNodeSort({ + currentSort: DEFAULT_SHARED_NODE_SORT, + column: "title", + }), + ).toEqual({ column: "title", direction: "descending" }); + }); + + it("toggles the active column between descending and ascending", () => { + const ascending = getNextSharedNodeSort({ + currentSort: DEFAULT_SHARED_NODE_SORT, + column: "lastModified", + }); + expect(ascending).toEqual({ + column: "lastModified", + direction: "ascending", + }); + expect( + getNextSharedNodeSort({ currentSort: ascending, column: "lastModified" }), + ).toEqual(DEFAULT_SHARED_NODE_SORT); + }); +}); + +describe("sortSharedNodes", () => { + it("defaults to most recently modified first, comparing instants", () => { + expect( + sortSharedNodes({ + nodes: [older, newest, newer], + sort: DEFAULT_SHARED_NODE_SORT, + }), + ).toEqual([newest, newer, older]); + }); + + it("sorts text columns ignoring case and comparing numbers by value", () => { + expect( + sortSharedNodes({ + nodes: [newest, older, newer], + sort: { column: "title", direction: "ascending" }, + }), + ).toEqual([newer, older, newest]); + expect( + sortSharedNodes({ + nodes: [newest, older, newer], + sort: { column: "spaceName", direction: "descending" }, + }), + ).toEqual([newest, older, newer]); + }); + + it("does not reorder the input", () => { + const nodes = [older, newest, newer]; + sortSharedNodes({ nodes, sort: DEFAULT_SHARED_NODE_SORT }); + expect(nodes).toEqual([older, newest, newer]); + }); +}); diff --git a/apps/roam/src/utils/__tests__/syncSharedNodeAssets.test.ts b/apps/roam/src/utils/__tests__/syncSharedNodeAssets.test.ts index 33bdf08d75..e73bc8c54b 100644 --- a/apps/roam/src/utils/__tests__/syncSharedNodeAssets.test.ts +++ b/apps/roam/src/utils/__tests__/syncSharedNodeAssets.test.ts @@ -18,6 +18,8 @@ const mocks = vi.hoisted(() => ({ // under test. vi.mock("~/utils/getDiscourseNodes", () => ({ default: () => [] })); vi.mock("~/utils/internalError", () => ({ default: vi.fn() })); +const capture = vi.hoisted(() => vi.fn()); +vi.mock("posthog-js", () => ({ default: { capture } })); vi.mock("~/components/settings/utils/accessors", () => ({ isSyncEnabled: () => false, })); @@ -42,7 +44,10 @@ vi.mock("~/utils/roamToCrossAppConverters", () => ({ }), })); -import { upsertSharedNodesFullContentWithAssets } from "~/utils/syncDgNodesToSupabase"; +import { + reportSharedNodeAssets, + upsertSharedNodesFullContentWithAssets, +} from "~/utils/syncDgNodesToSupabase"; const NODE_UID = "tgWb6JozF"; const SECOND_IMAGE = @@ -171,3 +176,27 @@ describe("upsertSharedNodesFullContentWithAssets", () => { ]); }); }); + +describe("reportSharedNodeAssets", () => { + it("keeps the download token out of the failure event", () => { + reportSharedNodeAssets({ + results: [ + { + status: "failed", + sourceLocalId: NODE_UID, + sourceRef: IMAGE, + error: `Could not fetch asset (HTTP 500): ${IMAGE}`, + }, + ], + retried: new Set([NODE_UID]), + }); + + const imagePath = IMAGE.split("?")[0]; + expect(capture).toHaveBeenCalledWith("Sync shared node asset failed", { + sourceLocalId: NODE_UID, + sourceRef: imagePath, + error: `Could not fetch asset (HTTP 500): ${imagePath}`, + retryScheduled: true, + }); + }); +}); diff --git a/apps/roam/src/utils/canonicalRoamUrl.ts b/apps/roam/src/utils/canonicalRoamUrl.ts index bb73c860c7..07f4f6e63e 100644 --- a/apps/roam/src/utils/canonicalRoamUrl.ts +++ b/apps/roam/src/utils/canonicalRoamUrl.ts @@ -1,4 +1,4 @@ -const ROAM_URL_PREFIX = "https://roamresearch.com/#/app/"; +export const ROAM_URL_PREFIX = "https://roamresearch.com/#/app/"; const canonicalRoamUrl = (graphName = window.roamAlphaAPI.graph.name) => ROAM_URL_PREFIX + graphName; export default canonicalRoamUrl; diff --git a/apps/roam/src/utils/conceptConversion.ts b/apps/roam/src/utils/conceptConversion.ts index 3792ce0d7d..08a277c15b 100644 --- a/apps/roam/src/utils/conceptConversion.ts +++ b/apps/roam/src/utils/conceptConversion.ts @@ -1,4 +1,3 @@ -import { InputTextNode } from "roamjs-components/types"; import getBlockProps from "./getBlockProps"; import { DiscourseNode } from "./getDiscourseNodes"; import { @@ -8,6 +7,7 @@ import { sourceIdOfNode, } from "./sourceSlot"; import extractContentFromTitle from "./extractContentFromTitle"; +import { nodeTemplateContent } from "./nodeTemplateContent"; import getDiscourseRelations from "./getDiscourseRelations"; import type { DiscourseRelation } from "./getDiscourseRelations"; import type { SupabaseContext } from "~/utils/supabaseContext"; @@ -65,33 +65,17 @@ const getNodeExtraData = ( /* eslint-enable @typescript-eslint/naming-convention */ }; -const indent = (s: string): string => - s - .split("\n") - .map((l) => " " + l) - .join("\n") + "\n"; - -const templateToText = (template: InputTextNode[]): string => - template - .filter((itn) => !itn.text.startsWith("{{")) - .map( - (itn) => - `* ${itn.text}\n${itn.children?.length ? indent(templateToText(itn.children)) : ""}`, - ) - .join(""); - export const discourseNodeSchemaToLocalConcept = ( context: SupabaseContext, node: DiscourseNode, ): LocalConceptDataInput => { - const titleParts = node.text.split("/"); - const label = titleParts[titleParts.length - 1] ?? node.text; const literalContent: Record = { - label, + label: node.text, format: node.format, }; - if (node.template !== undefined) - literalContent.template = templateToText(node.template); + const templateContent = nodeTemplateContent(node.template); + if (templateContent !== undefined) + literalContent.template_content = templateContent; const hasSourceSlot = schemaHasSourceSlot(node); if (hasSourceSlot) literalContent.roles = [SOURCE_SLOT]; return { diff --git a/apps/roam/src/utils/deleteRelationSchema.ts b/apps/roam/src/utils/deleteRelationSchema.ts new file mode 100644 index 0000000000..c22e8dac4c --- /dev/null +++ b/apps/roam/src/utils/deleteRelationSchema.ts @@ -0,0 +1,18 @@ +import deleteBlock from "roamjs-components/writes/deleteBlock"; +import { + getGlobalSettings, + setGlobalSetting, +} from "~/components/settings/utils/accessors"; +import { GLOBAL_KEYS } from "~/components/settings/utils/settingKeys"; +import refreshConfigTree from "./refreshConfigTree"; +import { markRelationSchemaDeleted } from "./relationSchemaChanges"; + +export const deleteRelationSchema = async (uid: string): Promise => { + await deleteBlock(uid); + markRelationSchemaDeleted(uid); + const remaining = { ...getGlobalSettings().Relations }; + delete remaining[uid]; + setGlobalSetting([GLOBAL_KEYS.relations], remaining); + await new Promise((resolve) => setTimeout(resolve, 50)); + refreshConfigTree(); +}; diff --git a/apps/roam/src/utils/importSharedNodes.ts b/apps/roam/src/utils/importSharedNodes.ts index f51db3c5ae..dbb8cb8cfb 100644 --- a/apps/roam/src/utils/importSharedNodes.ts +++ b/apps/roam/src/utils/importSharedNodes.ts @@ -16,7 +16,12 @@ export type FailedSharedNodeImport = { }; export type SharedNodeImportItem = - | { sharedNode: SharedNode; status: "imported" | "skipped"; warning?: string } + | { + sharedNode: SharedNode; + status: "imported" | "skipped"; + pageUid: string; + warning?: string; + } | FailedSharedNodeImport; export const isFailedSharedNodeImport = ( @@ -100,6 +105,7 @@ export const importSharedNodes = async ({ ? { sharedNode, status: result.action === "skipped" ? "skipped" : "imported", + pageUid: result.pageUid, ...(result.warning ? { warning: result.warning } : {}), } : { sharedNode, status: "failed", message: result.error.message }, diff --git a/apps/roam/src/utils/importSharedRelations.ts b/apps/roam/src/utils/importSharedRelations.ts index 70732bfc43..5b0262f0aa 100644 --- a/apps/roam/src/utils/importSharedRelations.ts +++ b/apps/roam/src/utils/importSharedRelations.ts @@ -27,6 +27,7 @@ import { import { discoverSharedRelations } from "./discoverSharedRelations"; import { DGSupabaseClient } from "@repo/database/lib/client"; import { deleteBlock } from "roamjs-components/writes"; +import refreshConfigTree from "./refreshConfigTree"; const matchImportedNodeSchemas = async ( nodeSchemas: CrossAppNodeSchema[], @@ -254,4 +255,6 @@ export const importSharedRelations = async ( ); ridToLocalId = { ...ridToLocalId, ...relationSchemaMap }; await importRelations(ridToLocalId, relations); + // Legacy settings read the cached grammar, including newly imported schemas. + refreshConfigTree(); }; diff --git a/apps/roam/src/utils/importedSourceIdentity.ts b/apps/roam/src/utils/importedSourceIdentity.ts index abff26d962..77774efb12 100644 --- a/apps/roam/src/utils/importedSourceIdentity.ts +++ b/apps/roam/src/utils/importedSourceIdentity.ts @@ -27,7 +27,7 @@ export const parseSourceIdentity = ( return { sourceModifiedAt, sourceNodeRid }; }; -const parseImportedSourceIdentity = ( +export const parseImportedSourceIdentity = ( props: Record, ): ImportedSourceIdentity | undefined => { const discourseGraphProps = props[DISCOURSE_GRAPH_PROP_NAME]; diff --git a/apps/roam/src/utils/nodeTemplateContent.ts b/apps/roam/src/utils/nodeTemplateContent.ts new file mode 100644 index 0000000000..b1782d286e --- /dev/null +++ b/apps/roam/src/utils/nodeTemplateContent.ts @@ -0,0 +1,24 @@ +import type { InputTextNode } from "roamjs-components/types"; + +const indent = (s: string): string => + s + .split("\n") + .map((l) => " " + l) + .join("\n") + "\n"; + +const templateToText = (template: InputTextNode[]): string => + template + .filter((itn) => !itn.text.startsWith("{{")) + .map( + (itn) => + `* ${itn.text}\n${itn.children?.length ? indent(templateToText(itn.children)) : ""}`, + ) + .join(""); + +// Shared by sync and publish so both write, or both omit, the same value. +export const nodeTemplateContent = ( + template: InputTextNode[] | undefined, +): string | undefined => { + const text = templateToText(template ?? []); + return text.length > 0 ? text : undefined; +}; diff --git a/apps/roam/src/utils/publishNodesToGroups.ts b/apps/roam/src/utils/publishNodesToGroups.ts index f7936572fb..70794d576c 100644 --- a/apps/roam/src/utils/publishNodesToGroups.ts +++ b/apps/roam/src/utils/publishNodesToGroups.ts @@ -32,6 +32,7 @@ import { SOURCE_SLOT } from "./sourceSlot"; import renderToast from "roamjs-components/components/Toast"; import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageUid"; import { publishNodeAssets, type NodeAssetResult } from "./publishNodeAssets"; +import { excludeProvisionalRelationSchemas } from "./relationSchemaAcceptance"; export type NodeUidWithType = { uid: string; @@ -128,7 +129,11 @@ export const gatherCorrespondingRelations = async ({ relationTripleSchemas: CrossAppRelationTripleSchema[]; relevantRelationIdsPerGroupId: Record; }> => { - const allRelationsSchemas = getDiscourseRelations(); + // Excluding provisional schemas here also drops their relation instances: + // relationSchemaIds below only keeps instances whose schema is in this map. + const allRelationsSchemas = excludeProvisionalRelationSchemas( + getDiscourseRelations(), + ); const allRelationSchemasById = Object.fromEntries( allRelationsSchemas.map((s) => [s.id, s]), ); diff --git a/apps/roam/src/utils/registerCommandPaletteCommands.ts b/apps/roam/src/utils/registerCommandPaletteCommands.ts index 17b215fb39..dd905aafc3 100644 --- a/apps/roam/src/utils/registerCommandPaletteCommands.ts +++ b/apps/roam/src/utils/registerCommandPaletteCommands.ts @@ -486,7 +486,7 @@ export const registerCommandPaletteCommands = (onloadArgs: OnloadArgs) => { void addCommand("DG: Export - Discourse graph", exportDiscourseGraph); void addCommand("DG: Open - Discourse settings", renderSettingsPopup); if (isSyncEnabled()) { - void addCommand("DG: Discover shared nodes", discoverSharedNodes); + void addCommand("DG: Import shared nodes", discoverSharedNodes); } if (isNodeSharingEnabled()) { void addCommand( diff --git a/apps/roam/src/utils/relationSchemaAcceptance.ts b/apps/roam/src/utils/relationSchemaAcceptance.ts new file mode 100644 index 0000000000..983684d5bd --- /dev/null +++ b/apps/roam/src/utils/relationSchemaAcceptance.ts @@ -0,0 +1,67 @@ +import { + isRelationSchemaDeleted, + notifyRelationSchemaChange, +} from "./relationSchemaChanges"; +import { DISCOURSE_GRAPH_PROP_NAME } from "./createReifiedBlock"; +import getBlockProps, { isJsonObject } from "./getBlockProps"; +import { setBlockPropsAsync } from "./setBlockProps"; +import { + parseImportedSourceIdentity, + type ImportedSourceIdentity, +} from "./importedSourceIdentity"; + +type ImportStatus = "provisional" | "accepted"; + +export const RELATION_SCHEMA_STATUS_PROP_KEY = "status"; +const ACCEPTED_STATUS: ImportStatus = "accepted"; + +export type RelationSchemaImportMeta = { + importedFrom: ImportedSourceIdentity; + status: ImportStatus; +}; + +// Origin (importedFrom) and acceptance (status) are stored as separate props so +// accepting never erases provenance; a schema with origin but no accepted +// status is provisional, which also covers schemas imported before acceptance +// existed. +export const readRelationSchemaImportMeta = ( + relationSchemaUid: string, +): RelationSchemaImportMeta | undefined => { + const props = getBlockProps(relationSchemaUid); + const importedFrom = parseImportedSourceIdentity(props); + if (importedFrom === undefined) return undefined; + const discourseGraphProps = props[DISCOURSE_GRAPH_PROP_NAME]; + const status = + isJsonObject(discourseGraphProps) && + discourseGraphProps[RELATION_SCHEMA_STATUS_PROP_KEY] === ACCEPTED_STATUS + ? ACCEPTED_STATUS + : "provisional"; + return { importedFrom, status }; +}; + +export const isProvisionalRelationSchema = ( + relationSchemaUid: string, +): boolean => + readRelationSchemaImportMeta(relationSchemaUid)?.status === "provisional"; + +export const isAcceptedRelationSchema = (relation: { id: string }): boolean => + !isRelationSchemaDeleted(relation.id) && + !isProvisionalRelationSchema(relation.id); + +export const excludeProvisionalRelationSchemas = ( + relations: T[], +): T[] => relations.filter(isAcceptedRelationSchema); + +export const acceptImportedRelationSchema = async ( + relationSchemaUid: string, +): Promise => { + const existing = getBlockProps(relationSchemaUid)[DISCOURSE_GRAPH_PROP_NAME]; + const discourseGraphProps = isJsonObject(existing) ? existing : {}; + await setBlockPropsAsync(relationSchemaUid, { + [DISCOURSE_GRAPH_PROP_NAME]: { + ...discourseGraphProps, + [RELATION_SCHEMA_STATUS_PROP_KEY]: ACCEPTED_STATUS, + }, + }); + notifyRelationSchemaChange(); +}; diff --git a/apps/roam/src/utils/relationSchemaChanges.ts b/apps/roam/src/utils/relationSchemaChanges.ts new file mode 100644 index 0000000000..da84c786fb --- /dev/null +++ b/apps/roam/src/utils/relationSchemaChanges.ts @@ -0,0 +1,39 @@ +import { useEffect, useState } from "react"; + +let revision = 0; +const listeners = new Set<() => void>(); +const deletedSchemaIds = new Set(); + +export const subscribeToRelationSchemaChanges = ( + listener: () => void, +): (() => void) => { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +}; + +export const notifyRelationSchemaChange = (): void => { + revision += 1; + listeners.forEach((listener) => listener()); +}; + +export const markRelationSchemaDeleted = (uid: string): void => { + // Mounted canvases retain schema metadata to render their existing shapes. + deletedSchemaIds.add(uid); + notifyRelationSchemaChange(); +}; + +export const isRelationSchemaDeleted = (uid: string): boolean => + deletedSchemaIds.has(uid); + +export const useRelationSchemaRevision = (): number => { + const [currentRevision, setCurrentRevision] = useState(revision); + useEffect(() => { + const update = (): void => setCurrentRevision(revision); + const unsubscribe = subscribeToRelationSchemaChanges(update); + update(); + return unsubscribe; + }, []); + return currentRevision; +}; diff --git a/apps/roam/src/utils/requestAssetRetry.ts b/apps/roam/src/utils/requestAssetRetry.ts new file mode 100644 index 0000000000..d0ae61548e --- /dev/null +++ b/apps/roam/src/utils/requestAssetRetry.ts @@ -0,0 +1,96 @@ +import { DISCOURSE_GRAPH_PROP_NAME } from "./createReifiedBlock"; +import getBlockProps, { isJsonObject } from "./getBlockProps"; +import internalError from "./internalError"; +import type { NodeAssetResult } from "./publishNodeAssets"; +import { setBlockPropsAsync } from "./setBlockProps"; + +/** + * Makes the next sync re-upload a node whose asset copy failed. Any block write moves the + * page's `:page/edit-time`, which the sync compares against the start of its last + * successful run. Retries are unbounded. + * + * A props write, not a string rewrite: rewriting the string reassigns `:edit/user` to + * whoever runs the sync. That a props write leaves it alone is undocumented. + */ + +/** + * Written, never read, and never cleared: clearing it would re-upload the node once more. + * Whether an asset reached storage is answered by its `FileReference` row. + */ +export const NEEDS_REFRESH_PROP_KEY = "needsRefresh"; + +/** + * An asset that reaches the markdown through a block ref or an embed lives on another + * page, where a write would not move this node's edit time. Those are not retried. + */ +const findBlocksReferencingAsset = async ({ + pageUid, + assetUrl, +}: { + pageUid: string; + assetUrl: string; +}): Promise => + (await window.roamAlphaAPI.data.async.q( + `[:find [?uid ...] + :in $ ?page-uid ?url + :where + [?page :block/uid ?page-uid] + [?block :block/page ?page] + [?block :block/string ?text] + [(clojure.string/includes? ?text ?url)] + [?block :block/uid ?uid]]`, + pageUid, + assetUrl, + )) as unknown as string[]; + +/** `discourse-graph` may already hold other data, so the flag is merged into it. */ +const markBlockForAssetRetry = async (blockUid: string): Promise => { + const existing = getBlockProps(blockUid)[DISCOURSE_GRAPH_PROP_NAME]; + const discourseGraphProps = isJsonObject(existing) ? existing : {}; + await setBlockPropsAsync(blockUid, { + [DISCOURSE_GRAPH_PROP_NAME]: { + ...discourseGraphProps, + [NEEDS_REFRESH_PROP_KEY]: true, + }, + }); +}; + +/** + * One write per node is enough: the re-upload retries every asset of the node, including + * those with no block on the page. Returns the nodes marked. + */ +export const requestAssetRetries = async ( + results: NodeAssetResult[], +): Promise> => { + const failedByNode = new Map(); + for (const result of results) { + if (result.status !== "failed") continue; + const assetUrls = failedByNode.get(result.sourceLocalId) ?? []; + assetUrls.push(result.sourceRef); + failedByNode.set(result.sourceLocalId, assetUrls); + } + + const retried = new Set(); + for (const [pageUid, assetUrls] of failedByNode) { + try { + for (const assetUrl of assetUrls) { + const [blockUid] = await findBlocksReferencingAsset({ + pageUid, + assetUrl, + }); + if (blockUid === undefined) continue; + await markBlockForAssetRetry(blockUid); + retried.add(pageUid); + break; + } + } catch (error) { + internalError({ + error, + type: "Asset retry failed", + context: { sourceLocalId: pageUid }, + sendEmail: false, + }); + } + } + return retried; +}; diff --git a/apps/roam/src/utils/roamToCrossAppConverters.ts b/apps/roam/src/utils/roamToCrossAppConverters.ts index 5d91074690..8b076b40cb 100644 --- a/apps/roam/src/utils/roamToCrossAppConverters.ts +++ b/apps/roam/src/utils/roamToCrossAppConverters.ts @@ -17,6 +17,7 @@ import getPageViewType from "roamjs-components/queries/getPageViewType"; import { contentTypes } from "@repo/content-model"; import getDiscourseNodes from "./getDiscourseNodes"; import extractContentFromTitle from "./extractContentFromTitle"; +import { nodeTemplateContent } from "./nodeTemplateContent"; import { SOURCE_SLOT, schemaHasSourceSlot, @@ -225,6 +226,7 @@ export const nodeSchemaToCrossApp = ( createdAt: new Date(createdTime), modifiedAt: new Date(Math.max(pageEditTime, createdTime)), format: s.format, + template: nodeTemplateContent(s.template), ...(hasSourceSlot ? { slotDefinitions: { [SOURCE_SLOT]: sourceSlotSchemaId() } } : {}), diff --git a/apps/roam/src/utils/sortSharedNodes.ts b/apps/roam/src/utils/sortSharedNodes.ts new file mode 100644 index 0000000000..5c458f45d6 --- /dev/null +++ b/apps/roam/src/utils/sortSharedNodes.ts @@ -0,0 +1,54 @@ +import type { SharedNode } from "@repo/database/lib/sharedNodes"; + +export type SharedNodeSortColumn = "spaceName" | "title" | "lastModified"; + +export type SharedNodeSort = { + column: SharedNodeSortColumn; + direction: "ascending" | "descending"; +}; + +type SortableSharedNode = Pick; + +export const DEFAULT_SHARED_NODE_SORT: SharedNodeSort = { + column: "lastModified", + direction: "descending", +}; + +export const getNextSharedNodeSort = ({ + currentSort, + column, +}: { + currentSort: SharedNodeSort; + column: SharedNodeSortColumn; +}): SharedNodeSort => ({ + column, + direction: + currentSort.column === column && currentSort.direction === "descending" + ? "ascending" + : "descending", +}); + +const compareByColumn = + (column: SharedNodeSortColumn) => + (left: SortableSharedNode, right: SortableSharedNode): number => + column === "lastModified" + ? Date.parse(left.lastModified) - Date.parse(right.lastModified) + : left[column].localeCompare(right[column], undefined, { + numeric: true, + sensitivity: "base", + }); + +export const sortSharedNodes = ({ + nodes, + sort, +}: { + nodes: readonly T[]; + sort: SharedNodeSort; +}): T[] => { + const compare = compareByColumn(sort.column); + return [...nodes].sort((left, right) => + sort.direction === "ascending" + ? compare(left, right) + : compare(right, left), + ); +}; diff --git a/apps/roam/src/utils/syncDgNodesToSupabase.ts b/apps/roam/src/utils/syncDgNodesToSupabase.ts index 843d74a725..6bbc28bd9b 100644 --- a/apps/roam/src/utils/syncDgNodesToSupabase.ts +++ b/apps/roam/src/utils/syncDgNodesToSupabase.ts @@ -29,6 +29,7 @@ import { summarizeAssetResults, type NodeAssetResult, } from "./publishNodeAssets"; +import { requestAssetRetries } from "./requestAssetRetry"; import type { DGSupabaseClient } from "@repo/database/lib/client"; import { intersection } from "@repo/utils/setOperations"; import { CORE_TITLE_PROBE_SELECT } from "@repo/database/lib/coreTitleBackfill"; @@ -930,10 +931,23 @@ const reportCoreTitleBackfill = ({ }; /** - * A failed copy is not retried until the node changes again, so the counts are the only - * standing signal that a shared node's asset never reached storage. + * A Roam asset URL's query string carries a download token that grants access to the + * file, so it must not reach analytics. The path still identifies the asset. */ -const reportSharedNodeAssets = (results: NodeAssetResult[]): void => { +const withoutUrlQueries = (text: string): string => + text.replace(/(https?:\/\/[^\s?]+)\?[^\s)]*/g, "$1"); + +/** + * Retries are unbounded, so the per-asset events are how a node that keeps failing is + * found. + */ +export const reportSharedNodeAssets = ({ + results, + retried, +}: { + results: NodeAssetResult[]; + retried: ReadonlySet; +}): void => { if (results.length === 0) return; const { copied, unchanged, distinctBlobs, tooLarge, failed } = summarizeAssetResults(results); @@ -944,6 +958,16 @@ const reportSharedNodeAssets = (results: NodeAssetResult[]): void => { tooLarge: tooLarge.length, failed: failed.length, }); + // Per node, unlike the summary: the same file failing in two nodes is two retries. + for (const result of results) { + if (result.status !== "failed") continue; + posthog.capture("Sync shared node asset failed", { + sourceLocalId: result.sourceLocalId, + sourceRef: withoutUrlQueries(result.sourceRef), + error: withoutUrlQueries(result.error), + retryScheduled: retried.has(result.sourceLocalId), + }); + } if (failed.length > 0) { console.warn( `Sync could not copy ${failed.length} shared node assets`, @@ -1493,14 +1517,18 @@ export const createOrUpdateDiscourseEmbedding = async ( activeContext, ), }); - reportSharedNodeAssets( - await upsertSharedNodesFullContentWithAssets({ + const sharedNodeAssetResults = await upsertSharedNodesFullContentWithAssets( + { nodes: sharedFullContentNodes, supabaseClient: activeSupabaseClient, context: activeContext, phases, - }), + }, ); + reportSharedNodeAssets({ + results: sharedNodeAssetResults, + retried: await requestAssetRetries(sharedNodeAssetResults), + }); await measureSyncPhase({ phase: "convertConcepts", phases, diff --git a/packages/database/src/lib/__tests__/crossAppConverters.test.ts b/packages/database/src/lib/__tests__/crossAppConverters.test.ts index c5f1b52c42..33be178f3b 100644 --- a/packages/database/src/lib/__tests__/crossAppConverters.test.ts +++ b/packages/database/src/lib/__tests__/crossAppConverters.test.ts @@ -30,6 +30,7 @@ describe("crossAppNodeSchemaToDbConcept", () => { format: "[[CLM]] - {content}", }); expect(concept.literal_content).toEqual({ + label: "Some concept", format: "[[CLM]] - {content}", }); }); @@ -42,15 +43,17 @@ describe("crossAppNodeSchemaToDbConcept", () => { templateTitle: "Claim template", }); expect(concept.literal_content).toEqual({ + label: "Some concept", format: "[[CLM]] - {content}", template: "Claim template", template_content: "* Evidence\n", }); }); - it("omits literal_content when no keys are set", () => { + it("writes the label into literal_content as well as the name", () => { const concept = crossAppNodeSchemaToDbConcept(baseSchema); - expect(concept.literal_content).toBeUndefined(); + expect(concept.name).toBe("Some concept"); + expect(concept.literal_content).toEqual({ label: "Some concept" }); }); it("stores slot definitions as roles plus local reference content", () => { @@ -60,6 +63,7 @@ describe("crossAppNodeSchemaToDbConcept", () => { slotDefinitions: { evidence: "evidence-type", claim: "claim-type" }, }); expect(result.literal_content).toEqual({ + label: "Some concept", template: "Template Title", roles: ["evidence", "claim"], }); @@ -74,7 +78,7 @@ describe("crossAppNodeSchemaToDbConcept", () => { ...baseSchema, slotDefinitions: {}, }); - expect(result).not.toHaveProperty("literal_content"); + expect(result.literal_content).toEqual({ label: "Some concept" }); expect(result).not.toHaveProperty("local_reference_content"); }); }); diff --git a/packages/database/src/lib/crossAppConverters.ts b/packages/database/src/lib/crossAppConverters.ts index f6c71a8229..471e0aa555 100644 --- a/packages/database/src/lib/crossAppConverters.ts +++ b/packages/database/src/lib/crossAppConverters.ts @@ -98,6 +98,7 @@ export const crossAppNodeSchemaToDbConcept = ( ): LocalConceptDataInput => { const slots = Object.keys(node.slotDefinitions ?? {}); const literalInfo = filterUndefined({ + label: node.label, template: node.templateTitle, template_content: node.template, format: node.format,