From 08c2adf13b31a6cbcf6e7c063c31bb82be626adb Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Mon, 6 Apr 2026 09:15:25 +0000 Subject: [PATCH 01/46] docs: Add plan and review for PR #3316 post-merge fixes Includes comprehensive implementation plan for quality improvements (tests, UI/UX, DB consistency) and detailed code review findings. Co-Authored-By: Claude Haiku 4.5 --- .../2026-03-26/pr-3316-review/plan.md | 1359 +++++++++++++++++ .../2026-03-26/pr-3316-review/review.md | 133 ++ 2 files changed, 1492 insertions(+) create mode 100644 docs/dev-notes/2026-03-26/pr-3316-review/plan.md create mode 100644 docs/dev-notes/2026-03-26/pr-3316-review/review.md diff --git a/docs/dev-notes/2026-03-26/pr-3316-review/plan.md b/docs/dev-notes/2026-03-26/pr-3316-review/plan.md new file mode 100644 index 000000000..a379e4df5 --- /dev/null +++ b/docs/dev-notes/2026-03-26/pr-3316-review/plan.md @@ -0,0 +1,1359 @@ +# PR #3316 マージ後修正 実装計画 + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** `review.md` に記載されたマージ後修正タスク(#1〜#13、#優先度中テスト)を実施し、コード品質・テスト・UI/UX・DB整合性を改善する。 + +**Architecture:** 低リスク(テスト・定数)→ 中リスク(UI・サービス)→ 高リスク(DB migration)の順で実施。`Pct → percentage` リネーム(#6)は現コードベースに `Pct` が存在しないため実施不要(既解決)。並行投票対策(#14)は優先度低・将来対応のためスコープ外。 + +**Tech Stack:** SvelteKit 2 + Svelte 5 Runes, TypeScript, Prisma, Vitest, Playwright, Flowbite Svelte + +--- + +## 対象ファイル一覧 + +| 操作 | ファイル | 理由 | +| -------------- | ------------------------------------------------------------------------- | ----------------------------------------------------------------------------------- | +| 修正 | `src/features/account/services/atcoder_verification.test.ts` | `vi.stubEnv` パターンへ移行(#11) | +| 新規 | `src/test/lib/services/users.test.ts` | `getUser` / `getUserById` / `deleteUser` のテスト追加(#10) | +| 修正 | `src/features/votes/services/vote_grade.ts` | `MIN_VOTES_FOR_STATISTICS` を export + `upsertVoteGradeTables` 戻り値追加(#4, #5) | +| 修正 | `src/features/votes/actions/vote_actions.ts` | `upsertVoteGradeTables` 戻り値を利用(#4) | +| 修正 | `src/routes/votes/[slug]/+page.svelte` | `MIN_VOTES_FOR_STATISTICS` 参照に変更(#5) | +| 修正 | `src/routes/(admin)/vote_management/+page.server.ts` | `updateTask` に try/catch 追加(#7) | +| 修正 | `src/lib/components/SubmissionButton.svelte` | `loading` prop 追加(#8a) | +| 修正 | `src/lib/components/FormWrapper.svelte` | `isSubmitting` context 追加(#8b) | +| 修正 | `src/features/account/components/settings/AtCoderVerificationForm.svelte` | ボタン間隔(#8c)+ Flowbite Clipboard 導入(#8d) | +| 新規 | `src/features/votes/services/vote_api.ts` | `fetchMyVote` / `submitVote` / `fetchMedianVote` 抽出(#1, #2) | +| 新規 | `src/features/votes/services/vote_api.test.ts` | vote_api.ts の単体テスト(#2) | +| 修正 | `src/features/votes/components/VotableGrade.svelte` | vote_api.ts 利用 + AbortController 連打対策(#1, #2, #3) | +| 修正 | `e2e/votes.spec.ts` | セレクタ修正 + 初期状態対応(#9) | +| 新規 migration | `prisma/migrations/.../migration.sql` | CHECK 制約追加(#12, #13) | +| 修正 | `prisma/schema.prisma` | コメント追加(#12, #13) | +| 修正 | `prisma/ERD.md` | CHECK 制約ドキュメント(#12, #13) | +| 修正 | `.claude/rules/svelte-components.md` | SSR 安全性・`{#each}` キー式ルール追加 | +| 修正 | `.claude/rules/prisma-db.md` | FK @relation・DB 値域制約ルール追加 | +| 修正 | `.claude/rules/coding-style.md` | load関数 try-catch・auth 監査・success フラグ・Dead Code ルール追加 | +| 修正 | `.claude/rules/testing.md` | `vi.stubEnv` ルール追加 | + +--- + +## Phase 1: テスト品質改善 + +### Task 1: vi.stubEnv パターンへ移行(atcoder_verification.test.ts) + +**Files:** + +- Modify: `src/features/account/services/atcoder_verification.test.ts:1-60` + +- [ ] **Step 1: テスト実行(修正前の失敗確認は不要。現在パスしていることを確認)** + +```bash +pnpm test:unit -- atcoder_verification +``` + +Expected: 全テスト PASS + +- [ ] **Step 2: `beforeEach` / `afterEach` の手動 `process.env` を `vi.stubEnv` に置換** + +ファイル冒頭の import に `afterAll` を追加し、以下のように変更する: + +```typescript +// 変更前 +beforeEach(() => { + process.env.CONFIRM_API_URL = SAMPLE_API_URL; +}); + +afterEach(() => { + delete process.env.CONFIRM_API_URL; +}); + +// 変更後 +beforeEach(() => { + vi.stubEnv('CONFIRM_API_URL', SAMPLE_API_URL); +}); + +afterEach(() => { + vi.unstubAllEnvs(); +}); +``` + +- [ ] **Step 3: テスト再実行** + +```bash +pnpm test:unit -- atcoder_verification +``` + +Expected: 全テスト PASS(11件) + +- [ ] **Step 4: コミット** + +```bash +git add src/features/account/services/atcoder_verification.test.ts +git commit -m "test: migrate process.env manual stubs to vi.stubEnv in atcoder_verification tests" +``` + +--- + +### Task 2: users.ts 単体テスト追加 + +**Files:** + +- Create: `src/test/lib/services/users.test.ts` + +- [ ] **Step 1: テストファイルを作成(実装なし・FAIL 確認用)** + +```typescript +import { describe, test, expect, vi, beforeEach } from 'vitest'; +import type { User } from '@prisma/client'; + +vi.mock('$lib/server/database', () => ({ + default: { + user: { + findUnique: vi.fn(), + delete: vi.fn(), + }, + }, +})); + +import db from '$lib/server/database'; +import { getUser, getUserById, deleteUser } from '$lib/services/users'; + +const mockFindUnique = vi.mocked(db.user.findUnique); +const mockDelete = vi.mocked(db.user.delete); + +// Type-safe fixture (no `as any`) +type UserWithAccount = User & { + atCoderAccount: { + userId: string; + handle: string; + isValidated: boolean; + validationCode: string; + createdAt: Date; + updatedAt: Date; + }; +}; + +const SAMPLE_USER: UserWithAccount = { + id: 'user-abc123', + username: 'testuser', + atCoderAccount: { + userId: 'user-abc123', + handle: 'testuser_ac', + isValidated: true, + validationCode: 'code-xyz', + createdAt: new Date('2024-01-01'), + updatedAt: new Date('2024-01-01'), + }, +} as UserWithAccount; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('getUser', () => { + test('returns user with atCoderAccount when found', async () => { + mockFindUnique.mockResolvedValueOnce(SAMPLE_USER); + + const result = await getUser('testuser'); + + expect(result).toEqual(SAMPLE_USER); + expect(mockFindUnique).toHaveBeenCalledWith({ + where: { username: 'testuser' }, + include: { atCoderAccount: true }, + }); + }); + + test('returns null when user is not found', async () => { + mockFindUnique.mockResolvedValueOnce(null); + + const result = await getUser('nonexistent'); + + expect(result).toBeNull(); + }); +}); + +describe('getUserById', () => { + test('returns user with atCoderAccount when found', async () => { + mockFindUnique.mockResolvedValueOnce(SAMPLE_USER); + + const result = await getUserById('user-abc123'); + + expect(result).toEqual(SAMPLE_USER); + expect(mockFindUnique).toHaveBeenCalledWith({ + where: { id: 'user-abc123' }, + include: { atCoderAccount: true }, + }); + }); + + test('returns null when user is not found', async () => { + mockFindUnique.mockResolvedValueOnce(null); + + const result = await getUserById('nonexistent-id'); + + expect(result).toBeNull(); + }); +}); + +describe('deleteUser', () => { + test('deletes user and subsequent getUser returns null', async () => { + mockDelete.mockResolvedValueOnce(SAMPLE_USER); + mockFindUnique.mockResolvedValueOnce(null); + + const deleteResult = await deleteUser('testuser'); + + expect(deleteResult).toEqual(SAMPLE_USER); + expect(mockDelete).toHaveBeenCalledWith({ where: { username: 'testuser' } }); + + // Verify that the user is actually deleted (subsequent getUser returns null) + const getResult = await getUser('testuser'); + expect(getResult).toBeNull(); + }); +}); +``` + +- [ ] **Step 2: テスト実行(FAIL 確認)** + +```bash +pnpm test:unit -- src/test/lib/services/users +``` + +Expected: テストファイルが存在しないためエラー(または関数が存在しないため FAIL) + +- [ ] **Step 3: テスト実行(PASS 確認)** + +実装は `src/lib/services/users.ts` に既存(変更不要)。 + +```bash +pnpm test:unit -- src/test/lib/services/users +``` + +Expected: 5件 PASS + +- [ ] **Step 4: コミット** + +```bash +git add src/test/lib/services/users.test.ts +git commit -m "test: add unit tests for getUser, getUserById, and deleteUser" +``` + +--- + +## Phase 2: サービス・ルート修正 + +### Task 3: MIN_VOTES_FOR_STATISTICS を export し hardcode を除去 + +**Files:** + +- Modify: `src/features/votes/services/vote_grade.ts:27` +- Modify: `src/routes/votes/[slug]/+page.svelte:46` + +- [ ] **Step 1: vote_grade.ts の定数を export に変更** + +```typescript +// 変更前(line 27) +const MIN_VOTES_FOR_STATISTICS = 3; + +// 変更後 +export const MIN_VOTES_FOR_STATISTICS = 3; +``` + +- [ ] **Step 2: votes/[slug]/+page.svelte の hardcode を置換** + +import を追加し、Tooltip テキストを定数参照に変更: + +```typescript +// script 内の import に追加 +import { MIN_VOTES_FOR_STATISTICS } from '$features/votes/services/vote_grade'; +``` + +```svelte + +3票以上集まると中央値が暫定グレードとして一覧表に反映されます。 + + +{MIN_VOTES_FOR_STATISTICS}票以上集まると中央値が暫定グレードとして一覧表に反映されます。 +``` + +- [ ] **Step 3: 型チェック** + +```bash +pnpm check +``` + +Expected: エラーなし + +- [ ] **Step 4: コミット** + +```bash +git add src/features/votes/services/vote_grade.ts src/routes/votes/[slug]/+page.svelte +git commit -m "refactor: export MIN_VOTES_FOR_STATISTICS and remove hardcoded '3' in votes page" +``` + +--- + +### Task 4: upsertVoteGradeTables の戻り値を { success: true } に変更 + +**Files:** + +- Modify: `src/features/votes/services/vote_grade.ts:30-34` +- Modify: `src/features/votes/actions/vote_actions.ts:54-58` + +- [ ] **Step 1: vote_grade.ts の関数シグネチャと戻り値を変更** + +```typescript +// 変更前 +export async function upsertVoteGradeTables( + userId: string, + taskId: string, + grade: TaskGrade, +): Promise { + await prisma.$transaction(async (tx) => { + // ... + }); +} + +// 変更後 +export async function upsertVoteGradeTables( + userId: string, + taskId: string, + grade: TaskGrade, +): Promise<{ success: true }> { + await prisma.$transaction(async (tx) => { + // ... (内容変更なし) + }); + return { success: true }; +} +``` + +- [ ] **Step 2: vote_actions.ts でサービスの戻り値を返すよう変更** + +```typescript +// 変更前 +try { + await upsertVoteGradeTables(userId, taskId, grade); +} catch (error) { + console.error('Failed to vote absolute grade: ', error); + return fail(INTERNAL_SERVER_ERROR, { message: 'Failed to record vote.' }); +} +// (暗黙の undefined return → SvelteKit が success として扱う) + +// 変更後 +try { + return await upsertVoteGradeTables(userId, taskId, grade); +} catch (error) { + console.error('Failed to vote absolute grade: ', error); + return fail(INTERNAL_SERVER_ERROR, { message: 'Failed to record vote.' }); +} +``` + +- [ ] **Step 3: 型チェック** + +```bash +pnpm check +``` + +Expected: エラーなし + +- [ ] **Step 4: コミット** + +```bash +git add src/features/votes/services/vote_grade.ts src/features/votes/actions/vote_actions.ts +git commit -m "refactor: make upsertVoteGradeTables return { success: true } for explicit action response" +``` + +--- + +### Task 5: vote_management の updateTask に 404 エラー処理を追加 + +**Files:** + +- Modify: `src/routes/(admin)/vote_management/+page.server.ts:54` + +- [ ] **Step 1: `updateTask` 呼び出しを try/catch で囲む** + +Prisma の `P2025`(RecordNotFound)を明示的に処理する。import を追加: + +```typescript +import { Prisma } from '@prisma/client'; +``` + +アクション内を変更: + +```typescript +// 変更前(line 54-55) +await updateTask(taskId, grade as TaskGrade); +return { success: true }; + +// 変更後 +try { + await updateTask(taskId, grade as TaskGrade); +} catch (error) { + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2025') { + return { success: false, message: `Not found task: ${taskId}` }; + } + + throw error; +} +return { success: true }; +``` + +- [ ] **Step 2: 型チェック** + +```bash +pnpm check +``` + +Expected: エラーなし + +- [ ] **Step 3: コミット** + +```bash +git add src/routes/(admin)/vote_management/+page.server.ts +git commit -m "fix: handle RecordNotFound error in setTaskGrade action for vote_management" +``` + +--- + +## Phase 3: UI コンポーネント改善 + +### Task 6: SubmissionButton loading prop + FormWrapper isSubmitting context + +**Files:** + +- Modify: `src/lib/components/SubmissionButton.svelte` +- Modify: `src/lib/components/FormWrapper.svelte` + +- [ ] **Step 1: SubmissionButton に loading prop を追加し getContext で自動取得** + +`SubmissionButton.svelte` の内容を差し替え: + +```svelte + + + +``` + +- [ ] **Step 2: FormWrapper に isSubmitting state と setContext を追加** + +`FormWrapper.svelte` の内容を差し替え: + +```svelte + + +
+ {#if children} + {@render children()} + {/if} +
+``` + +- [ ] **Step 3: 型チェック** + +```bash +pnpm check +``` + +Expected: エラーなし + +- [ ] **Step 4: コミット** + +```bash +git add src/lib/components/SubmissionButton.svelte src/lib/components/FormWrapper.svelte +git commit -m "feat: add loading state to SubmissionButton via FormWrapper context (prevent double-submit)" +``` + +--- + +### Task 7: AtCoderVerificationForm ボタン間隔追加 + Flowbite Clipboard 導入 + +**Files:** + +- Modify: `src/features/account/components/settings/AtCoderVerificationForm.svelte` + +- [ ] **Step 1: ボタン間隔追加(8c)とリセット FormWrapper に `marginTop` を指定** + +`status === 'generated'` ブロックの2つ目の `FormWrapper` を変更(現在 `marginTop=""` が未指定で隙間なし): + +```svelte + + + + + +``` + +- [ ] **Step 2: Flowbite Clipboard 導入(8d)** + +`AtCoderVerificationForm.svelte` の ` -
+ {#if children} {@render children()} {/if} diff --git a/src/lib/components/SubmissionButton.svelte b/src/lib/components/SubmissionButton.svelte index ef389c9fb..e966edee9 100644 --- a/src/lib/components/SubmissionButton.svelte +++ b/src/lib/components/SubmissionButton.svelte @@ -1,12 +1,20 @@ - + From a24be954edacffb9ef598c369b1e466de0184923 Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Mon, 6 Apr 2026 10:01:38 +0000 Subject: [PATCH 09/46] feat: add button gap and replace custom clipboard with Flowbite Clipboard in AtCoderVerificationForm --- .../settings/AtCoderVerificationForm.svelte | 53 +++++-------------- 1 file changed, 14 insertions(+), 39 deletions(-) diff --git a/src/features/account/components/settings/AtCoderVerificationForm.svelte b/src/features/account/components/settings/AtCoderVerificationForm.svelte index 319358776..2100400df 100644 --- a/src/features/account/components/settings/AtCoderVerificationForm.svelte +++ b/src/features/account/components/settings/AtCoderVerificationForm.svelte @@ -1,38 +1,13 @@ {#if status === 'nothing'} @@ -112,19 +81,25 @@