Skip to content

Commit fb4d1a6

Browse files
KATO-Hiroclaude
andcommitted
fix: implement CodeRabbit findings #1-10 (Phase 1-4)
## Phase 1: Doc / Rules - Add TSDoc to ButtonColor type (#3) - Update prisma-db.md: Prisma error handling pattern (#1) ## Phase 2: Internal client fixes - Add isValidTaskGrade type guard (src/lib/utils/task_grade.ts) to replace `as TaskGrade` casts (#5) - Replace `data.grade as TaskGrade` with guard validation in fetchMyVote/fetchMedianVote (#5) - Add SSR safety check to getBaseUrl with browser guard (#4) - Add signal parameter to fetchMedianVote for cancellation support (#10) ## Phase 3: Component fixes - Add `signal?.aborted` check in VotableGrade to suppress AbortError toast (#9) - Pass signal to fetchMedianVote to prevent race condition in concurrent requests (#10) - Improve comments in vote submission flow (#9) ## Phase 4: Service / Action layer - Catch P2025 error in updateTask, return null instead of throwing (#1) - Update vote_management and tasks route handlers to handle null return (#1) - Change vote_actions return from `await upsertVoteGradeTables(...)` to explicit `return { success: true as const }` (#7) ## Tests - Add src/test/utils/task_grade.test.ts for isValidTaskGrade type guard - Add src/test/lib/services/tasks.test.ts for updateTask error handling Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent 2d3494f commit fb4d1a6

11 files changed

Lines changed: 188 additions & 54 deletions

File tree

.claude/rules/prisma-db.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ Add `CHECK` constraints (via manual migration SQL) for count and invalid enum va
103103
## Service Layer Error Handling
104104

105105
Catch Prisma errors in service functions, return domain values:
106+
106107
- `P2025` (record not found) → `null` (no exception)
107108
- Other errors → re-throw (caller handles as 500)
108109

src/features/votes/actions/vote_actions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ export const voteAbsoluteGrade = async ({
5252
const grade = gradeRaw as TaskGrade;
5353

5454
try {
55-
return await upsertVoteGradeTables(userId, taskId, grade);
55+
await upsertVoteGradeTables(userId, taskId, grade);
56+
return { success: true as const };
5657
} catch (error) {
5758
console.error('Failed to vote absolute grade: ', error);
5859
return fail(INTERNAL_SERVER_ERROR, { message: 'Failed to record vote.' });

src/features/votes/components/VotableGrade.svelte

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,21 @@
9898
9999
submitVote(action, formData, signal)
100100
.then(async (succeeded) => {
101+
if (signal?.aborted) {
102+
return; // Intentional abort — user selected a different grade
103+
}
104+
101105
if (!succeeded) {
102106
throw new Error('vote failed');
103107
}
104108
105-
// 投票したグレードをローカル状態に反映(チェックマーク更新)
109+
// Reflect the voted grade in local state (to update the checkmark in the dropdown), even though the server response may later indicate a different median grade due to other voters or admin overrides.
106110
votedGrade = selectedVoteGrade ?? null;
107111
108-
// 成功したらサーバから最新の中央値を取得して表示を更新
112+
// Update the displayed grade to the latest median from the server, which may differ from the just-submitted vote due to other voters or admin overrides.
109113
const taskId = formData.get('taskId') as string;
110-
const medianGrade = await fetchMedianVote(taskId);
114+
const medianGrade = await fetchMedianVote(taskId, signal);
115+
111116
if (medianGrade !== null && taskResult.grade === TaskGrade.PENDING) {
112117
displayGrade = medianGrade;
113118
}

src/features/votes/internal_clients/vote_grade.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import type { TaskGrade } from '$lib/types/task';
2+
import { isValidTaskGrade } from '$lib/utils/task_grade';
3+
import { browser } from '$app/environment';
24

35
/**
46
* Fetches the current user's vote grade for a given task.
@@ -19,7 +21,7 @@ export async function fetchMyVote(taskId: string): Promise<TaskGrade | null> {
1921
}
2022

2123
const data = await response.json();
22-
return (data.grade as TaskGrade) ?? null;
24+
return isValidTaskGrade(data.grade) ? data.grade : null;
2325
} catch {
2426
return null;
2527
}
@@ -55,13 +57,17 @@ export async function submitVote(
5557
* Fetches the current median vote grade for a given task.
5658
* @returns The median grade, or null if not enough votes or request fails.
5759
*/
58-
export async function fetchMedianVote(taskId: string): Promise<TaskGrade | null> {
60+
export async function fetchMedianVote(
61+
taskId: string,
62+
signal?: AbortSignal,
63+
): Promise<TaskGrade | null> {
5964
try {
6065
const response = await fetch(
6166
`${getBaseUrl()}/problems/getMedianVote?taskId=${encodeURIComponent(taskId)}`,
6267
{
6368
credentials: 'same-origin',
6469
headers: { Accept: 'application/json' },
70+
signal,
6571
},
6672
);
6773

@@ -70,7 +76,7 @@ export async function fetchMedianVote(taskId: string): Promise<TaskGrade | null>
7076
}
7177

7278
const data = await response.json();
73-
return (data.grade as TaskGrade) ?? null;
79+
return isValidTaskGrade(data.grade) ? data.grade : null;
7480
} catch {
7581
return null;
7682
}
@@ -81,5 +87,15 @@ function getBaseUrl(): string {
8187
if (typeof globalThis !== 'undefined' && globalThis.location?.origin) {
8288
return globalThis.location.origin;
8389
}
90+
91+
if (!browser) {
92+
console.warn(
93+
'getBaseUrl() called in SSR context where browser APIs are unavailable. ' +
94+
'This should only be called from client-side code.',
95+
);
96+
97+
return '';
98+
}
99+
84100
return 'http://localhost';
85101
}

src/lib/services/tasks.ts

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Prisma } from '@prisma/client';
12
import { default as db } from '$lib/server/database';
23

34
import { getContestTaskPairs } from '$lib/services/contest_task_pairs';
@@ -179,42 +180,29 @@ export async function createTask(
179180
console.log(task);
180181
}
181182

182-
export async function updateTask(task_id: string, task_grade: TaskGrade) {
183-
const task = await db.task.update({
184-
where: { task_id: task_id },
185-
data: {
186-
grade: task_grade,
187-
},
188-
});
183+
/**
184+
* Updates a task's grade by task_id.
185+
*
186+
* @param task_id - The task ID to update
187+
* @param task_grade - The new grade value
188+
*
189+
* @returns undefined if successful, null if task not found (P2025)
190+
*/
191+
export async function updateTask(task_id: string, task_grade: TaskGrade): Promise<void | null> {
192+
try {
193+
const task = await db.task.update({
194+
where: { task_id: task_id },
195+
data: {
196+
grade: task_grade,
197+
},
198+
});
189199

190-
console.log(task);
191-
}
200+
console.log(task);
201+
} catch (error) {
202+
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') {
203+
return null;
204+
}
192205

193-
// TODO: deleteTask()
194-
195-
// Note:
196-
// Uncomment only when executing the following commands directly from the script.
197-
//
198-
// pnpm dlx vite-node ./prisma/tasks.ts
199-
//
200-
//
201-
// async function main() {
202-
// const tasks = getTasks();
203-
// console.log(tasks);
204-
205-
// const task_id = 'abc322_e';
206-
// const task = getTask(task_id);
207-
// console.log(task);
208-
209-
// // updateTask(task_id, TaskGrade.Q1);
210-
// // console.log(task);
211-
// }
212-
213-
// main()
214-
// .catch(async (e) => {
215-
// console.error(e);
216-
// process.exit(1);
217-
// })
218-
// .finally(async () => {
219-
// await db.$disconnect();
220-
// });
206+
throw error;
207+
}
208+
}
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Extract ButtonColor from ButtonProps (Flowbite Svelte exports ButtonProps)
22
import type { ButtonProps } from 'flowbite-svelte';
33

4-
// Get the color type from ButtonProps
4+
/**
5+
* Valid color variants for Flowbite Svelte button component.
6+
*/
57
export type ButtonColor = NonNullable<ButtonProps['color']>;

src/lib/utils/task_grade.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { taskGradeValues, type TaskGrade } from '$lib/types/task';
2+
3+
/**
4+
* Type guard to validate if an unknown value is a valid TaskGrade.
5+
* @param value - The value to validate
6+
* @returns true if value is a valid TaskGrade enum member
7+
*/
8+
export function isValidTaskGrade(value: unknown): value is TaskGrade {
9+
return typeof value === 'string' && taskGradeValues.includes(value as TaskGrade);
10+
}

src/routes/(admin)/tasks/+page.server.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,14 @@ export const actions: Actions = {
179179
};
180180
}
181181

182-
await taskService.updateTask(task_id, task_grade);
182+
const updateResult = await taskService.updateTask(task_id, task_grade);
183+
184+
if (updateResult === null) {
185+
return {
186+
success: false,
187+
};
188+
}
189+
183190
const contest_id = formData.get('contest_id')?.toString() as string;
184191

185192
const tasks = await apiClient.getTasks();

src/routes/(admin)/vote_management/+page.server.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { Actions, PageServerLoad } from './$types';
22

3-
import { Prisma } from '@prisma/client';
43
import { type TaskGrade, TaskGrade as TaskGradeEnum } from '$lib/types/task';
54
import { getTasksByTaskId, updateTask } from '$lib/services/tasks';
65
import {
@@ -52,15 +51,13 @@ export const actions: Actions = {
5251
) {
5352
return { success: false };
5453
}
55-
try {
56-
await updateTask(taskId, grade as TaskGrade);
57-
} catch (error) {
58-
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') {
59-
return { success: false, message: `Not found task: ${taskId}` };
60-
}
6154

62-
throw error;
55+
const result = await updateTask(taskId, grade as TaskGrade);
56+
57+
if (result === null) {
58+
return { success: false, message: `Not found task: ${taskId}` };
6359
}
60+
6461
return { success: true };
6562
},
6663
};
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { describe, test, expect, beforeEach, vi } from 'vitest';
2+
3+
import { Prisma } from '@prisma/client';
4+
5+
import { TaskGrade } from '$lib/types/task';
6+
import { updateTask } from '$lib/services/tasks';
7+
8+
vi.mock('$lib/server/database', () => ({
9+
default: {
10+
task: {
11+
update: vi.fn(),
12+
},
13+
},
14+
}));
15+
16+
import db from '$lib/server/database';
17+
18+
describe('updateTask', () => {
19+
const mockDb = db as unknown as { task: { update: ReturnType<typeof vi.fn> } };
20+
21+
beforeEach(() => {
22+
vi.clearAllMocks();
23+
});
24+
25+
describe('successful case', () => {
26+
test('returns undefined when task is updated successfully', async () => {
27+
mockDb.task.update.mockResolvedValue({
28+
id: '1',
29+
task_id: 'abc450_d',
30+
grade: TaskGrade.Q1,
31+
});
32+
33+
const result = await updateTask('abc450_d', TaskGrade.Q2);
34+
35+
expect(result).toBeUndefined();
36+
expect(mockDb.task.update).toHaveBeenCalledWith({
37+
where: { task_id: 'abc450_d' },
38+
data: { grade: TaskGrade.Q2 },
39+
});
40+
});
41+
});
42+
43+
describe('error cases', () => {
44+
test('returns null when task is not found (P2025)', async () => {
45+
const error = new Prisma.PrismaClientKnownRequestError(
46+
'An operation failed because it depends on one or more records that were required but not found. Record to update not found.',
47+
{ code: 'P2025', clientVersion: '5.0.0' },
48+
);
49+
50+
mockDb.task.update.mockRejectedValue(error);
51+
52+
const result = await updateTask('nonexistent_task', TaskGrade.Q1);
53+
54+
expect(result).toBeNull();
55+
});
56+
57+
test('re-throws non-P2025 errors', async () => {
58+
const error = new Error('Database connection error');
59+
mockDb.task.update.mockRejectedValue(error);
60+
61+
await expect(updateTask('abc450_d', TaskGrade.Q1)).rejects.toThrow(
62+
'Database connection error',
63+
);
64+
});
65+
});
66+
});

0 commit comments

Comments
 (0)