Skip to content

[ENG-1859] Add Roam import action for selected shared nodes #1269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sid597 wants to merge 4 commits into from
Open

[ENG-1859] Add Roam import action for selected shared nodes #1269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cfdaad4
[ENG-1859] Add Roam import action for selected shared nodes
sid597 Jul 30, 2026
8f4a15c
[ENG-1859] Keep the landed discovered-node shape, add sharedNode addi…
sid597 Jul 30, 2026
c25151e
[ENG-1859] Send the numeric space id as a flat key instead of embeddi…
sid597 Jul 30, 2026
5dd063b
[ENG-1859] Use SharedNode directly and derive already-imported from t…
sid597 Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 225 additions & 31 deletions apps/roam/src/components/DiscoverSharedNodesDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
Button,
Callout,
Checkbox,
Classes,
Dialog,
HTMLTable,
Expand All @@ -13,34 +14,64 @@ import {
} from "@blueprintjs/core";
import React, { useCallback, useEffect, useMemo, useState } from "react";
import createOverlayRender from "roamjs-components/util/createOverlayRender";
import type { SharedNode } from "@repo/database/lib/sharedNodes";
import { discoverSharedNodes } from "~/utils/discoverSharedNodes";
import {
discoverSharedNodes,
type DiscoveredSharedNode,
} from "~/utils/discoverSharedNodes";
importSharedNodes,
isFailedSharedNodeImport,
type SharedNodeImportItem,
} from "~/utils/importSharedNodes";
import internalError from "~/utils/internalError";
import { getLoggedInClient, getSupabaseContext } from "~/utils/supabaseContext";

const IMPORT_ERROR_TYPE = "Shared node import failed";
const IMPORT_ERROR_OPERATION = "import-shared-nodes";

const formatModifiedAt = (modifiedAt: string): string =>
new Date(modifiedAt).toLocaleString();

const SharedNodeRow = ({ node }: { node: DiscoveredSharedNode }) => (
const isImportableSharedNode = (node: SharedNode): boolean =>

@sid597 sid597 Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obsidian-only by MVP scope: Obsidian is currently the only producer of importable full content, so its rows are the only ones with something to materialize. Roam-origin rows still show in the list (visibility of what's shared) but aren't selectable. When another producer lands, this predicate is the one place to widen.

node.platform === "Obsidian";

const SharedNodeRow = ({
node,
alreadyImported,
selected,
selectionDisabled,
onToggleSelected,
}: {
node: SharedNode;
alreadyImported: boolean;
selected: boolean;
selectionDisabled: boolean;
onToggleSelected: () => void;
}) => (
<tr>
<td>
<Tag minimal>{node.sourceApp}</Tag>
<Checkbox
aria-label={`Select ${node.title}`}
checked={selected}
className="m-0"
disabled={selectionDisabled || !isImportableSharedNode(node)}
onChange={onToggleSelected}
/>
</td>
<td>
<Tag minimal>{node.platform}</Tag>
</td>
<td>
<div className="max-w-52 font-medium [overflow-wrap:anywhere]">
{node.sourceSpaceName}
{node.spaceName}
</div>
<div
className={[
Classes.MONOSPACE_TEXT,
Classes.TEXT_MUTED,
"max-w-52 truncate text-xs",
].join(" ")}
title={node.sourceSpaceId}
title={node.spaceUri}
>
{node.sourceSpaceId}
{node.spaceUri}
</div>
</td>
<td>
Expand All @@ -49,24 +80,24 @@ const SharedNodeRow = ({ node }: { node: DiscoveredSharedNode }) => (
</div>
</td>
<td>
{node.sourceNodeId ? (
{node.sourceLocalId ? (
<div
className={[Classes.MONOSPACE_TEXT, "max-w-44 truncate text-xs"].join(
" ",
)}
title={node.sourceNodeRid}
title={node.rid}
>
{node.sourceNodeId}
{node.sourceLocalId}
</div>
) : (
<span className={Classes.TEXT_MUTED}>Not provided</span>
)}
</td>
<td className="whitespace-nowrap" title={node.modifiedAt}>
{formatModifiedAt(node.modifiedAt)}
<td className="whitespace-nowrap" title={node.lastModified}>
{formatModifiedAt(node.lastModified)}
</td>
<td>
{node.alreadyImported ? (
{alreadyImported ? (
<Tag intent={Intent.SUCCESS} minimal>
Imported
</Tag>
Expand All @@ -77,26 +108,72 @@ const SharedNodeRow = ({ node }: { node: DiscoveredSharedNode }) => (
</tr>
);

const ImportResultsSummary = ({
results,
}: {
results: SharedNodeImportItem[];
}) => {
const importedCount = results.filter(
(item) => item.status === "imported",
).length;
const skippedCount = results.filter(
(item) => item.status === "skipped",
).length;
const failedImports = results.filter(isFailedSharedNodeImport);
return (
<Callout
intent={failedImports.length > 0 ? Intent.WARNING : Intent.SUCCESS}
title={`${importedCount} imported, ${skippedCount} skipped, ${failedImports.length} failed`}
>
{skippedCount > 0 && (
<div>Skipped nodes were already up to date in this graph.</div>
)}
{failedImports.length > 0 && (
<ul className="mb-0 mt-2 list-disc pl-5">
{failedImports.map((item) => (
<li key={item.sharedNode.rid}>
<span className="font-medium">{item.sharedNode.title}</span>:{" "}
{item.message}
</li>
))}
</ul>
)}
</Callout>
);
};

const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
const [nodes, setNodes] = useState<DiscoveredSharedNode[]>([]);
const [nodes, setNodes] = useState<SharedNode[]>([]);
const [importedRids, setImportedRids] = useState<Set<string>>(new Set());

@sid597 sid597 Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single source of truth for imported-ness. Rows don't carry an alreadyImported flag anymore — the badge derives from importedRids.has(node.rid) at render, and a successful import just adds rids to this set (previously this meant walking the nodes array and patching a stored flag on each matching row).

const [loading, setLoading] = useState(true);
const [error, setError] = useState("");
const [searchTerm, setSearchTerm] = useState("");
const [selectedRids, setSelectedRids] = useState<Set<string>>(new Set());
const [importProgress, setImportProgress] = useState<{
current: number;
total: number;
} | null>(null);
const [importResults, setImportResults] = useState<
SharedNodeImportItem[] | null
>(null);
const importing = importProgress !== null;

const loadNodes = useCallback(async (): Promise<void> => {
setLoading(true);
setError("");
setSelectedRids(new Set());
setImportResults(null);
try {
const context = await getSupabaseContext();
if (!context) throw new Error("Could not connect to shared persistence.");
const client = await getLoggedInClient();
if (!client) throw new Error("Could not connect to shared persistence.");
setNodes(
await discoverSharedNodes({
client,
currentSpaceId: context.spaceId,
}),
);
const { sharedNodes, importedSourceRids } = await discoverSharedNodes({
client,
currentSpaceId: context.spaceId,
});
setNodes(sharedNodes);
setImportedRids(importedSourceRids);
} catch (loadError) {
internalError({
error: loadError,
Expand All @@ -123,21 +200,106 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
if (!normalizedSearch) return nodes;
return nodes.filter((node) =>
[
node.sourceApp,
node.sourceSpaceName,
node.sourceSpaceId,
node.platform,
node.spaceName,
node.spaceUri,
node.title,
node.sourceNodeId,
].some((value) => value?.toLocaleLowerCase().includes(normalizedSearch)),
node.sourceLocalId,
].some((value) => value.toLocaleLowerCase().includes(normalizedSearch)),

@sid597 sid597 Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No ?. here on purpose: all five searched fields are non-optional strings on SharedNode.

);
}, [nodes, searchTerm]);

const importableVisibleRids = visibleNodes
.filter(isImportableSharedNode)
.map((node) => node.rid);
const allVisibleSelected =
importableVisibleRids.length > 0 &&
importableVisibleRids.every((rid) => selectedRids.has(rid));
const someVisibleSelected = importableVisibleRids.some((rid) =>
selectedRids.has(rid),
);

const toggleNodeSelected = (rid: string): void => {
setSelectedRids((previous) => {
const next = new Set(previous);
if (next.has(rid)) next.delete(rid);
else next.add(rid);
return next;
});
};

const toggleAllVisibleSelected = (): void => {
setSelectedRids((previous) => {
const next = new Set(previous);
if (allVisibleSelected)
importableVisibleRids.forEach((rid) => next.delete(rid));
else importableVisibleRids.forEach((rid) => next.add(rid));
return next;
});
};
Comment on lines +231 to +239

@graphite-app graphite-app Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race condition: allVisibleSelected is a stale closure captured from render time, but the state updater function receives the current state. If the checkbox is toggled rapidly or state updates between renders, the toggle logic will use outdated allVisibleSelected values, causing incorrect selection behavior.

const toggleAllVisibleSelected = (): void => {
  setSelectedRids((previous) => {
    const next = new Set(previous);
    // Compute allSelected based on current state, not captured render value
    const allSelected = importableVisibleRids.length > 0 &&
      importableVisibleRids.every((rid) => previous.has(rid));
    if (allSelected)
      importableVisibleRids.forEach((rid) => next.delete(rid));
    else importableVisibleRids.forEach((rid) => next.add(rid));
    return next;
  });
};

The fix computes the selection state inside the updater function based on the actual previous state rather than the captured allVisibleSelected value.

Suggested change
const toggleAllVisibleSelected = (): void => {
setSelectedRids((previous) => {
const next = new Set(previous);
if (allVisibleSelected)
importableVisibleRids.forEach((rid) => next.delete(rid));
else importableVisibleRids.forEach((rid) => next.add(rid));
return next;
});
};
const toggleAllVisibleSelected = (): void => {
setSelectedRids((previous) => {
const next = new Set(previous);
const allSelected =
importableVisibleRids.length > 0 &&
importableVisibleRids.every((rid) => previous.has(rid));
if (allSelected)
importableVisibleRids.forEach((rid) => next.delete(rid));
else importableVisibleRids.forEach((rid) => next.add(rid));
return next;
});
};

Spotted by Graphite

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.


const importSelectedNodes = async (): Promise<void> => {
const selectedNodes = nodes.filter((node) => selectedRids.has(node.rid));

setImportResults(null);
setImportProgress({ current: 0, total: selectedNodes.length });
try {
const client = await getLoggedInClient();
if (!client) throw new Error("Could not connect to shared persistence.");
const results = await importSharedNodes({
client,
sharedNodes: selectedNodes,
onProgress: (current, total) => setImportProgress({ current, total }),
});
setImportResults(results);
const newlyImportedRids = results
.filter((item) => item.status !== "failed")
.map((item) => item.sharedNode.rid);
setImportedRids((previous) => {
const next = new Set(previous);
newlyImportedRids.forEach((rid) => next.add(rid));
return next;
});
const failedImports = results.filter(isFailedSharedNodeImport);
setSelectedRids(

@sid597 sid597 Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selection is not fully cleared after a run: failed nodes stay selected so a retry after fixing the reported problem (e.g. renaming a colliding page) is one click; successful ones are deselected.

new Set(failedImports.map((item) => item.sharedNode.rid)),
);
if (failedImports.length > 0) {
internalError({
error: new Error(
`${failedImports.length} of ${results.length} shared node imports failed`,
),
type: IMPORT_ERROR_TYPE,
context: {
operation: IMPORT_ERROR_OPERATION,
failureMessages: failedImports.map((item) => item.message),
},
sendEmail: false,
});
}
} catch (importError) {
internalError({
error: importError,
type: IMPORT_ERROR_TYPE,
context: { operation: IMPORT_ERROR_OPERATION },
sendEmail: false,
userMessage:
importError instanceof Error
? importError.message
: "Could not import the selected shared nodes.",
});
} finally {
setImportProgress(null);
}
};

return (
<Dialog
autoFocus={false}
canEscapeKeyClose
canOutsideClickClose
canEscapeKeyClose={!importing}

@sid597 sid597 Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All close paths (escape, outside click, X, Close button) are disabled while an import runs because the underlying Roam writes can't be cancelled — closing would just hide the progress and the failure report.

canOutsideClickClose={!importing}
enforceFocus={false}
isCloseButtonShown={!importing}
style={{ width: "min(68rem, calc(100vw - 2rem))" }}
isOpen
onClose={onClose}
Expand All @@ -161,14 +323,16 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
<Tooltip content="Reload shared nodes">
<Button
aria-label="Reload shared nodes"
disabled={loading}
disabled={loading || importing}
icon="refresh"
minimal
onClick={() => void loadNodes()}
/>
</Tooltip>
</div>

{importResults && <ImportResultsSummary results={importResults} />}

{loading ? (
<div className="flex min-h-52 items-center justify-center">
<Spinner />
Expand All @@ -194,6 +358,16 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
<HTMLTable striped className="w-full">
<thead>
<tr>
<th>
<Checkbox
aria-label="Select all importable nodes"
checked={allVisibleSelected}
className="m-0"
disabled={importing || importableVisibleRids.length === 0}
indeterminate={!allVisibleSelected && someVisibleSelected}
onChange={toggleAllVisibleSelected}
/>
</th>
<th>Source app</th>
<th>Source space</th>
<th>Title</th>
Expand All @@ -204,7 +378,14 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
</thead>
<tbody>
{visibleNodes.map((node) => (
<SharedNodeRow key={node.sourceNodeRid} node={node} />
<SharedNodeRow
key={node.rid}
node={node}
alreadyImported={importedRids.has(node.rid)}
onToggleSelected={() => toggleNodeSelected(node.rid)}
selected={selectedRids.has(node.rid)}
selectionDisabled={importing}
/>
))}
</tbody>
</HTMLTable>
Expand All @@ -218,7 +399,20 @@ const DiscoverSharedNodesDialog = ({ onClose }: { onClose: () => void }) => {
? ""
: `${visibleNodes.length} of ${nodes.length} nodes`}
</span>
<Button onClick={onClose}>Close</Button>
<div className="flex items-center gap-2">
<Button disabled={importing} onClick={onClose}>
Close
</Button>
<Button
disabled={importing || selectedRids.size === 0}
intent={Intent.PRIMARY}
onClick={() => void importSelectedNodes()}
>
{importProgress
? `Importing ${importProgress.current} of ${importProgress.total}…`
: `Import selected (${selectedRids.size})`}
</Button>
</div>
</div>
</div>
</Dialog>
Expand Down
Loading
Loading