Skip to content

fix(share): return to video after signing in from denied share page (#2192) #2287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
massmarketconsumer-arch wants to merge 1 commit into
base: main
Choose a base branch
from
+130 −36
Open

fix(share): return to video after signing in from denied share page (#2192) #2287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 70 additions & 0 deletions apps/web/__tests__/unit/share-policy-denied.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { isValidElement, type ReactElement } from "react";
import { describe, expect, it } from "vitest";
import { PolicyDeniedView } from "../../app/s/[videoId]/_components/PolicyDeniedView";

describe("PolicyDeniedView", () => {
it("renders private video sign in link and button with next param", () => {
const videoId = "vid12345";
const view = PolicyDeniedView({ videoId });
expect(isValidElement(view)).toBe(true);

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();

Comment on lines +11 to +18

@greptile-apps greptile-apps Bot Sep 12, 2026

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.

P2 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.

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!

const linkChild = buttonElement.props.children;
expect(linkChild.props.href).toBe("/login?next=%2Fs%2Fvid12345");
});

it("renders email restriction login required with next param", () => {
const videoId = "restricted987";
const view = PolicyDeniedView({
videoId,
reason: "email_restriction_login_required",
});
expect(isValidElement(view)).toBe(true);

const children = (view as ReactElement<{ children: ReactElement[] }>).props
.children;
const titleElement = children[1];
const buttonElement = children[3];

expect(titleElement.props.children).toBe("This video requires sign-in");
expect(buttonElement).toBeDefined();

const linkChild = buttonElement.props.children;
expect(linkChild.props.href).toBe("/login?next=%2Fs%2Frestricted987");
});

it("renders email restriction denied without sign in button", () => {
const videoId = "restricted987";
const view = PolicyDeniedView({
videoId,
reason: "email_restriction_denied",
});
expect(isValidElement(view)).toBe(true);

const children = (
view as ReactElement<{ children: (ReactElement | boolean)[] }>
).props.children;
const titleElement = children[1] as ReactElement;
const buttonElement = children[3];

expect(titleElement.props.children).toBe("Access restricted");
expect(buttonElement).toBe(false);
});

it("falls back to /login when videoId is not provided", () => {
const view = PolicyDeniedView({});
const children = (view as ReactElement<{ children: ReactElement[] }>).props
.children;
const buttonElement = children[3];
expect(buttonElement).toBeDefined();
const linkChild = buttonElement.props.children;
expect(linkChild.props.href).toBe("/login");
});
});
56 changes: 56 additions & 0 deletions apps/web/app/s/[videoId]/_components/PolicyDeniedView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { Button, Logo } from "@cap/ui";
import Link from "next/link";
import type React from "react";

export interface PolicyDeniedViewProps {
videoId?: string;
reason?: string;
}

export function PolicyDeniedView({ videoId, reason }: PolicyDeniedViewProps) {
const loginHref = videoId
? `/login?next=${encodeURIComponent(`/s/${videoId}`)}`
: "/login";
Comment on lines +10 to +13

@greptile-apps greptile-apps Bot Sep 12, 2026

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.

P2 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.

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!

let title = "This video is private";
let description: React.ReactNode = (
<>
If you own this video, please{" "}
<Link href={loginHref} className="underline">
sign in
</Link>{" "}
to manage sharing.
</>
);
let showLoginButton = true;

if (reason === "email_restriction_login_required") {
title = "This video requires sign-in";
description = (
<>
The owner of this video has restricted access. Please{" "}
<Link href={loginHref} className="underline">
sign in
</Link>{" "}
with an authorized email address to view.
</>
);
} else if (reason === "email_restriction_denied") {
title = "Access restricted";
description =
"Your email address does not meet the requirements set by the video owner.";
showLoginButton = false;
}

return (
<div className="flex flex-col justify-center items-center p-4 min-h-screen text-center">
<Logo className="size-32" />
<h1 className="mb-2 text-2xl font-semibold">{title}</h1>
<p className="text-gray-400">{description}</p>
{showLoginButton && (
<Button asChild className="mt-6">
<Link href={loginHref}>Sign in</Link>
</Button>
)}
</div>
);
}
40 changes: 4 additions & 36 deletions apps/web/app/s/[videoId]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
} from "@cap/database/schema";
import type { VideoMetadata } from "@cap/database/types";
import { buildEnv, serverEnv } from "@cap/env";
import { Logo } from "@cap/ui";
import { userIsPro } from "@cap/utils";
import {
Database,
Expand All @@ -36,7 +35,6 @@ import { and, eq, type InferSelectModel, isNull, sql } from "drizzle-orm";
import { Effect, Option } from "effect";
import type { Metadata } from "next";
import { headers } from "next/headers";
import Link from "next/link";
import { notFound } from "next/navigation";
import { getVideoAnalytics } from "@/actions/videos/get-analytics";
import {
Expand Down Expand Up @@ -76,6 +74,7 @@ import { optionFromTOrFirst } from "@/utils/effect";
import { isAiGenerationEnabled } from "@/utils/flags";
import { PasswordOverlay } from "./_components/PasswordOverlay";
import { PendingRecordingShare } from "./_components/PendingRecordingShare";
import { PolicyDeniedView } from "./_components/PolicyDeniedView";
import { ShareHeader } from "./_components/ShareHeader";
import { Share } from "./Share";

Expand Down Expand Up @@ -184,41 +183,10 @@ async function getSharedSpacesForVideo(videoId: Video.VideoId) {
};
}

function PolicyDeniedView({ reason }: { reason?: string }) {
let title = "This video is private";
let description: React.ReactNode = (
<>
If you own this video, please <Link href="/login">sign in</Link> to manage
sharing.
</>
);

if (reason === "email_restriction_login_required") {
title = "This video requires sign-in";
description = (
<>
The owner of this video has restricted access. Please{" "}
<Link href="/login">sign in</Link> with an authorized email address to
view.
</>
);
} else if (reason === "email_restriction_denied") {
title = "Access restricted";
description =
"Your email address does not meet the requirements set by the video owner.";
}

return (
<div className="flex flex-col justify-center items-center p-4 min-h-screen text-center">
<Logo className="size-32" />
<h1 className="mb-2 text-2xl font-semibold">{title}</h1>
<p className="text-gray-400">{description}</p>
</div>
);
}

const renderPolicyDenied = (videoId: Video.VideoId, reason?: string) =>
Effect.succeed(<PolicyDeniedView key={videoId} reason={reason} />);
Effect.succeed(
<PolicyDeniedView key={videoId} videoId={videoId} reason={reason} />,
);

const renderNoSuchElement = (awaitRecording: boolean) =>
awaitRecording
Expand Down