Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
123 changes: 123 additions & 0 deletions frontend/src/__tests__/signinCallback.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/**
* Tests for app/signin/callback/page.tsx
*
* Covers the new error branches added to the OAuth callback handler:
* - Successful session → redirect to /dashboard
* - 403 from session API → redirect to /pending
* - Other failures from session API → inline error
* - Network errors → inline error
* - Missing user_id/name params → inline error
* - Missing is_approved param → inline error (not a /pending redirect)
* - Explicit is_approved=false → /pending redirect
*/
import React from 'react';
import { render, screen, waitFor } from '@testing-library/react';

const replace = jest.fn();
const setActiveUser = jest.fn();
const confirmApproved = jest.fn();
let searchParams = new URLSearchParams();

jest.mock('next/navigation', () => ({
useRouter: () => ({ replace }),
useSearchParams: () => searchParams,
}));

jest.mock('@/context/UserContext', () => ({
useUser: () => ({ setActiveUser, confirmApproved }),
}));

import CallbackPage from '@/app/signin/callback/page';

beforeEach(() => {
jest.clearAllMocks();
searchParams = new URLSearchParams();
global.fetch = jest.fn();
});

afterEach(() => {
// @ts-expect-error allow cleanup
delete global.fetch;
});

function setParams(params: Record<string, string>) {
searchParams = new URLSearchParams(params);
}

describe('signin/callback page', () => {
it('redirects to /dashboard on a successful session', async () => {
setParams({ user_id: 'u1', name: 'Ada', is_approved: 'true' });
(global.fetch as jest.Mock).mockResolvedValue({ ok: true, status: 200 });

render(<CallbackPage />);

await waitFor(() => expect(replace).toHaveBeenCalledWith('/dashboard'));
expect(setActiveUser).toHaveBeenCalledWith('u1', 'Ada', '');
expect(confirmApproved).toHaveBeenCalled();
});

it('redirects to /pending when session API returns 403', async () => {
setParams({ user_id: 'u1', name: 'Ada', is_approved: 'true' });
(global.fetch as jest.Mock).mockResolvedValue({ ok: false, status: 403 });

render(<CallbackPage />);

await waitFor(() => expect(replace).toHaveBeenCalledWith('/pending'));
expect(confirmApproved).not.toHaveBeenCalled();
});

it('shows an inline error when session API returns a non-403 failure', async () => {
setParams({ user_id: 'u1', name: 'Ada', is_approved: 'true' });
(global.fetch as jest.Mock).mockResolvedValue({ ok: false, status: 500 });

render(<CallbackPage />);

expect(await screen.findByText(/unable to complete sign-in/i)).toBeInTheDocument();
expect(replace).not.toHaveBeenCalled();
});

it('shows an inline error when the session fetch rejects', async () => {
setParams({ user_id: 'u1', name: 'Ada', is_approved: 'true' });
(global.fetch as jest.Mock).mockRejectedValue(new Error('network down'));

render(<CallbackPage />);

expect(await screen.findByText(/unable to reach the server/i)).toBeInTheDocument();
expect(replace).not.toHaveBeenCalled();
});

it('shows an inline error when user_id is missing', async () => {
setParams({ name: 'Ada', is_approved: 'true' });

render(<CallbackPage />);

expect(await screen.findByText(/sign-in failed/i)).toBeInTheDocument();
expect(global.fetch).not.toHaveBeenCalled();
expect(replace).not.toHaveBeenCalled();
});

it('shows an inline error when is_approved is missing entirely', async () => {
setParams({ user_id: 'u1', name: 'Ada' });

render(<CallbackPage />);

expect(await screen.findByText(/sign-in failed/i)).toBeInTheDocument();
expect(replace).not.toHaveBeenCalled();
});
Comment on lines +89 to +106

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a regression test for missing name query param.

The callback logic requires both user_id and name, but this suite currently only asserts the missing-user_id branch. Please add a missing-name case to lock the behavior.

Based on learnings: "If you change how a feature works (not just its interface), update the tests that cover it."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/__tests__/signinCallback.test.tsx` around lines 89 - 106, Add a
new test case mirroring the existing missing-param tests that asserts the page
shows an inline error when the name query parameter is absent: use setParams to
provide user_id and is_approved (e.g., setParams({ user_id: 'u1', is_approved:
'true' })), render the CallbackPage component, then assert await
screen.findByText(/sign-in failed/i) is in the document and that global.fetch
and replace were not called; place this alongside the other tests for missing
user_id and missing is_approved so the suite covers the missing-name regression.


it('redirects to /pending when is_approved is explicitly "false"', async () => {
setParams({ user_id: 'u1', name: 'Ada', is_approved: 'false' });

render(<CallbackPage />);

await waitFor(() => expect(replace).toHaveBeenCalledWith('/pending'));
});

it('redirects to /pending when error=not_approved is set', async () => {
setParams({ error: 'not_approved' });

render(<CallbackPage />);

await waitFor(() => expect(replace).toHaveBeenCalledWith('/pending'));
});
});
51 changes: 46 additions & 5 deletions frontend/src/app/signin/callback/page.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,32 @@
'use client';

import { useEffect, Suspense } from 'react';
import { useEffect, useState, Suspense } from 'react';
import { useSearchParams, useRouter } from 'next/navigation';
import { useUser } from '@/context/UserContext';

function CallbackInner() {
const searchParams = useSearchParams();
const router = useRouter();
const { setActiveUser, confirmApproved } = useUser();
const [errorMsg, setErrorMsg] = useState<string | null>(null);

useEffect(() => {
const userId = searchParams.get('user_id');
const name = searchParams.get('name');
const avatar = searchParams.get('avatar');
const isApproved = searchParams.get('is_approved') === 'true';
const approvedParam = searchParams.get('is_approved');
const error = searchParams.get('error');

if (error === 'not_approved' || !isApproved) {
if (error === 'not_approved' || approvedParam === 'false') {
router.replace('/pending');
return;
}

if (approvedParam !== 'true') {
setErrorMsg('Sign-in failed. Please try again.');
return;
}

if (userId && name) {
setActiveUser(userId, name, avatar || '');
fetch('/api/auth/session', {
Expand All @@ -31,15 +37,50 @@ function CallbackInner() {
if (res.ok) {
confirmApproved();
router.replace('/dashboard');
} else if (res.status === 403) {
router.replace('/pending');
} else {
router.replace('/signin');
setErrorMsg('Unable to complete sign-in. Please try again.');
}
}).catch(() => {
setErrorMsg('Unable to reach the server. Please try again.');
});
} else {
router.replace('/signin');
setErrorMsg('Sign-in failed. Please try again.');
}
}, []); // eslint-disable-line react-hooks/exhaustive-deps

if (errorMsg) {
return (
<div style={{
minHeight: '100vh',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
flexDirection: 'column',
gap: '16px',
background: '#f0f5f0',
padding: '24px',
}}>
<p style={{ color: '#374151', fontSize: '15px', textAlign: 'center' }}>{errorMsg}</p>
<a
href="/api/auth/google"
style={{
padding: '10px 24px',
background: '#1a5c2a',
color: '#fff',
borderRadius: '8px',
fontSize: '14px',
fontWeight: 500,
textDecoration: 'none',
}}
>
Try again
</a>
</div>
);
}

return (
<div style={{
minHeight: '100vh',
Expand Down
Loading