Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 12 additions & 2 deletions apps/roam/src/components/DiscourseContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import getDiscourseContextResults from "~/utils/getDiscourseContextResults";
import ResultsView from "./results-view/ResultsView";
import posthog from "posthog-js";
import { CreateRelationButton } from "./CreateRelationDialog";
import TentativeRelationInstances from "./TentativeRelationInstances";
import { useDiscourseContextMutationRefresh } from "~/utils/discourseContextMutationRefresh";

export type DiscourseContextResults = Awaited<
Expand Down Expand Up @@ -172,7 +173,10 @@ export const ContextContent = ({ uid, results, overlayRefresh }: Props) => {
});
const [tabId, setTabId] = useState(0);
const [groupByTarget, setGroupByTarget] = useState(false);
return queryResults.length ? (
const [tentativeCount, setTentativeCount] = useState<number | undefined>(
undefined,
);
const body = queryResults.length ? (
<>
<style>{`@media (hover: hover) and (pointer: fine) {
.roamjs-discourse-result-panel .roamjs-query-results-delete-relation {
Expand Down Expand Up @@ -249,10 +253,16 @@ export const ContextContent = ({ uid, results, overlayRefresh }: Props) => {
</Tabs>
) : (
<div className="flex flex-col items-start">
<span>No discourse relations found.</span>
{tentativeCount === 0 && <span>No discourse relations found.</span>}
<CreateRelationButton sourceNodeUid={uid} onCreated={delayedRefresh} />
</div>
);
return (
<>
{body}
<TentativeRelationInstances uid={uid} onCountChange={setTentativeCount} />
</>
);
};

const DiscourseContext = ({ uid }: Props) => {
Expand Down
212 changes: 212 additions & 0 deletions apps/roam/src/components/TentativeRelationInstances.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
import React, { useCallback, useEffect, useState } from "react";
import { Button, Classes, Tag } from "@blueprintjs/core";
import { render as renderToast } from "roamjs-components/components/Toast";
import deleteBlock from "roamjs-components/writes/deleteBlock";
import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageUid";
import posthog from "posthog-js";
import { isRid, ridToSpaceUriAndLocalId } from "@repo/database/lib/rid";
import getDiscourseRelations from "~/utils/getDiscourseRelations";
import internalError from "~/utils/internalError";
import { getErrorMessage } from "~/utils/materializeSharedNode";
import { getStoredRelationsEnabled } from "~/utils/storedRelations";
import {
refreshDiscourseContextsForMutatedUids,
useDiscourseContextMutationRefresh,
} from "~/utils/discourseContextMutationRefresh";
import { acceptTentativeRelationInstance } from "~/utils/createReifiedBlock";
import {
getTentativeRelationInstances,
type TentativeRelationInstance,
} from "~/utils/tentativeRelations";
import type { ImportedSourceIdentity } from "~/utils/importedSourceIdentity";

type TentativeRelationRow = TentativeRelationInstance & {
label: string;
otherText: string;
provenance: string;
};

const buildProvenance = (importedFrom?: ImportedSourceIdentity): string => {
if (!importedFrom || !isRid(importedFrom.sourceNodeRid)) return "";
const { spaceUri, sourceLocalId } = ridToSpaceUriAndLocalId(
importedFrom.sourceNodeRid,
);
const modifiedAt = new Date(importedFrom.sourceModifiedAt);
const modified = Number.isNaN(modifiedAt.getTime())
? undefined
: modifiedAt.toLocaleString();
return `from ${[spaceUri, sourceLocalId, modified].filter(Boolean).join(" · ")}`;
};

const TentativeRelationInstances = ({
uid,
onCountChange,
}: {
uid: string;
onCountChange: (count: number | undefined) => void;
}): React.JSX.Element | null => {
const [rows, setRows] = useState<TentativeRelationRow[]>([]);
const [pending, setPending] = useState<{
uid: string;
action: "accept" | "remove";
} | null>(null);

const loadRows = useCallback(async () => {
if (!getStoredRelationsEnabled()) {
onCountChange(0);
return;
}
onCountChange(undefined);
try {
const instances = await getTentativeRelationInstances();
const relevant = instances.filter(
(instance) =>
instance.sourceUid === uid || instance.destinationUid === uid,
);
const relationById = new Map(
getDiscourseRelations().map((r) => [r.id, r]),
);
const nextRows = relevant.map((instance) => {
const isOutgoing = instance.sourceUid === uid;
const otherUid = isOutgoing
? instance.destinationUid
: instance.sourceUid;
const schema = relationById.get(instance.schemaUid);
const label =
(isOutgoing ? schema?.label : schema?.complement || schema?.label) ||
"Unknown relation";
return {
...instance,
label,
otherText: getPageTitleByPageUid(otherUid) || otherUid,
provenance: buildProvenance(instance.importedFrom),
};
});
setRows(nextRows);
onCountChange(nextRows.length);
} catch (error) {
internalError({
error,
type: "Load Tentative Relations Failed",
context: { uid },
userMessage:
"Could not load imported relations. Refresh and try again.",
sendEmail: false,
});
}
}, [uid, onCountChange]);

useEffect(() => {
void loadRows();
}, [loadRows]);
Comment on lines +54 to +101

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.

Unhandled promise rejections in async operations. The loadRows function is called with void (lines 83, 86), which prevents error handling. If getTentativeRelationInstances() or any other async operation fails, it will cause an unhandled promise rejection that could crash the application or leave the UI in a broken state.

const loadRows = useCallback(async () => {
  if (!getStoredRelationsEnabled()) return;
  try {
    const instances = await getTentativeRelationInstances();
    // ... rest of the logic
  } catch (error) {
    internalError({
      error,
      type: "Load Tentative Relations Failed",
      context: { uid },
      sendEmail: false,
    });
    setRows([]);
    onCountChange(0);
  }
}, [uid, onCountChange]);
Suggested change
const loadRows = useCallback(async () => {
if (!getStoredRelationsEnabled()) return;
const instances = await getTentativeRelationInstances();
const relevant = instances.filter(
(instance) =>
instance.sourceUid === uid || instance.destinationUid === uid,
);
const relationById = new Map(getDiscourseRelations().map((r) => [r.id, r]));
const nextRows = relevant.map((instance) => {
const isOutgoing = instance.sourceUid === uid;
const otherUid = isOutgoing
? instance.destinationUid
: instance.sourceUid;
const schema = relationById.get(instance.schemaUid);
const label =
(isOutgoing ? schema?.label : schema?.complement || schema?.label) ||
"Unknown relation";
return {
...instance,
label,
otherText: getPageTitleByPageUid(otherUid) || otherUid,
provenance: buildProvenance(instance.importedFrom),
};
});
setRows(nextRows);
onCountChange(nextRows.length);
}, [uid, onCountChange]);
useEffect(() => {
void loadRows();
}, [loadRows]);
const loadRows = useCallback(async () => {
if (!getStoredRelationsEnabled()) return;
try {
const instances = await getTentativeRelationInstances();
const relevant = instances.filter(
(instance) =>
instance.sourceUid === uid || instance.destinationUid === uid,
);
const relationById = new Map(getDiscourseRelations().map((r) => [r.id, r]));
const nextRows = relevant.map((instance) => {
const isOutgoing = instance.sourceUid === uid;
const otherUid = isOutgoing
? instance.destinationUid
: instance.sourceUid;
const schema = relationById.get(instance.schemaUid);
const label =
(isOutgoing ? schema?.label : schema?.complement || schema?.label) ||
"Unknown relation";
return {
...instance,
label,
otherText: getPageTitleByPageUid(otherUid) || otherUid,
provenance: buildProvenance(instance.importedFrom),
};
});
setRows(nextRows);
onCountChange(nextRows.length);
} catch (error) {
internalError({
error,
type: "Load Tentative Relations Failed",
context: { uid },
sendEmail: false,
});
setRows([]);
onCountChange(0);
}
}, [uid, onCountChange]);
useEffect(() => {
void loadRows();
}, [loadRows]);

Spotted by Graphite

Fix in Graphite


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


const onMutationRefresh = useCallback(() => void loadRows(), [loadRows]);
useDiscourseContextMutationRefresh({ uid, onMutationRefresh });

const onAccept = async (row: TentativeRelationRow): Promise<void> => {
posthog.capture("Discourse Context: Accept Tentative Relation Triggered", {
instanceUid: row.instanceUid,
uid,
});
setPending({ uid: row.instanceUid, action: "accept" });
try {
await acceptTentativeRelationInstance({ instanceUid: row.instanceUid });
renderToast({
id: "accept-relation-success",
content: "Relation accepted",
intent: "success",
});
refreshDiscourseContextsForMutatedUids({
uids: [row.sourceUid, row.destinationUid],
});
} catch (error) {
internalError({
error,
type: "Accept Tentative Relation Failed",
context: { instanceUid: row.instanceUid },
userMessage: `Could not accept relation: ${getErrorMessage(error)}`,
sendEmail: false,
});
} finally {
setPending(null);
}
};

const onRemove = async (row: TentativeRelationRow): Promise<void> => {
posthog.capture("Discourse Context: Remove Tentative Relation Triggered", {
instanceUid: row.instanceUid,
uid,
});
setPending({ uid: row.instanceUid, action: "remove" });
try {
await deleteBlock(row.instanceUid);
renderToast({
id: "remove-relation-success",
content: "Relation removed",
intent: "success",
});
refreshDiscourseContextsForMutatedUids({
uids: [row.sourceUid, row.destinationUid],
});
} catch (error) {
internalError({
error,
type: "Remove Tentative Relation Failed",
context: { instanceUid: row.instanceUid },
userMessage: `Could not remove relation: ${getErrorMessage(error)}`,
sendEmail: false,
});
} finally {
setPending(null);
}
};

if (!rows.length) return null;

return (
<div className="roamjs-discourse-tentative-relations mt-2 px-2">
<div className={`${Classes.TEXT_MUTED} text-xs font-semibold`}>
Imported relations pending review ({rows.length})
</div>
{rows.map((row) => (
<div key={row.instanceUid} className="flex items-center gap-2 py-1">
<div className="min-w-0 flex-1">
<div className="truncate" title={row.otherText}>
<Tag minimal>{row.label}</Tag> {row.otherText}
</div>
{row.provenance && (
<div
className={`${Classes.TEXT_MUTED} truncate text-xs`}
title={row.importedFrom?.sourceNodeRid}
>
{row.provenance}
</div>
)}
</div>
<Button
minimal
icon="tick"
title="Accept relation"
disabled={pending !== null}
loading={
pending?.uid === row.instanceUid && pending.action === "accept"
}
onClick={() => void onAccept(row)}
/>
<Button
minimal
icon="delete"
title="Remove relation"
disabled={pending !== null}
loading={
pending?.uid === row.instanceUid && pending.action === "remove"
}
onClick={() => void onRemove(row)}
/>
</div>
))}
</div>
);
};

export default TentativeRelationInstances;
3 changes: 2 additions & 1 deletion apps/roam/src/components/results-view/ResultsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,8 @@ const ResultRow = ({
hasSchema: r["id"],
} as Record<string, string>;
// types got checked as a condition for displaying the button
strictQueryForReifiedBlocks(data)
// The visible row is accepted; a pending twin must not be its delete target.
strictQueryForReifiedBlocks(data, { acceptedOnly: true })
.then((blockUid) => {
if (blockUid === null) {
renderToast({
Expand Down
Loading