Fix Discord linking error handling and section routing issues
- Reset linkingDiscord state and show error message when linkSocial throws, so users can retry if the OAuth flow fails - Move VALID_SECTION_IDS to module level and derive it from SECTIONS to avoid per-render allocation and prevent the two from drifting out of sync - Remove premature SectionId cast before the allowlist validation in the ?section= URL param handling - Add tests for loading state and error recovery in AccountSection Fixes #455 https://claude.ai/code/session_01UerTpTuBWjreTNEns8srkp
This commit is contained in:
parent
737a808777
commit
a35df3f5b0
3 changed files with 62 additions and 17 deletions
|
|
@ -22,13 +22,20 @@ const PROVIDER_LABELS: Record<string, string> = {
|
||||||
|
|
||||||
export function AccountSection({ email, linkedAccounts }: Props) {
|
export function AccountSection({ email, linkedAccounts }: Props) {
|
||||||
const [linkingDiscord, setLinkingDiscord] = useState(false);
|
const [linkingDiscord, setLinkingDiscord] = useState(false);
|
||||||
|
const [linkError, setLinkError] = useState<string | null>(null);
|
||||||
const hasPassword = linkedAccounts.some((a) => a.providerId === "credential");
|
const hasPassword = linkedAccounts.some((a) => a.providerId === "credential");
|
||||||
const oauthAccounts = linkedAccounts.filter((a) => a.providerId !== "credential");
|
const oauthAccounts = linkedAccounts.filter((a) => a.providerId !== "credential");
|
||||||
const hasDiscord = linkedAccounts.some((a) => a.providerId === "discord");
|
const hasDiscord = linkedAccounts.some((a) => a.providerId === "discord");
|
||||||
|
|
||||||
async function handleConnectDiscord() {
|
async function handleConnectDiscord() {
|
||||||
setLinkingDiscord(true);
|
setLinkingDiscord(true);
|
||||||
await authClient.linkSocial({ provider: "discord", callbackURL: "/settings?section=account" });
|
setLinkError(null);
|
||||||
|
try {
|
||||||
|
await authClient.linkSocial({ provider: "discord", callbackURL: "/settings?section=account" });
|
||||||
|
} catch {
|
||||||
|
setLinkError("Failed to connect Discord. Please try again.");
|
||||||
|
setLinkingDiscord(false);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
|
|
@ -68,14 +75,19 @@ export function AccountSection({ email, linkedAccounts }: Props) {
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
{!hasDiscord && (
|
{!hasDiscord && (
|
||||||
<Button
|
<div className="space-y-2">
|
||||||
variant="outline"
|
<Button
|
||||||
size="sm"
|
variant="outline"
|
||||||
onClick={handleConnectDiscord}
|
size="sm"
|
||||||
disabled={linkingDiscord}
|
onClick={handleConnectDiscord}
|
||||||
>
|
disabled={linkingDiscord}
|
||||||
{linkingDiscord ? "Redirecting…" : "Connect Discord"}
|
>
|
||||||
</Button>
|
{linkingDiscord ? "Redirecting…" : "Connect Discord"}
|
||||||
|
</Button>
|
||||||
|
{linkError && (
|
||||||
|
<p className="text-sm text-destructive">{linkError}</p>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
import { render, screen, fireEvent } from "@testing-library/react";
|
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
|
||||||
import { AccountSection } from "../AccountSection";
|
import { AccountSection } from "../AccountSection";
|
||||||
|
|
||||||
vi.mock("react-router", () => ({
|
vi.mock("react-router", () => ({
|
||||||
|
|
@ -44,7 +44,7 @@ describe("AccountSection", () => {
|
||||||
expect(screen.queryByRole("button", { name: /connect discord/i })).not.toBeInTheDocument();
|
expect(screen.queryByRole("button", { name: /connect discord/i })).not.toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("calls authClient.linkSocial with discord provider on button click", () => {
|
it("calls authClient.linkSocial with discord provider on button click", async () => {
|
||||||
render(
|
render(
|
||||||
<AccountSection
|
<AccountSection
|
||||||
email="user@example.com"
|
email="user@example.com"
|
||||||
|
|
@ -52,9 +52,42 @@ describe("AccountSection", () => {
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
fireEvent.click(screen.getByRole("button", { name: /connect discord/i }));
|
fireEvent.click(screen.getByRole("button", { name: /connect discord/i }));
|
||||||
expect(mockLinkSocial).toHaveBeenCalledWith({
|
await waitFor(() => {
|
||||||
provider: "discord",
|
expect(mockLinkSocial).toHaveBeenCalledWith({
|
||||||
callbackURL: "/settings?section=account",
|
provider: "discord",
|
||||||
|
callbackURL: "/settings?section=account",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows Redirecting… and disables the button while linking", async () => {
|
||||||
|
// Never resolves so we can inspect the in-flight state
|
||||||
|
mockLinkSocial.mockReturnValue(new Promise(() => {}));
|
||||||
|
render(
|
||||||
|
<AccountSection
|
||||||
|
email="user@example.com"
|
||||||
|
linkedAccounts={[{ id: "1", providerId: "credential" }]}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: /connect discord/i }));
|
||||||
|
await waitFor(() => {
|
||||||
|
const btn = screen.getByRole("button", { name: /redirecting/i });
|
||||||
|
expect(btn).toBeDisabled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("re-enables the button and shows an error message when linkSocial throws", async () => {
|
||||||
|
mockLinkSocial.mockRejectedValue(new Error("OAuth failed"));
|
||||||
|
render(
|
||||||
|
<AccountSection
|
||||||
|
email="user@example.com"
|
||||||
|
linkedAccounts={[{ id: "1", providerId: "credential" }]}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: /connect discord/i }));
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByRole("button", { name: /connect discord/i })).not.toBeDisabled();
|
||||||
|
expect(screen.getByText(/failed to connect discord/i)).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -35,6 +35,7 @@ const SECTIONS: readonly SettingsGridSection[] = [
|
||||||
] as const;
|
] as const;
|
||||||
|
|
||||||
const VALID_TIMEZONES = new Set(TIMEZONE_OPTIONS.flatMap((g) => g.zones.map((z) => z.value)));
|
const VALID_TIMEZONES = new Set(TIMEZONE_OPTIONS.flatMap((g) => g.zones.map((z) => z.value)));
|
||||||
|
const VALID_SECTION_IDS = new Set(SECTIONS.map((s) => s.id as SectionId));
|
||||||
|
|
||||||
const DATA_REQUEST_COOLDOWN_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
|
const DATA_REQUEST_COOLDOWN_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
|
||||||
|
|
||||||
|
|
@ -207,9 +208,8 @@ export async function action(args: Route.ActionArgs): Promise<ActionData | Respo
|
||||||
export default function SettingsPage({ loaderData, actionData }: Route.ComponentProps) {
|
export default function SettingsPage({ loaderData, actionData }: Route.ComponentProps) {
|
||||||
const { user, isInActiveDraft, linkedAccounts, dataRequestCooldownUntil } = loaderData;
|
const { user, isInActiveDraft, linkedAccounts, dataRequestCooldownUntil } = loaderData;
|
||||||
const [searchParams] = useSearchParams();
|
const [searchParams] = useSearchParams();
|
||||||
const VALID_SECTION_IDS = new Set<SectionId>(["profile", "account", "notifications", "api", "privacy"]);
|
const raw = searchParams.get("section");
|
||||||
const sectionParam = searchParams.get("section") as SectionId | null;
|
const initialSection: SectionId = raw && (VALID_SECTION_IDS as Set<string>).has(raw) ? (raw as SectionId) : "profile";
|
||||||
const initialSection: SectionId = sectionParam && VALID_SECTION_IDS.has(sectionParam) ? sectionParam : "profile";
|
|
||||||
const [activeSection, setActiveSection] = useState<SectionId>(initialSection);
|
const [activeSection, setActiveSection] = useState<SectionId>(initialSection);
|
||||||
const [mobileView, setMobileView] = useState<"grid" | "section">("grid");
|
const [mobileView, setMobileView] = useState<"grid" | "section">("grid");
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue