Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@greptileai review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (!user.isPro) { | ||
| setUpgradeModalOpen(true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
openDialog opens the upgrade prompt before mounting the dialog for owners without Pro. They cannot reach “Remove default,” even though updateDefaultCallToAction(null) allows removal without Pro. Allow owners with an existing default to open the dialog; keep the Pro check on saving.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/(org)/dashboard/settings/account/components/DefaultCallToAction.tsx
Line: 31-34
Comment:
**Downgraded owners cannot remove defaults**
`openDialog` opens the upgrade prompt before mounting the dialog for owners without Pro. They cannot reach “Remove default,” even though `updateDefaultCallToAction(null)` allows removal without Pro. Allow owners with an existing default to open the dialog; keep the Pro check on saving.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if (options.asDefault) { | ||
| await storeDefaultCallToAction(user.id, result.value); | ||
| await db() | ||
| .update(videos) | ||
| .set({ | ||
| settings: sql`JSON_REMOVE(${jsonObject(videos.settings)}, '$.callToAction')`, | ||
| }) | ||
| .where(ownedVideo); |
There was a problem hiding this comment.
Saving with asDefault commits the account default before clearing the video's own value. If the second write fails, other videos pick up the new default while this video keeps its old button—or stays off—and the dialog reports that saving failed. Put both writes in one database transaction so a failed save leaves both values unchanged.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/videos/call-to-action.ts
Line: 83-90
Comment:
**Failed saves change other videos**
Saving with `asDefault` commits the account default before clearing the video's own value. If the second write fails, other videos pick up the new default while this video keeps its old button—or stays off—and the dialog reports that saving failed. Put both writes in one database transaction so a failed save leaves both values unchanged.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…wngraded owners remove it
|
@greptileai review |
|
@richiemcilroy this one's ready too - a default call to action for every video, which a few people asked for on Discord. Greptile's at 5/5 and I tested it end to end locally (share page, embed, settings, Pro gating). |
Adds a default call to action. Set it once and it shows on every video that doesn't have its own call to action, so people no longer have to add the same "Book a call" button to each video by hand. It was asked for a few times on Discord, Loom-style.
How it works
defaultPlaybackSpeedfalls back to the organization setting. That means it covers every way a video gets created (desktop, web upload, Loom import, API) without touching any of them, it applies to videos you've already shared, and editing the default updates every video that uses it.callToAction: falseon the video.users.preferences.defaultCallToAction, so there's no migration. It's only sent to the page for the owner; viewers just get the resolved call to action.Two things worth knowing
falseinstead of deleting the key. Without that, a default would come back on a video where you'd removed the call to action. Videos whose call to action was removed before this PR have no key, so they'll pick up a default if their owner sets one.JSON_TYPEinstead ofCOALESCE. New users'preferencesis a JSONnull, not SQLNULL.COALESCEkeeps a JSONnull, andJSON_SETcan't add a key to it, so the first save silently did nothing. All writes in the action now treat anything that isn't a JSON object as{}. There's a test for it.Testing
false. Saving as default writes the user's preferences and clears the video's own copy. Setting a default requires Pro, but removing one doesn't.homepage-seo.test.ts(it expects noFAQPageon the homepage), also fails onmain.bun run typecheckand Biome: clean.{"callToAction": false}, and viewers saw no call to action on it.Open questions
The PR appears safe to merge; both previous findings are addressed.Summary
Adds a personal default call to action for videos without their own.
No new actionable issues were found. Checks were limited to reading code; tests and browser checks were not run.
Reviews (3) · Last reviewed commit: "fix(web): save a default call to action ..." · Reviewed by Greptile