ENG-2167 nodeSchemaToCrossApp is missing modifiedAt - #1335
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
e2b4f6b to
77fde60
Compare
| if (!relData) return null; | ||
| const userUid = (relData[":create/user"] ?? {})[":user/uid"]; | ||
| if (!userUid) return null; | ||
| const createdTime = relData[":create/time"] || Date.now(); |
There was a problem hiding this comment.
Inconsistent operator usage creates a bug with timestamp value 0. Line 199 uses || (OR operator) while lines 203-204 use ?? (nullish coalescing). If :create/time is 0 (Unix epoch, a valid timestamp), the || operator treats it as falsy and incorrectly falls back to Date.now(), creating an incorrect creation time. This is inconsistent with the nullish coalescing used elsewhere.
Fix:
const createdTime = relData[":create/time"] ?? Date.now();This ensures only null or undefined trigger the fallback, not the valid timestamp 0.
| const createdTime = relData[":create/time"] || Date.now(); | |
| const createdTime = relData[":create/time"] ?? Date.now(); |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
77fde60 to
5579a4e
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| s: DiscourseNode, | ||
| ): CrossAppNodeSchema | null => { | ||
| const relData = window.roamAlphaAPI.pull( | ||
| "[:create/time :edit/time {:create/user [:user/uid]}]", |
There was a problem hiding this comment.
Does :page/edit-time not track both blocks+page edit time?
| s: DiscourseNode, | ||
| ): CrossAppNodeSchema | null => { | ||
| const relData = window.roamAlphaAPI.pull( | ||
| "[:create/time :edit/time {:create/user [:user/uid]}]", |
| if (!userUid) return null; | ||
| const createdTime = relData[":create/time"] || Date.now(); | ||
| // A node type's settings live either in the page's props or in blocks below it, | ||
| // depending on the settings store in use, so neither time alone sees every edit: |
| authorId: userUid, | ||
| createdAt: new Date(relData[":create/time"] || Date.now()), | ||
| createdAt: new Date(createdTime), | ||
| modifiedAt: new Date(Math.max(editTime, pageEditTime, createdTime)), |
There was a problem hiding this comment.
nit: probably just need pageEditTime
https://linear.app/discourse-graphs/issue/ENG-2167/nodeschematocrossapp-is-missing-modifiedat
https://www.loom.com/share/da17bd5e58b8479b95934fbabf5b1acf