fix(share): return to video after signing in from denied share page (#2192) - #2287
Conversation
| export function PolicyDeniedView({ videoId, reason }: PolicyDeniedViewProps) { | ||
| const loginHref = videoId | ||
| ? `/login?next=${encodeURIComponent(`/s/${videoId}`)}` | ||
| : "/login"; |
There was a problem hiding this comment.
A signed-in non-owner can reach this private-video view, but both sign-in links send them to /login?next=/s/{videoId}. Because the login page detects the existing session and immediately returns them to the same denied page, this action creates a no-op redirect loop. This is non-blocking, but the sign-in option should only appear for anonymous viewers, or authenticated viewers should get an account-switch path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/s/[videoId]/_components/PolicyDeniedView.tsx
Line: 10-13
Comment:
**Authenticated sign-in loops**
A signed-in non-owner can reach this private-video view, but both sign-in links send them to `/login?next=/s/{videoId}`. Because the login page detects the existing session and immediately returns them to the same denied page, this action creates a no-op redirect loop. This is non-blocking, but the sign-in option should only appear for anonymous viewers, or authenticated viewers should get an account-switch path.
---
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!
| const children = (view as ReactElement<{ children: ReactElement[] }>).props | ||
| .children; | ||
| const titleElement = children[1]; | ||
| const buttonElement = children[3]; | ||
|
|
||
| expect(titleElement.props.children).toBe("This video is private"); | ||
| expect(buttonElement).toBeDefined(); | ||
|
|
There was a problem hiding this comment.
These tests inspect unrendered React children through fixed array positions. An unrelated markup reorder will break them, while they do not verify the actual anchor produced by Button, Radix Slot, and Next.js Link. This makes the tests brittle and gives less confidence in the rendered behavior; render the component to static markup and assert its visible text and anchor attributes instead. The same pattern also appears in the other tests in this file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/__tests__/unit/share-policy-denied.test.ts
Line: 11-18
Comment:
**Tests depend on child order**
These tests inspect unrendered React children through fixed array positions. An unrelated markup reorder will break them, while they do not verify the actual anchor produced by `Button`, Radix `Slot`, and Next.js `Link`. This makes the tests brittle and gives less confidence in the rendered behavior; render the component to static markup and assert its visible text and anchor attributes instead. The same pattern also appears in the other tests in this file.
---
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!
Resolves #2192
Summary
When a viewer lands on a restricted share page (
/s/{videoId}withemail_restriction_login_requiredor a private video),PolicyDeniedViewpreviously linked to/loginwithout anextredirect query parameter. After authenticating, the viewer was redirected to/dashboardrather than back to the requested video.This PR:
videoIdintoPolicyDeniedViewand constructsloginHref = videoId ?/login?next=${encodeURIComponent(/s/${videoId})}: "/login".@cap/uiButton(withasChild) for restricted access and private video views.email_restriction_deniedcopy-only (omits sign-in button since the viewer is already logged in with an unauthorized domain).PolicyDeniedViewintoapps/web/app/s/[videoId]/_components/PolicyDeniedView.tsxfollowing the modular structure of_components/.Testing
apps/web/__tests__/unit/share-policy-denied.test.tscovering:/login?next=%2Fs%2F{videoId}links and sign-in button.email_restriction_login_requiredgenerating return links and sign-in button.email_restriction_deniedrendering copy-only without sign-in button./loginwhenvideoIdis omitted.The PR appears safe to merge, with non-blocking improvements recommended for the authenticated private-video experience and test robustness.
Findings
Fix with agent prompt
Summary
nextparameter.Reviews (1) · Last reviewed commit: "fix(share): redirect back to video after..."