From e1d37a2228eeb592e8fbe2ebd4fabb6ac6a4130b Mon Sep 17 00:00:00 2001 From: river0525 <0525sotaro@gmail.com> Date: Mon, 30 Mar 2026 10:44:09 +0900 Subject: [PATCH 1/2] fix: address CodeRabbit round 4 findings - +page.server.ts: delete action now redirects to /login after successful deletion - vote_actions.ts: separate locals.user existence check (500) from is_validated check (403) to distinguish missing user data from unverified state - e2e/votes.spec.ts: remove redundant voteForm.toBeVisible() (guaranteed by or() wait) - .claude/rules/testing.md: remove duplicate Prisma-mock note (appeared twice) - .claude/rules/testing-e2e.md: simplify toBeChecked({checked:true}) -> toBeChecked(); collapse double .locator() chain into single selector '.container nav' - .claude/rules/sveltekit.md: fix naming inconsistency in load() example (both Bad and Good now use camelCase-based field names, consistent with Prisma model) Skip: locals.user?.is_validated -> session?.user.is_validated in page.server.ts files (session.user type only has userId/username/role; is_validated lives on locals.user) Co-Authored-By: Claude Sonnet 4.6 --- .claude/rules/sveltekit.md | 10 +++++----- .claude/rules/testing-e2e.md | 4 ++-- .claude/rules/testing.md | 2 -- e2e/votes.spec.ts | 5 ----- src/features/votes/actions/vote_actions.ts | 8 +++++++- src/routes/users/edit/+page.server.ts | 9 ++------- 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/.claude/rules/sveltekit.md b/.claude/rules/sveltekit.md index 058816523..ffdcbaf6c 100644 --- a/.claude/rules/sveltekit.md +++ b/.claude/rules/sveltekit.md @@ -84,12 +84,12 @@ group them as an object rather than flattening to top-level keys. Apply default values at this boundary so the page component typically does not need to handle `undefined`. ```typescript -// Bad: flat, scattered across top-level keys -atcoder_username: user?.atCoderAccount?.handle ?? '', -atcoder_validationcode: user?.atCoderAccount?.validationCode ?? '', -is_validated: user?.atCoderAccount?.isValidated ?? false, +// Bad: flat top-level keys (legacy snake_case naming, scattered fields) +atcoder_handle: user?.atCoderAccount?.handle ?? '', +atcoder_validation_code: user?.atCoderAccount?.validationCode ?? '', +atcoder_is_validated: user?.atCoderAccount?.isValidated ?? false, -// Good: grouped by model, defaults absorbed here +// Good: grouped by model with camelCase; defaults absorbed here atCoderAccount: { handle: user?.atCoderAccount?.handle ?? '', validationCode: user?.atCoderAccount?.validationCode ?? '', diff --git a/.claude/rules/testing-e2e.md b/.claude/rules/testing-e2e.md index 9c394361a..89163fa71 100644 --- a/.claude/rules/testing-e2e.md +++ b/.claude/rules/testing-e2e.md @@ -83,7 +83,7 @@ const toggleInput = page.locator('input[aria-label=""]'); const toggleLabel = page.locator('label:has(input[aria-label=""])'); await toggleLabel.click(); -await expect(toggleInput).toBeChecked({ checked: true }); +await expect(toggleInput).toBeChecked(); ``` The same pattern applies to any Flowbite component that visually overlays its native input (e.g. `Checkbox`, `Radio`). @@ -97,7 +97,7 @@ When the navbar and page body both contain a link or button with the same text ( await page.getByRole('link', { name: 'グレード投票' }).click(); // Good: scoped to page content only -await page.locator('.container').locator('nav').getByRole('link', { name: 'グレード投票' }).click(); +await page.locator('.container nav').getByRole('link', { name: 'グレード投票' }).click(); await page.locator('.container').getByRole('link', { name: 'ログイン' }).click(); ``` diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 24f663018..58ccedbb9 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -118,8 +118,6 @@ try { } ``` -This is not needed for standard service unit tests that use Prisma mocks. - ### File Split for Testability When a service file mixes DB operations and pure functions, split it into two files: diff --git a/e2e/votes.spec.ts b/e2e/votes.spec.ts index 17bc86914..5fa1f0685 100644 --- a/e2e/votes.spec.ts +++ b/e2e/votes.spec.ts @@ -131,11 +131,6 @@ test.describe('vote detail page (/votes/[slug])', () => { const isUnverified = await unverifiedMessage.isVisible(); test.skip(isUnverified, 'test user is not AtCoder-verified'); - // Explicit check: voteForm is already guaranteed visible by the or() wait above, - // but this documents the expected state for verified users. - await expect(voteForm).toBeVisible({ - timeout: TIMEOUT, - }); // The grade buttons should include Q11 (11Q) await expect(page.getByRole('button', { name: '11Q' })).toBeVisible({ timeout: TIMEOUT }); }); diff --git a/src/features/votes/actions/vote_actions.ts b/src/features/votes/actions/vote_actions.ts index 41ebd438c..679f4a288 100644 --- a/src/features/votes/actions/vote_actions.ts +++ b/src/features/votes/actions/vote_actions.ts @@ -28,7 +28,13 @@ export const voteAbsoluteGrade = async ({ }); } - if (!locals.user?.is_validated) { + if (!locals.user) { + return fail(INTERNAL_SERVER_ERROR, { + message: 'ユーザー情報の取得に失敗しました。', + }); + } + + if (!locals.user.is_validated) { return fail(FORBIDDEN, { message: 'AtCoderアカウントの認証が必要です。', }); diff --git a/src/routes/users/edit/+page.server.ts b/src/routes/users/edit/+page.server.ts index 77e2f44a9..c61caa447 100644 --- a/src/routes/users/edit/+page.server.ts +++ b/src/routes/users/edit/+page.server.ts @@ -136,17 +136,12 @@ export const actions: Actions = { try { await userService.deleteUser(username); locals.auth.setSession(null); // remove cookie - - return { - success: true, - username, - message_type: 'green', - message: 'Successfully deleted.', - }; } catch (error) { console.error('Failed to delete user account', error); return fail(INTERNAL_SERVER_ERROR, { message: 'Failed to delete account.' }); } + + redirect(302, '/login'); }, }; From 2fbb4598c41eb14595bec254999e96683a97ed72 Mon Sep 17 00:00:00 2001 From: river0525 <0525sotaro@gmail.com> Date: Mon, 30 Mar 2026 11:09:15 +0900 Subject: [PATCH 2/2] fix: address CodeRabbit round 5 findings Co-Authored-By: Claude Sonnet 4.6 --- .claude/rules/testing-e2e.md | 2 +- .claude/rules/testing.md | 4 ++-- .env.example | 1 + src/routes/problems/+page.svelte | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.claude/rules/testing-e2e.md b/.claude/rules/testing-e2e.md index 89163fa71..efbcaa1ab 100644 --- a/.claude/rules/testing-e2e.md +++ b/.claude/rules/testing-e2e.md @@ -86,7 +86,7 @@ await toggleLabel.click(); await expect(toggleInput).toBeChecked(); ``` -The same pattern applies to any Flowbite component that visually overlays its native input (e.g. `Checkbox`, `Radio`). +The same pattern may apply to similar Flowbite components that visually overlay their native input (e.g. `Checkbox`, `Radio`). Verify the component structure before applying this workaround. ## Strict Mode: Scope Locators to the Content Area diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 58ccedbb9..ca79c1e07 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -105,7 +105,7 @@ Extract `mockFindUnique`, `mockFindMany`, and `mockCount` as the standard trio f ### Cleanup for Integration Tests and Tests with Real Side Effects -This does not apply to standard service layer unit tests that use Prisma mocks. +> **Note:** This does not apply to standard service layer unit tests that use Prisma mocks. If a test performs real DB mutations, file system changes, external API calls, or other stateful side effects that persist beyond the test (e.g., integration tests, seed scripts), wrap assertions in `try/finally` — a failing assertion skips cleanup and contaminates later tests: @@ -133,7 +133,7 @@ E2E tests are complementary to, not a substitute for, unit tests. Add Vitest uni You may omit a component-level Vitest test when **both** conditions hold: -1. The component is template-only (no logic beyond prop bindings and simple `{#if}`/`{#each}` blocks that only render — no inline function calls, ternaries with side effects, derived computations, or nested logic) +1. The component is template-only (no logic beyond prop bindings and simple `{#if}`/`{#each}` blocks that only render — no inline function calls, ternaries with complex conditional logic, derived computations, or nested logic) 2. The component's rendering paths are covered by E2E tests When a component contains extracted logic (e.g. derived values, event handlers, utility calls), add unit tests for that logic in the nearest `utils/` file instead of testing the component directly. diff --git a/.env.example b/.env.example index 3f811d189..1896accd0 100644 --- a/.env.example +++ b/.env.example @@ -1,3 +1,4 @@ # AtCoder affiliation confirmation API endpoint (NoviSteps organization crawler) +# WARNING: Do NOT commit the actual endpoint URL. Set it in your local .env file. # See team documentation for the actual URL. CONFIRM_API_URL=https://your-confirm-api-endpoint.example.com/confirm diff --git a/src/routes/problems/+page.svelte b/src/routes/problems/+page.svelte index ecf9aef2a..4f618008b 100644 --- a/src/routes/problems/+page.svelte +++ b/src/routes/problems/+page.svelte @@ -20,7 +20,7 @@ let { data } = $props(); - let taskResults: TaskResults = $derived(data.taskResults.sort(compareByContestIdAndTaskId)); + const taskResults: TaskResults = $derived(data.taskResults.sort(compareByContestIdAndTaskId)); const isAdmin = $derived(data.isAdmin); const isLoggedIn = $derived(data.isLoggedIn);