fix(web): apply render_mode from access-link live updates (AP-317)#558
fix(web): apply render_mode from access-link live updates (AP-317)#558isuttell wants to merge 1 commit into
Conversation
When a live update publishes a revision with a different render mode, the share-link viewer now updates render_mode and title from the pointer alongside iframe_src. Previously only iframe_src was copied, leaving AccessLinkBrandBar stale until a full reload. Adds a contract test asserting markdown → html mode flips without reload.
📝 WalkthroughWalkthroughThe live-update handler in the Access Link viewer route now propagates ChangesAccess Link Live Pointer Update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/test/viewer-live-revoked.test.tsx`:
- Around line 220-223: The test is missing an assertion to validate that the
live title is properly propagated to the brand-bar metadata. After the existing
expect statements that check the iframe src attribute and the presence of "html"
text, add an additional assertion that verifies the title in the brand-bar
metadata has been updated correctly to complete the contract validation for this
regression test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c364fda8-89f3-40ca-819e-c942e2c6c2d4
📒 Files selected for processing (2)
apps/web/src/routes/al.$publicId.tsxapps/web/test/viewer-live-revoked.test.tsx
| await waitFor(() => expect(screen.getByTitle("Artifact content")).toHaveAttribute("src", nextIframeSrc)); | ||
| expect(screen.getByText("html")).toBeInTheDocument(); | ||
| expect(screen.queryByText("markdown")).not.toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Assert live title propagation in this new regression test.
This test validates render_mode and iframe_src, but the PR also changes live title updates. Add a post-pointer assertion for the updated title in the brand-bar metadata to lock the full contract.
Suggested patch
await waitFor(() => expect(screen.getByTitle("Artifact content")).toHaveAttribute("src", nextIframeSrc));
expect(screen.getByText("html")).toBeInTheDocument();
expect(screen.queryByText("markdown")).not.toBeInTheDocument();
+ expect(screen.getByText("Shared artifact")).toBeInTheDocument();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await waitFor(() => expect(screen.getByTitle("Artifact content")).toHaveAttribute("src", nextIframeSrc)); | |
| expect(screen.getByText("html")).toBeInTheDocument(); | |
| expect(screen.queryByText("markdown")).not.toBeInTheDocument(); | |
| }); | |
| await waitFor(() => expect(screen.getByTitle("Artifact content")).toHaveAttribute("src", nextIframeSrc)); | |
| expect(screen.getByText("html")).toBeInTheDocument(); | |
| expect(screen.queryByText("markdown")).not.toBeInTheDocument(); | |
| expect(screen.getByText("Shared artifact")).toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/test/viewer-live-revoked.test.tsx` around lines 220 - 223, The test
is missing an assertion to validate that the live title is properly propagated
to the brand-bar metadata. After the existing expect statements that check the
iframe src attribute and the presence of "html" text, add an additional
assertion that verifies the title in the brand-bar metadata has been updated
correctly to complete the contract validation for this regression test.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
First-pass review (AP-317)
Risk: simple
Decision: approve
Ticket triage
- Intended change: Share-link viewer (
/al/$publicId) should applyrender_mode(and other revision-scoped pointer fields) from live-update SSE events, not onlyiframe_src. - Scope match: Yes — two-line state merge fix plus a focused regression test.
Review findings
Blocking: none
Non-blocking:
titleis also synced from the pointer, which matchesLiveUpdatePointerand prevents a similar stale-metadata bug if a publish renames the artifact.- A title-only live update is untested, but the ticket only required a
render_modecontract test.
Merge checklist
- Ticket linked and understood: AP-317
- PR scope matches ticket: yes
- Checks green: Validate, Postgres smoke, CodeQL, secret scan
- Tests/docs appropriate: new vitest case covers markdown → html mode flip via
AccessLinkBrandBar - No blocking findings
- No high-risk areas (public viewer state sync only; no auth/billing/infra)
- Merge-safe: yes
Why this is safe
The live-update handler now mirrors all display-relevant LiveUpdatePointer fields already consumed by AccessLinkBrandBar (iframe_src, render_mode, title). The test exercises the reported bug path (stale render_mode in the floating metadata panel) and confirms the iframe src still updates.
Sent by Cursor Automation: First Pass PR Reviewer
2 participants
Summary
Fixes a pre-existing bug where the share-link viewer (
/al/$publicId) only appliedpointer.iframe_srcon live updates, leavingrender_mode(and title) stale until a full page reload.Changes
apps/web/src/routes/al.$publicId.tsx: OnonPointer, mergerender_modeandtitlefromLiveUpdatePointerinto resolved viewer state alongsideiframe_src.apps/web/test/viewer-live-revoked.test.tsx: Add contract test asserting a markdown → html live revision updates the brand bar without reload.Testing
Closes AP-317.
Linear Issue: AP-317
Summary by CodeRabbit
Bug Fixes
Tests