Skip to content

Commit 8ff9987

Browse files
rgilksrgilks
committed
refactor: consolidate session user logic, remove getAuthenticatedUserId, strengthen type safety and tests
1 parent ef42d83 commit 8ff9987

File tree

4 files changed

+60
-123
lines changed

4 files changed

+60
-123
lines changed

‎app/actions/authUtils.test.ts‎

Lines changed: 15 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { describe, it, expect, vi, beforeEach, Mock } from 'vitest';
1+
import { describe, it, expect, vi, beforeEach } from 'vitest';
22
import { getServerSession } from 'next-auth/next';
3-
import { getAuthenticatedUserId, getAuthenticatedSessionUser } from './authUtils';
3+
import { getAuthenticatedSessionUser, SessionUserSchema } from './authUtils';
44

55
// Mock next-auth/next
66
vi.mock('next-auth/next', () => ({
@@ -12,101 +12,43 @@ vi.mock('@/lib/authOptions', () => ({
1212
authOptions: { someConfig: 'value' },
1313
}));
1414

15-
// Define a type for the mock session user for clarity
16-
interface MockSessionUser {
17-
dbId?: number;
18-
name?: string;
19-
email?: string;
20-
}
21-
2215
describe('Auth Utility Functions', () => {
23-
// Cast the mock function to the correct type for TypeScript
24-
const mockGetServerSession = getServerSession as Mock;
16+
const mockGetServerSession = getServerSession as unknown as ReturnType<typeof vi.fn>;
2517

2618
beforeEach(() => {
2719
// Reset mocks before each test
2820
vi.clearAllMocks();
2921
});
3022

31-
describe('getAuthenticatedUserId', () => {
32-
it('should return the user ID when session and dbId exist', async () => {
33-
const mockSession = { user: { dbId: 123 } };
34-
mockGetServerSession.mockResolvedValue(mockSession);
35-
const userId = await getAuthenticatedUserId();
36-
expect(userId).toBe(123);
37-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
38-
});
39-
40-
it('should return null when session exists but dbId is missing', async () => {
41-
const mockSession = { user: { name: 'Test User' } }; // No dbId
42-
mockGetServerSession.mockResolvedValue(mockSession);
43-
const userId = await getAuthenticatedUserId();
44-
expect(userId).toBeNull();
45-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
46-
});
47-
48-
it('should return null when session exists but user is missing', async () => {
49-
const mockSession = {}; // No user
50-
mockGetServerSession.mockResolvedValue(mockSession);
51-
const userId = await getAuthenticatedUserId();
52-
expect(userId).toBeNull();
53-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
54-
});
55-
56-
it('should return null when session does not exist', async () => {
57-
mockGetServerSession.mockResolvedValue(null);
58-
const userId = await getAuthenticatedUserId();
59-
expect(userId).toBeNull();
60-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
61-
});
62-
63-
it('should return null if getServerSession throws an error', async () => {
64-
const error = new Error('Session fetch failed');
65-
mockGetServerSession.mockRejectedValue(error);
66-
await expect(getAuthenticatedUserId()).rejects.toThrow('Session fetch failed');
67-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
68-
});
69-
});
70-
7123
describe('getAuthenticatedSessionUser', () => {
72-
it('should return the session user object when session and dbId exist', async () => {
73-
const mockUser: MockSessionUser = { dbId: 456, name: 'Another User' };
74-
const mockSession = { user: mockUser };
75-
mockGetServerSession.mockResolvedValue(mockSession);
24+
it('returns the session user when valid', async () => {
25+
const user = { dbId: 456, name: 'User' };
26+
mockGetServerSession.mockResolvedValue({ user });
7627
const sessionUser = await getAuthenticatedSessionUser();
77-
expect(sessionUser).toEqual(mockUser);
78-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
28+
expect(sessionUser).toEqual(SessionUserSchema.parse(user));
7929
});
8030

81-
it('should return null when session exists but dbId is missing', async () => {
82-
const mockUser: MockSessionUser = { name: 'User Without ID' };
83-
const mockSession = { user: mockUser };
84-
mockGetServerSession.mockResolvedValue(mockSession);
31+
it('returns null when dbId is missing', async () => {
32+
mockGetServerSession.mockResolvedValue({ user: { name: 'NoId' } });
8533
const sessionUser = await getAuthenticatedSessionUser();
8634
expect(sessionUser).toBeNull();
87-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
8835
});
8936

90-
it('should return null when session exists but user object is missing', async () => {
91-
const mockSession = {}; // No user object
92-
mockGetServerSession.mockResolvedValue(mockSession);
37+
it('returns null when user is missing', async () => {
38+
mockGetServerSession.mockResolvedValue({});
9339
const sessionUser = await getAuthenticatedSessionUser();
9440
expect(sessionUser).toBeNull();
95-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
9641
});
9742

98-
it('should return null when session does not exist', async () => {
43+
it('returns null when session is null', async () => {
9944
mockGetServerSession.mockResolvedValue(null);
10045
const sessionUser = await getAuthenticatedSessionUser();
10146
expect(sessionUser).toBeNull();
102-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
10347
});
10448

105-
it('should return null if getServerSession throws an error', async () => {
106-
const error = new Error('Session fetch failed');
107-
mockGetServerSession.mockRejectedValue(error);
108-
await expect(getAuthenticatedSessionUser()).rejects.toThrow('Session fetch failed');
109-
expect(mockGetServerSession).toHaveBeenCalledTimes(1);
49+
it('throws if getServerSession throws', async () => {
50+
mockGetServerSession.mockRejectedValue(new Error('fail'));
51+
await expect(getAuthenticatedSessionUser()).rejects.toThrow('fail');
11052
});
11153
});
11254
});

‎app/actions/authUtils.ts‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,20 @@
11
import { getServerSession } from 'next-auth/next';
2-
import type { Session } from 'next-auth';
32
import { authOptions } from '@/lib/authOptions';
3+
import { z } from 'zod';
44

5-
interface SessionUser extends NonNullable<Session['user']> {
6-
dbId?: number;
7-
}
5+
export const SessionUserSchema = z.object({
6+
dbId: z.number(),
7+
name: z.string().optional(),
8+
email: z.string().optional(),
9+
image: z.string().optional(),
10+
});
811

9-
export const getAuthenticatedUserId = async (): Promise<number | null> => {
10-
const session = await getServerSession(authOptions);
11-
const sessionUser = session?.user as SessionUser | undefined;
12-
13-
if (!session || !sessionUser?.dbId) {
14-
return null;
15-
}
16-
17-
return sessionUser.dbId;
18-
};
12+
export type SessionUser = z.infer<typeof SessionUserSchema>;
1913

2014
export const getAuthenticatedSessionUser = async (): Promise<SessionUser | null> => {
2115
const session = await getServerSession(authOptions);
22-
const sessionUser = session?.user as SessionUser | undefined;
23-
24-
if (!session || !sessionUser?.dbId) {
25-
return null;
26-
}
27-
28-
return sessionUser;
16+
const user = session?.user;
17+
const parsed = SessionUserSchema.safeParse(user);
18+
if (!parsed.success) return null;
19+
return parsed.data;
2920
};

‎app/actions/progress.test.ts‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest';
22
import { updateProgress, submitAnswer, getProgress, submitFeedback } from '@/app/actions/progress';
33
import { calculateAndUpdateProgress } from '@/lib/progressUtils';
4-
import { getAuthenticatedUserId } from '@/app/actions/authUtils';
4+
import { getAuthenticatedSessionUser } from '@/app/actions/authUtils';
55
import { findQuizById } from '@/lib/repositories/quizRepository';
66
import { getProgress as findUserProgress } from '@/lib/repositories/progressRepository';
77
import { createFeedback } from '@/lib/repositories/feedbackRepository';
@@ -76,15 +76,15 @@ describe('User Progress Server Actions', () => {
7676
const params = { isCorrect: true, language: MOCK_LANGUAGE };
7777

7878
it('should return Unauthorized if user is not authenticated', async () => {
79-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(null);
79+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue(null);
8080
const result = await updateProgress(params);
8181
expect(result.error).toBe('Unauthorized');
8282
expect(result.currentLevel).toBe('A1');
8383
expect(vi.mocked(calculateAndUpdateProgress)).not.toHaveBeenCalled();
8484
});
8585

8686
it('should return Invalid parameters for invalid input', async () => {
87-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
87+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
8888
const invalidParams = { isCorrect: 'yes' as any, language: 'e' };
8989
const result = await updateProgress(invalidParams);
9090
expect(result.error).toBe('Invalid parameters');
@@ -93,7 +93,7 @@ describe('User Progress Server Actions', () => {
9393
});
9494

9595
it('should call calculateAndUpdateProgress and return its result on success', async () => {
96-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
96+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
9797
const mockProgressResult = {
9898
currentLevel: 'B1' as const,
9999
currentStreak: 1,
@@ -113,7 +113,7 @@ describe('User Progress Server Actions', () => {
113113
});
114114

115115
it('should return error from calculateAndUpdateProgress if it fails', async () => {
116-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
116+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
117117
const mockErrorResult = {
118118
currentLevel: 'A1' as const,
119119
currentStreak: 0,
@@ -132,22 +132,22 @@ describe('User Progress Server Actions', () => {
132132
const baseParams = { learn: MOCK_LANGUAGE, lang: 'de', id: MOCK_QUIZ_ID };
133133

134134
it('should return Invalid request parameters for invalid input', async () => {
135-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
135+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
136136
const invalidParams = { ...baseParams, ans: 'too long' };
137137
const result = await submitAnswer(invalidParams);
138138
expect(result.error).toBe('Invalid request parameters.');
139139
expect(vi.mocked(findQuizById)).not.toHaveBeenCalled();
140140
});
141141

142142
it('should return Missing or invalid quiz ID if id is missing', async () => {
143-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
143+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
144144
const invalidParams = { ...baseParams, id: undefined };
145145
const result = await submitAnswer(invalidParams);
146146
expect(result.error).toBe('Missing or invalid quiz ID.');
147147
});
148148

149149
it('should return Quiz data unavailable if quiz is not found', async () => {
150-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
150+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
151151
vi.mocked(findQuizById).mockReturnValue(null);
152152

153153
const result = await submitAnswer({ ...baseParams, ans: 'a' });
@@ -157,7 +157,7 @@ describe('User Progress Server Actions', () => {
157157
});
158158

159159
it('should return Quiz data unavailable if quiz content parsing fails against QuizDataSchema', async () => {
160-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
160+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
161161
const repoReturnWithMalformedContent = {
162162
...MOCK_RAW_QUIZ_REPO_RETURN,
163163
content: { ...MOCK_RAW_QUIZ_REPO_RETURN.content, question: undefined }, // Missing required question
@@ -170,7 +170,7 @@ describe('User Progress Server Actions', () => {
170170
});
171171

172172
it('should process correct answer, generate feedback, and update progress for authenticated user', async () => {
173-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
173+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
174174
const repoReturnWithValidContent = {
175175
...MOCK_RAW_QUIZ_REPO_RETURN,
176176
content: { ...MOCK_PARSED_QUIZ_DATA_CONTENT },
@@ -202,7 +202,7 @@ describe('User Progress Server Actions', () => {
202202
});
203203

204204
it('should process incorrect answer, generate feedback, and update progress for authenticated user', async () => {
205-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
205+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
206206
const repoReturnWithValidContent = {
207207
...MOCK_RAW_QUIZ_REPO_RETURN,
208208
content: { ...MOCK_PARSED_QUIZ_DATA_CONTENT },
@@ -232,7 +232,7 @@ describe('User Progress Server Actions', () => {
232232
});
233233

234234
it('should generate feedback but not update progress for anonymous user', async () => {
235-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(null);
235+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue(null);
236236
const repoReturnWithValidContent = {
237237
...MOCK_RAW_QUIZ_REPO_RETURN,
238238
content: { ...MOCK_PARSED_QUIZ_DATA_CONTENT },
@@ -251,7 +251,7 @@ describe('User Progress Server Actions', () => {
251251
});
252252

253253
it('should return error from calculateAndUpdateProgress if it fails during progress update', async () => {
254-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
254+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
255255
const repoReturnWithValidContent = {
256256
...MOCK_RAW_QUIZ_REPO_RETURN,
257257
content: { ...MOCK_PARSED_QUIZ_DATA_CONTENT },
@@ -285,7 +285,7 @@ describe('User Progress Server Actions', () => {
285285
const params = { language: MOCK_LANGUAGE };
286286

287287
it('should return Unauthorized if user is not authenticated', async () => {
288-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(null);
288+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue(null);
289289
const result = await getProgress(params);
290290
expect(result.error).toBe('Unauthorized: User not logged in.');
291291
expect(result.currentLevel).toBe('A1');
@@ -295,7 +295,7 @@ describe('User Progress Server Actions', () => {
295295
});
296296

297297
it('should return Invalid parameters for invalid input', async () => {
298-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
298+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
299299
const invalidParams = { language: 'e' };
300300
const result = await getProgress(invalidParams);
301301
expect(result.error).toBe('Invalid parameters provided.');
@@ -306,7 +306,7 @@ describe('User Progress Server Actions', () => {
306306
});
307307

308308
it('should fetch and return existing progress for authenticated user', async () => {
309-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
309+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
310310
vi.mocked(findUserProgress).mockReturnValue(MOCK_PROGRESS_DATA);
311311

312312
const result = await getProgress(params);
@@ -319,7 +319,7 @@ describe('User Progress Server Actions', () => {
319319
});
320320

321321
it('should return default progress if no record exists for authenticated user', async () => {
322-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
322+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
323323
vi.mocked(findUserProgress).mockReturnValue(null);
324324

325325
const result = await getProgress(params);
@@ -332,7 +332,7 @@ describe('User Progress Server Actions', () => {
332332
});
333333

334334
it('should handle repository error during progress fetch', async () => {
335-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
335+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
336336
const repoError = new Error('Repo Connection Lost');
337337
vi.mocked(findUserProgress).mockImplementation(() => {
338338
throw repoError;
@@ -358,7 +358,7 @@ describe('User Progress Server Actions', () => {
358358
};
359359

360360
it('should return Unauthorized if user is not authenticated', async () => {
361-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(null);
361+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue(null);
362362
const result = await submitFeedback(params);
363363
expect(result.success).toBe(false);
364364
expect(result.error).toBe('Unauthorized');
@@ -367,7 +367,7 @@ describe('User Progress Server Actions', () => {
367367
});
368368

369369
it('should return Invalid parameters for invalid input', async () => {
370-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
370+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
371371
const invalidParams = { ...params, quizId: -5 };
372372
const result = await submitFeedback(invalidParams);
373373
expect(result.success).toBe(false);
@@ -377,7 +377,7 @@ describe('User Progress Server Actions', () => {
377377
});
378378

379379
it('should return Quiz not found if quiz ID does not exist', async () => {
380-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
380+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
381381
vi.mocked(findQuizById).mockReturnValue(null);
382382

383383
const result = await submitFeedback(params);
@@ -389,7 +389,7 @@ describe('User Progress Server Actions', () => {
389389
});
390390

391391
it('should call createFeedback and return success if quiz exists and repo call succeeds', async () => {
392-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
392+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
393393
vi.mocked(findQuizById).mockReturnValue(MOCK_RAW_QUIZ_REPO_RETURN as any);
394394
vi.mocked(createFeedback).mockReturnValue(1);
395395

@@ -409,7 +409,7 @@ describe('User Progress Server Actions', () => {
409409
});
410410

411411
it('should handle optional userAnswer and isCorrect (passing undefined/boolean to repo)', async () => {
412-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
412+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
413413
vi.mocked(findQuizById).mockReturnValue(MOCK_RAW_QUIZ_REPO_RETURN as any);
414414
vi.mocked(createFeedback).mockReturnValue(1);
415415

@@ -435,7 +435,7 @@ describe('User Progress Server Actions', () => {
435435
});
436436

437437
it('should return error if repository createFeedback fails', async () => {
438-
vi.mocked(getAuthenticatedUserId).mockResolvedValue(MOCK_USER_ID);
438+
vi.mocked(getAuthenticatedSessionUser).mockResolvedValue({ dbId: MOCK_USER_ID });
439439
vi.mocked(findQuizById).mockReturnValue(MOCK_RAW_QUIZ_REPO_RETURN as any);
440440
const repoError = new Error('Insert failed');
441441
vi.mocked(createFeedback).mockImplementation(() => {

0 commit comments

Comments
 (0)