From b6f6d97c599bc5cf7f38b0072e2f93004dd3cff4 Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Thu, 19 Mar 2026 05:03:57 +0000 Subject: [PATCH 01/14] docs: Add refactoring plan for workbook list feature (#3280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 問題集一覧のリファクタリング計画(Phase 0-8)を追加。N+1クエリ修正、サービス層からSvelteKit依存除去、コンポーネント分割などの実装方針を記録。 Co-Authored-By: Claude Sonnet 4.6 --- .../refactor-for-workbook-list/plan.md | 277 ++++++++++++++++++ 1 file changed, 277 insertions(+) create mode 100644 docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md diff --git a/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md b/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md new file mode 100644 index 000000000..b23051413 --- /dev/null +++ b/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md @@ -0,0 +1,277 @@ +# Refactoring Plan: 問題集一覧機能 (#3280) + +## Context + +Issue #3269(管理者指定の並び順で問題集を表示)をスムーズに実装するための前工程。 +現状、`+page.svelte` の肥大化・`+page.server.ts` の N+1 クエリ・サービス層への SvelteKit 依存・テスト不足が課題。 +コード整理と責務分離を行い、#3269 への接続コストを下げる。 + +--- + +## Findings + +### テスト欠落(最優先) + +- `services/workbooks.ts` (202行) — テストなし +- `services/workbook_tasks.ts` (18行) — テストなし + +### +page.server.ts + +- **N+1クエリ**: `getWorkBooks()` → ループ内で `getUserById()` (N回) → `include: { user }` で1クエリ化可能 +- try/catch が `workbooksWithAuthors` を囲っていない(著者取得失敗時も 500 が返らない) +- `console.log('form -> actions -> delete')` デバッグログ残留 + +### +page.svelte + +- `getWorkBooksByType()` — TODO コメントあり、utils に移すべき純粋関数 +- `fetchTaskResultsWithWorkBookId()` — 複雑なロジックがコンポーネント内に埋め込まれている + +### services/workbooks.ts + +- `getWorkbookWithAuthor()` がサービス層で SvelteKit の `error()` を呼ぶ(テスト不能、責務混在) +- `isExistingWorkBook()` が存在確認のために全フィールドを `findUnique` している(`count` で十分) + +### WorkBookList.svelte + +- `loggedInUser: any` — 型安全性の欠如(使用フィールドは `.id` と `.role` のみ) +- `$derived(() => countReadableWorkbooks(...))` — Svelte 5 の誤用、`$derived(countReadableWorkbooks(...))` が正しい + +### WorkBookBaseTable.svelte + +- FIXME コメント: 「問題集の種類別にコンポーネントを分ける」 +- CURRICULUM / SOLUTION / CREATED_BY_USER でヘッダー・ボディ両方に if 分岐が散在 + +### utils/workbooks.ts テストギャップ + +- `calcWorkBookGradeModes` のタイブレーク動作(同頻度グレードの場合)が未テスト + +--- + +## 実装フェーズ + +### Phase 0: 未テストサービスへのテスト追加(純粋追加、リスクなし) + +- [ ] `services/workbook_tasks.test.ts` を新規作成 + - `getWorkBookTasks`: 通常ケース、空配列、comment フィールドの取り扱い + - `validateRequiredFields`: 正常ケース、taskId 欠損、priority 欠損(index 0/中/末/負/小数)、空配列 + - **注意**: `validateRequiredFields` は `!task.priority` を使用しているが `priority === 0` は `false` になる潜在バグ → テストで明確化してから修正を判断 + +### Phase 1: +page.svelte から utils へ純粋関数を抽出(低リスク) + +- [ ] `utils/workbooks.ts` に `getWorkBooksByType(workbooks, type)` を追加(TODO コメント対応) + - テスト追加(通常フィルタ、空配列、type が一致しない場合) +- [ ] `utils/workbooks.ts` に `buildTaskResultsByWorkBookId(workbooks, taskResultsByTaskId)` を追加 + - 現在 +page.svelte の `fetchTaskResultsWithWorkBookId()` に相当 + - テスト追加(task 結果あり/なし、空配列) +- [ ] `+page.svelte` を更新: 両関数を import に置き換え + +### Phase 2: +page.server.ts の N+1 修正とクリーンアップ(中リスク) + +- [ ] `services/workbooks.ts` に `getWorkBooksWithAuthors()` を追加 + - `include: { user: { select: { username: true } }, workBookTasks: { orderBy: { priority: 'asc' } } }` を使用 + - 戻り値型 `WorkbooksWithAuthors` を `types/workbook.ts` に追加(`authorName: string` を含む) + - 著者削除済みの場合は `user?.username ?? 'unknown'` で対応 +- [ ] `+page.server.ts` の `load()` を更新: + - `getWorkBooks()` + N+1 ループ → `getWorkBooksWithAuthors()` に置き換え + - try/catch の範囲を拡大(workbooks 取得も含める) +- [ ] CRUD アクションのログを「受信時」→「成功後」に移動(3ファイル) + +**判断根拠**: 現在の `console.log('form -> actions -> ...')` はバリデーション**前**(アクション受信時)に発火するため、不正なリクエストでも記録される。また「どのリソースを誰が操作したか」が分からず本番デバッグに役立たない。成功を確認できる位置に移動し、識別情報を含めることで意味のあるログにする。 + +```ts +// Before(受信時、情報量が低い) +console.log('form -> actions -> delete'); +// ... バリデーション ... +await workBooksCrud.deleteWorkBook(workBookId); + +// After(操作成功後、識別情報を含む) +await workBooksCrud.deleteWorkBook(workBookId); +console.log(`Deleted workbook ${workBookId} by user ${loggedInUser?.id}`); +``` + +対象ファイルと成功後ログの内容: + +| ファイル | ログ内容 | +|----------|---------| +| `routes/workbooks/+page.server.ts` (delete) | `Deleted workbook ${workBookId} by user ${loggedInUser?.id}` | +| `routes/workbooks/create/+page.server.ts` | `Created workbook "${workBook.title}" by user ${author.id}` | +| `routes/workbooks/edit/[slug]/+page.server.ts` | `Updated workbook ${workBookId}`(edit action に `locals` がなくユーザー情報なし) | + +### Phase 3: サービス層から SvelteKit error() を除去(中リスク) + +`getWorkbookWithAuthor()` の error() 移動 — 方針: + +- スラッグバリデーション(`parseWorkBookId/parseWorkBookUrlSlug`)は**入力バリデーション = ルートの責務** +- サービスは DB アクセスのみ、存在しない場合は `null` を返す +- 2種類のエラー(BAD_REQUEST vs NOT_FOUND)をルートが区別できる構造にする + +``` +// Before: サービスが error() を直接呼ぶ +export async function getWorkbookWithAuthor(slug) { + if (workBookId === null && workBookUrlSlug === null) { + error(BAD_REQUEST, '...'); // ← SvelteKit 依存 + } + if (workBook === null) { + error(NOT_FOUND, '...'); // ← SvelteKit 依存 + } +} + +// After: ルートが責務を持つ +// service: null を返すだけ +// route: slug バリデーション → null チェック → error() 呼び出し +const workBookId = parseWorkBookId(slug); +if (!workBookId) { error(BAD_REQUEST, '...'); } +const workbook = await getWorkbookWithAuthor(slug); +if (!workbook) { error(NOT_FOUND, '...'); } +``` + +- [ ] `getWorkbookWithAuthor()` から `error()` 呼び出しを削除、`null` を返すように変更 +- [ ] 呼び出し元のルートハンドラーを更新(`src/routes/workbooks/[slug]/+page.server.ts` 等): + - slug バリデーションを呼び出し元に移動 + - null チェック後に `error(BAD_REQUEST/NOT_FOUND, ...)` を呼び出す +- [ ] `isExistingWorkBook()` を `db.workBook.count({ where: { id } }) > 0` に変更 + +### Phase 4: services/workbooks.ts の統合テスト追加(Phase 3 後) + +- [ ] `services/workbooks.test.ts` を新規作成(`@quramy/prisma-fabbrica` ファクトリー使用) + - `getWorkBook`: 存在する/しない + - `getWorkBooksWithAuthors`: 著者あり/著者削除済み + - `createWorkBook`: 正常、重複スラッグ + - `updateWorkBook`: 正常、存在しない ID + - `deleteWorkBook`: 正常、存在しない ID + +### Phase 5: WorkBookList.svelte 型修正と Svelte 5 パターン修正(低リスク) + +- [ ] `loggedInUser: any` → 使用フィールド(`id: string`, `role: Roles`)の最小 interface を定義 + - `$lib/types/user.ts` の既存型を確認して再利用 +- [ ] `$derived(() => countReadableWorkbooks(...))` を `$derived(countReadableWorkbooks(...))` に修正(2箇所) + +### Phase 6: WorkBookBaseTable.svelte を3種コンポーネントに分割(中リスク) + +ユーザー方針: 「3種類コンポーネントをベースに、if 文のところは interface で分岐をなくす」 + +**設計判断: 完全分離 vs 基底+差分** + +`WorkBookBaseTable.svelte` を「共通の基底コンポーネント」として残す案もあるが、**完全分離を採用する**。 + +| 方針 | メリット | デメリット | +| -------------------- | ---------------------------------------- | ----------------------------------------------- | +| **完全分離(採用)** | 各ファイルが自己完結、開けば全体が見える | `` 等のテンプレート構造が3箇所に存在 | +| 基底 + 差分 | テンプレートの行数が少ない | 「共通はどこ?差分はどこ?」と2層読み回しが必要 | + +分割後の1ファイルは概算 50〜70行。共通の**ロジック**(`AcceptedCounter`, `ThermometerProgressBar` 等)は import で共有するため実質的な重複はなく、重複するのはテンプレート構造だけ → 許容範囲。`WorkBookBaseTable.svelte` はリネームせず削除する。 + +**命名判断: `WorkBook` プレフィックスを省略する** + +同ディレクトリの `WorkBookList.svelte` や `WorkbookTabItem.svelte` は routes 側(`+page.svelte`)からもインポートされるため、`WorkBook` プレフィックスが文脈の補足として機能する。一方、3分割コンポーネントは workbooks feature 内でしか使われず、`src/features/workbooks/components/list/` というパスがすでに "workbook" の文脈を提供している。プレフィックスは冗長になるため省略する。 + +``` +// 省略なし(冗長) +import CurriculumWorkbookTable from '.../workbooks/components/list/CurriculumWorkbookTable.svelte' + +// 省略あり(採用) +import CurriculumTable from '.../workbooks/components/list/CurriculumTable.svelte' +``` + +- [ ] 共通 `interface WorkbookTableProps` を定義(3コンポーネントが同じシグネチャを持つ) + - `workbooks`, `workbookGradeModes`, `userId`, `role`, `taskResults` を共通 props に +- [ ] 3コンポーネントを作成(全て `WorkbookTableProps` を実装): + - `CurriculumTable.svelte` — グレード + タイトル列 + - `SolutionTable.svelte` — タイトル列(padding 広め) + - `CreatedByUserTable.svelte` — 作者 + タイトル列 + - 各コンポーネントは同じ `interface Props` を持ち、内部に WorkBookType の if 分岐なし + - 共通列(ProgressBar、AcceptedCounter、CompletedTasks、編集/削除ボタン)は既存コンポーネントをそのまま利用 +- [ ] `WorkBookList.svelte` を更新: `Record` ルックアップで切り替え + +```svelte + + + +{@const TableComponent = tableComponents[workbookType]} + +``` + +- [ ] `WorkBookBaseTable.svelte` を削除 + +### Phase 7: コーナーケーステストの強化(純粋追加) + +- [ ] `utils/workbooks.ts` `calcWorkBookGradeModes`: + - タイブレーク動作のテスト(同頻度グレードが2つある場合、`calcGradeMode` の動作を確認して文書化) +- [ ] 既存テストの `toBeTruthy()`/`toBeFalsy()` を `toBe(true)`/`toBe(false)` に置き換え(coding-style.md 準拠) + +### Phase 8: E2E テスト追加(純粋追加) + +新規ファイル `tests/workbooks_list.test.ts` を作成。`loginAsAdmin` ヘルパーは `workbook_order.test.ts` から抽出して共有する。 + +**アクセス制御** + +- [ ] 未ログインで `/workbooks` にアクセスすると `/login` にリダイレクトされる + +**ログインユーザー(一般)** + +- [ ] カリキュラム・解法別タブが表示される +- [ ] ユーザ作成タブは非管理者には表示されない +- [ ] カリキュラムタブでグレードフィルター(`10Q` → `9Q`)をクリックするとリストが切り替わる +- [ ] 補充問題集が存在するグレードでトグル(`aria-label="Toggle visibility of replenishment workbooks for curriculum"`)をクリックすると補充セクションが表示/非表示になる + +**管理者** + +- [ ] 「新規作成」ボタンが表示される +- [ ] ユーザ作成タブが表示される +- [ ] 問題集行に「編集」リンクと「削除」ボタンが表示される + +**補足**: グレードラベルの文字列(`10Q` 等)は `getTaskGradeLabel()` の戻り値に依存するため、実装時に `workbook_order.test.ts` の既存セレクター(`{ name: '10Q' }`)と一致していることを確認すること。 + +--- + +## 変更ファイル一覧 + +### 新規作成 + +- `src/features/workbooks/services/workbook_tasks.test.ts` +- `src/features/workbooks/services/workbooks.test.ts` +- `src/features/workbooks/components/list/CurriculumTable.svelte` +- `src/features/workbooks/components/list/SolutionTable.svelte` +- `src/features/workbooks/components/list/CreatedByUserTable.svelte` +- `tests/workbooks_list.test.ts` + +### 変更 + +- `src/features/workbooks/services/workbooks.ts` — `getWorkBooksWithAuthors()` 追加、`isExistingWorkBook()` 修正、`getWorkbookWithAuthor()` から error() 除去 +- `src/features/workbooks/types/workbook.ts` — `WorkbooksWithAuthors` 型追加 +- `src/features/workbooks/utils/workbooks.ts` — `getWorkBooksByType()`, `buildTaskResultsByWorkBookId()` 追加 +- `src/features/workbooks/utils/workbooks.test.ts` — 追加テスト +- `src/features/workbooks/components/list/WorkBookList.svelte` — 型修正・Svelte 5 パターン修正・3コンポーネント使用 +- `src/routes/workbooks/+page.svelte` — 関数 import 化 +- `src/routes/workbooks/+page.server.ts` — N+1 修正・ログを成功後に移動 +- `src/routes/workbooks/create/+page.server.ts` — ログを成功後に移動 +- `src/routes/workbooks/edit/[slug]/+page.server.ts` — ログを成功後に移動 +- `src/routes/workbooks/[slug]/+page.server.ts` 等 — error() 呼び出し元更新 + +### 削除 + +- `src/features/workbooks/components/list/WorkBookBaseTable.svelte`(Phase 6 後) + +--- + +## 検証方法 + +```bash +pnpm test:unit # Phase 0, 4, 7 のテスト確認 +pnpm check # Svelte 型チェック(Phase 5, 6) +pnpm lint # ESLint チェック +pnpm test:integration # Phase 8 の E2E テスト確認 +pnpm dev # ローカルで3タブ(カリキュラム/解法別/ユーザ作成)が正常表示されること +``` From db583e5e0170da2dd76b654665e10f1cdc9dbd39 Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Thu, 19 Mar 2026 05:55:11 +0000 Subject: [PATCH 02/14] refactor: Workbook list feature cleanup and component split (#3280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 0-6の実装: - N+1クエリ修正(getWorkBooksWithAuthors追加) - サービス層からSvelteKit error()を除去、nullを返すよう変更 - WorkBookBaseTableをCurriculumTable/SolutionTable/CreatedByUserTableに分割 - WorkBookListをRecordルックアップに変更 - +page.svelte・+page.server.tsを簡略化 - utils/workbooks.tsにgetWorkBooksByType・buildTaskResultsByWorkBookId追加 - テスト追加(workbook_tasks.test.ts、utils/workbooks.test.ts拡充) - Svelte 5 $derivedパターン修正とloggedInUser型修正 - eslint.config.mjsにvarsIgnorePattern追加(デストラクチャの_プレフィックス対応) Co-Authored-By: Claude Sonnet 4.6 --- .claude/rules/svelte-components.md | 8 + AGENTS.md | 1 + .../refactor-for-workbook-list/plan.md | 114 ++++--------- eslint.config.mjs | 5 +- .../components/list/CreatedByUserTable.svelte | 109 +++++++++++++ ...aseTable.svelte => CurriculumTable.svelte} | 54 ++----- .../components/list/SolutionTable.svelte | 101 ++++++++++++ .../components/list/WorkBookList.svelte | 48 ++++-- .../workbooks/services/workbook_tasks.test.ts | 122 ++++++++++++++ src/features/workbooks/services/workbooks.ts | 42 +++-- src/features/workbooks/types/workbook.ts | 3 + .../workbooks/utils/workbooks.test.ts | 151 +++++++++++++++++- src/features/workbooks/utils/workbooks.ts | 58 ++++++- src/routes/workbooks/+page.server.ts | 24 +-- src/routes/workbooks/+page.svelte | 59 ++----- src/routes/workbooks/[slug]/+page.server.ts | 12 +- src/routes/workbooks/create/+page.server.ts | 2 +- .../workbooks/edit/[slug]/+page.server.ts | 21 ++- 18 files changed, 707 insertions(+), 227 deletions(-) create mode 100644 src/features/workbooks/components/list/CreatedByUserTable.svelte rename src/features/workbooks/components/list/{WorkBookBaseTable.svelte => CurriculumTable.svelte} (67%) create mode 100644 src/features/workbooks/components/list/SolutionTable.svelte create mode 100644 src/features/workbooks/services/workbook_tasks.test.ts diff --git a/.claude/rules/svelte-components.md b/.claude/rules/svelte-components.md index 4903254c4..e3f2e15df 100644 --- a/.claude/rules/svelte-components.md +++ b/.claude/rules/svelte-components.md @@ -39,6 +39,14 @@ Referencing `$props()` inside `$state()` initializer triggers "This reference on let count = $state(untrack(() => initialCount)); // intentional: prop is initial seed only ``` +## `$derived` — No Arrow Wrapper + +Use `$derived(expr)`, not `$derived(() => expr)`. The arrow form makes the derived value a _function_, not a reactive value — dependencies may not be tracked and the template call site is confusing. + +## `{@const}` Placement + +`{@const}` must be an **immediate child** of a block statement (`{#if}`, `{#each}`, `{:else}`, `{#snippet}`, etc.). Placing it inside an HTML element is a compile error: + ## `{#snippet}` Placement Define snippets at the **top level**, outside component tags. Inside a tag = named slot = type error: diff --git a/AGENTS.md b/AGENTS.md index 818436226..c21b083bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,6 +69,7 @@ prisma/schema.prisma # Database schema ## Key Conventions - **Svelte 5 Runes**: Use `$props()`, `$state()`, `$derived()` in all new components +- **Service layer**: Services return data or `null`; never call `error()` or `redirect()`. HTTP error translation belongs in the route handler — the service must stay framework-agnostic and unit-testable. - **Server data**: `+page.server.ts` → `+page.svelte` via `data` prop - **Forms**: Superforms + Zod validation - **Tests**: Write tests before implementation (TDD). Use `@quramy/prisma-fabbrica` for factories, Nock for HTTP mocking diff --git a/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md b/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md index b23051413..cfc63d6de 100644 --- a/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md +++ b/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md @@ -49,32 +49,32 @@ Issue #3269(管理者指定の並び順で問題集を表示)をスムーズ ## 実装フェーズ -### Phase 0: 未テストサービスへのテスト追加(純粋追加、リスクなし) +### Phase 0: 未テストサービスへのテスト追加(純粋追加、リスクなし)✅ -- [ ] `services/workbook_tasks.test.ts` を新規作成 +- [x] `services/workbook_tasks.test.ts` を新規作成 - `getWorkBookTasks`: 通常ケース、空配列、comment フィールドの取り扱い - `validateRequiredFields`: 正常ケース、taskId 欠損、priority 欠損(index 0/中/末/負/小数)、空配列 - - **注意**: `validateRequiredFields` は `!task.priority` を使用しているが `priority === 0` は `false` になる潜在バグ → テストで明確化してから修正を判断 + - `priority === 0` は `!task.priority` で falsy になりエラーとなる動作を確認・文書化。priority は実際 1 以上なので意図的動作として許容。 -### Phase 1: +page.svelte から utils へ純粋関数を抽出(低リスク) +### Phase 1: +page.svelte から utils へ純粋関数を抽出(低リスク)✅ -- [ ] `utils/workbooks.ts` に `getWorkBooksByType(workbooks, type)` を追加(TODO コメント対応) +- [x] `utils/workbooks.ts` に `getWorkBooksByType(workbooks, type)` を追加(TODO コメント対応) - テスト追加(通常フィルタ、空配列、type が一致しない場合) -- [ ] `utils/workbooks.ts` に `buildTaskResultsByWorkBookId(workbooks, taskResultsByTaskId)` を追加 +- [x] `utils/workbooks.ts` に `buildTaskResultsByWorkBookId(workbooks, taskResultsByTaskId)` を追加 - 現在 +page.svelte の `fetchTaskResultsWithWorkBookId()` に相当 - テスト追加(task 結果あり/なし、空配列) -- [ ] `+page.svelte` を更新: 両関数を import に置き換え +- [x] `+page.svelte` を更新: 両関数を import に置き換え -### Phase 2: +page.server.ts の N+1 修正とクリーンアップ(中リスク) +### Phase 2: +page.server.ts の N+1 修正とクリーンアップ(中リスク)✅ -- [ ] `services/workbooks.ts` に `getWorkBooksWithAuthors()` を追加 +- [x] `services/workbooks.ts` に `getWorkBooksWithAuthors()` を追加 - `include: { user: { select: { username: true } }, workBookTasks: { orderBy: { priority: 'asc' } } }` を使用 - 戻り値型 `WorkbooksWithAuthors` を `types/workbook.ts` に追加(`authorName: string` を含む) - 著者削除済みの場合は `user?.username ?? 'unknown'` で対応 -- [ ] `+page.server.ts` の `load()` を更新: +- [x] `+page.server.ts` の `load()` を更新: - `getWorkBooks()` + N+1 ループ → `getWorkBooksWithAuthors()` に置き換え - try/catch の範囲を拡大(workbooks 取得も含める) -- [ ] CRUD アクションのログを「受信時」→「成功後」に移動(3ファイル) +- [x] CRUD アクションのログを「受信時」→「成功後」に移動(3ファイル) **判断根拠**: 現在の `console.log('form -> actions -> ...')` はバリデーション**前**(アクション受信時)に発火するため、不正なリクエストでも記録される。また「どのリソースを誰が操作したか」が分からず本番デバッグに役立たない。成功を確認できる位置に移動し、識別情報を含めることで意味のあるログにする。 @@ -91,10 +91,10 @@ console.log(`Deleted workbook ${workBookId} by user ${loggedInUser?.id}`); 対象ファイルと成功後ログの内容: -| ファイル | ログ内容 | -|----------|---------| -| `routes/workbooks/+page.server.ts` (delete) | `Deleted workbook ${workBookId} by user ${loggedInUser?.id}` | -| `routes/workbooks/create/+page.server.ts` | `Created workbook "${workBook.title}" by user ${author.id}` | +| ファイル | ログ内容 | +| ---------------------------------------------- | ---------------------------------------------------------------------------------- | +| `routes/workbooks/+page.server.ts` (delete) | `Deleted workbook ${workBookId} by user ${loggedInUser?.id}` | +| `routes/workbooks/create/+page.server.ts` | `Created workbook "${workBook.title}" by user ${author.id}` | | `routes/workbooks/edit/[slug]/+page.server.ts` | `Updated workbook ${workBookId}`(edit action に `locals` がなくユーザー情報なし) | ### Phase 3: サービス層から SvelteKit error() を除去(中リスク) @@ -125,11 +125,11 @@ const workbook = await getWorkbookWithAuthor(slug); if (!workbook) { error(NOT_FOUND, '...'); } ``` -- [ ] `getWorkbookWithAuthor()` から `error()` 呼び出しを削除、`null` を返すように変更 -- [ ] 呼び出し元のルートハンドラーを更新(`src/routes/workbooks/[slug]/+page.server.ts` 等): +- [x] `getWorkbookWithAuthor()` から `error()` 呼び出しを削除、`null` を返すように変更 +- [x] 呼び出し元のルートハンドラーを更新(`src/routes/workbooks/[slug]/+page.server.ts` 等): - slug バリデーションを呼び出し元に移動 - null チェック後に `error(BAD_REQUEST/NOT_FOUND, ...)` を呼び出す -- [ ] `isExistingWorkBook()` を `db.workBook.count({ where: { id } }) > 0` に変更 +- [x] `isExistingWorkBook()` を `db.workBook.count({ where: { id } }) > 0` に変更 ### Phase 4: services/workbooks.ts の統合テスト追加(Phase 3 後) @@ -140,76 +140,30 @@ if (!workbook) { error(NOT_FOUND, '...'); } - `updateWorkBook`: 正常、存在しない ID - `deleteWorkBook`: 正常、存在しない ID -### Phase 5: WorkBookList.svelte 型修正と Svelte 5 パターン修正(低リスク) +### Phase 5: WorkBookList.svelte 型修正と Svelte 5 パターン修正(低リスク)✅ -- [ ] `loggedInUser: any` → 使用フィールド(`id: string`, `role: Roles`)の最小 interface を定義 - - `$lib/types/user.ts` の既存型を確認して再利用 -- [ ] `$derived(() => countReadableWorkbooks(...))` を `$derived(countReadableWorkbooks(...))` に修正(2箇所) +- [x] `loggedInUser: any` → `interface LoggedInUser { id: string; role: Roles }` を定義(インライン) + - `$lib/types/user.ts` の `User` は `userId` フィールドで不一致のため、最小 interface をコンポーネント内に定義 + - `+page.svelte` から渡す際に `as { id: string; role: Roles }` キャスト(Prisma Roles vs $lib Roles の型差異) +- [x] `$derived(() => countReadableWorkbooks(...))` を `$derived(countReadableWorkbooks(...))` に修正(2箇所) -### Phase 6: WorkBookBaseTable.svelte を3種コンポーネントに分割(中リスク) +### Phase 6: WorkBookBaseTable.svelte を3種コンポーネントに分割(中リスク)✅ -ユーザー方針: 「3種類コンポーネントをベースに、if 文のところは interface で分岐をなくす」 - -**設計判断: 完全分離 vs 基底+差分** - -`WorkBookBaseTable.svelte` を「共通の基底コンポーネント」として残す案もあるが、**完全分離を採用する**。 - -| 方針 | メリット | デメリット | -| -------------------- | ---------------------------------------- | ----------------------------------------------- | -| **完全分離(採用)** | 各ファイルが自己完結、開けば全体が見える | `
` 等のテンプレート構造が3箇所に存在 | -| 基底 + 差分 | テンプレートの行数が少ない | 「共通はどこ?差分はどこ?」と2層読み回しが必要 | - -分割後の1ファイルは概算 50〜70行。共通の**ロジック**(`AcceptedCounter`, `ThermometerProgressBar` 等)は import で共有するため実質的な重複はなく、重複するのはテンプレート構造だけ → 許容範囲。`WorkBookBaseTable.svelte` はリネームせず削除する。 - -**命名判断: `WorkBook` プレフィックスを省略する** - -同ディレクトリの `WorkBookList.svelte` や `WorkbookTabItem.svelte` は routes 側(`+page.svelte`)からもインポートされるため、`WorkBook` プレフィックスが文脈の補足として機能する。一方、3分割コンポーネントは workbooks feature 内でしか使われず、`src/features/workbooks/components/list/` というパスがすでに "workbook" の文脈を提供している。プレフィックスは冗長になるため省略する。 - -``` -// 省略なし(冗長) -import CurriculumWorkbookTable from '.../workbooks/components/list/CurriculumWorkbookTable.svelte' - -// 省略あり(採用) -import CurriculumTable from '.../workbooks/components/list/CurriculumTable.svelte' -``` - -- [ ] 共通 `interface WorkbookTableProps` を定義(3コンポーネントが同じシグネチャを持つ) - - `workbooks`, `workbookGradeModes`, `userId`, `role`, `taskResults` を共通 props に -- [ ] 3コンポーネントを作成(全て `WorkbookTableProps` を実装): +- [x] 各コンポーネントに同じ `interface Props` を定義(`workbooks`, `workbookGradeModes`, `userId`, `role`, `taskResults`) +- [x] 3コンポーネントを作成: - `CurriculumTable.svelte` — グレード + タイトル列 - `SolutionTable.svelte` — タイトル列(padding 広め) - `CreatedByUserTable.svelte` — 作者 + タイトル列 - - 各コンポーネントは同じ `interface Props` を持ち、内部に WorkBookType の if 分岐なし - - 共通列(ProgressBar、AcceptedCounter、CompletedTasks、編集/削除ボタン)は既存コンポーネントをそのまま利用 -- [ ] `WorkBookList.svelte` を更新: `Record` ルックアップで切り替え - -```svelte - - - -{@const TableComponent = tableComponents[workbookType]} - -``` - -- [ ] `WorkBookBaseTable.svelte` を削除 +- [x] `WorkBookList.svelte` を更新: `Record` ルックアップで切り替え + - `{@const TableComponent}` は `{#if}` の直接の子として配置(HTML 要素内では使用不可) + - 廃止型(TEXTBOOK/GENRE/THEME/OTHERS)は近似の既存コンポーネントにフォールバック +- [x] `WorkBookBaseTable.svelte` を削除 -### Phase 7: コーナーケーステストの強化(純粋追加) +### Phase 7: コーナーケーステストの強化(純粋追加)✅ -- [ ] `utils/workbooks.ts` `calcWorkBookGradeModes`: - - タイブレーク動作のテスト(同頻度グレードが2つある場合、`calcGradeMode` の動作を確認して文書化) -- [ ] 既存テストの `toBeTruthy()`/`toBeFalsy()` を `toBe(true)`/`toBe(false)` に置き換え(coding-style.md 準拠) +- [x] `utils/workbooks.ts` `calcWorkBookGradeModes`: + - タイブレーク: 同頻度グレードが2つある場合は最も易しいグレードを返す(7Q vs 6Q → 7Q、Q2 vs Q1 → Q2) +- [x] 既存テストの `toBeTruthy()`/`toBeFalsy()` を `toBe(true)`/`toBe(false)` に置き換え(coding-style.md 準拠) ### Phase 8: E2E テスト追加(純粋追加) diff --git a/eslint.config.mjs b/eslint.config.mjs index 9a1b06ed3..10d3cbaa1 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -71,7 +71,10 @@ export default [ }, ], // Add TypeScript ESLint rules manually - '@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_' }], + '@typescript-eslint/no-unused-vars': [ + 'error', + { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }, + ], '@typescript-eslint/no-explicit-any': 'warn', // Disable some strict Svelte rules that are too aggressive 'svelte/require-each-key': 'warn', diff --git a/src/features/workbooks/components/list/CreatedByUserTable.svelte b/src/features/workbooks/components/list/CreatedByUserTable.svelte new file mode 100644 index 000000000..4d264c8d0 --- /dev/null +++ b/src/features/workbooks/components/list/CreatedByUserTable.svelte @@ -0,0 +1,109 @@ + + +
+
+ + 作者 + + 回答状況 + + 修了 + + + + + {#each workbooks as workbook} + {#if canRead(workbook.isPublished, userId, workbook.authorId)} + + + +
+ {workbook.authorName} +
+
+ + + + + + + + +
+ +
+
+ +
+ +
+
+ +
+ {#if canEdit(userId, workbook.authorId, role, workbook.isPublished)} + 編集 + {/if} + + {#if canDelete(userId, workbook.authorId)} +
+ + + {/if} +
+
+
+ {/if} + {/each} +
+
+ diff --git a/src/features/workbooks/components/list/WorkBookBaseTable.svelte b/src/features/workbooks/components/list/CurriculumTable.svelte similarity index 67% rename from src/features/workbooks/components/list/WorkBookBaseTable.svelte rename to src/features/workbooks/components/list/CurriculumTable.svelte index 71f9509a9..5b2889110 100644 --- a/src/features/workbooks/components/list/WorkBookBaseTable.svelte +++ b/src/features/workbooks/components/list/CurriculumTable.svelte @@ -12,7 +12,7 @@ import type { Roles } from '$lib/types/user'; import { TaskGrade, type TaskResults } from '$lib/types/task'; - import { WorkBookType, type WorkbooksList } from '$features/workbooks/types/workbook'; + import type { WorkbooksList } from '$features/workbooks/types/workbook'; import GradeLabel from '$lib/components/GradeLabel.svelte'; import CompletedTasks from '$lib/components/Trophies/CompletedTasks.svelte'; @@ -25,7 +25,6 @@ import { getUrlSlugFrom } from '$features/workbooks/utils/workbooks'; interface Props { - workbookType: WorkBookType; workbooks: WorkbooksList; workbookGradeModes: Map; userId: string; @@ -33,7 +32,7 @@ taskResults: Map; } - let { workbookType, workbooks, workbookGradeModes, userId, role, taskResults }: Props = $props(); + let { workbooks, workbookGradeModes, userId, role, taskResults }: Props = $props(); function getGradeMode(workbookId: number): TaskGrade { return workbookGradeModes.get(workbookId) ?? TaskGrade.PENDING; @@ -44,23 +43,13 @@ } - -
- {#if workbookType === WorkBookType.CURRICULUM} - -
グレード
-
- - {:else if workbookType === WorkBookType.SOLUTION} - - {:else if workbookType === WorkBookType.CREATED_BY_USER} - 作者 - - {/if} - + +
グレード
+
+ 回答状況 修了 @@ -71,30 +60,15 @@ {#each workbooks as workbook} {#if canRead(workbook.isPublished, userId, workbook.authorId)} - {#if workbookType === WorkBookType.CURRICULUM} - - -
- -
-
- - - - {:else if workbookType === WorkBookType.SOLUTION} - - - {:else if workbookType === WorkBookType.CREATED_BY_USER} - - -
- {workbook.authorName} -
-
+ + +
+ +
+
- - - {/if} + + + import { enhance } from '$app/forms'; + + import { + Table, + TableBody, + TableBodyCell, + TableBodyRow, + TableHead, + TableHeadCell, + } from 'flowbite-svelte'; + + import type { Roles } from '$lib/types/user'; + import { TaskGrade, type TaskResults } from '$lib/types/task'; + import type { WorkbooksList } from '$features/workbooks/types/workbook'; + + import CompletedTasks from '$lib/components/Trophies/CompletedTasks.svelte'; + import ThermometerProgressBar from '$lib/components/ThermometerProgressBar.svelte'; + import AcceptedCounter from '$features/workbooks/components/list/AcceptedCounter.svelte'; + import TitleTableHeadCell from '$features/workbooks/components/list/TitleTableHeadCell.svelte'; + import TitleTableBodyCell from '$features/workbooks/components/list/TitleTableBodyCell.svelte'; + + import { canRead, canEdit, canDelete } from '$lib/utils/authorship'; + import { getUrlSlugFrom } from '$features/workbooks/utils/workbooks'; + + interface Props { + workbooks: WorkbooksList; + workbookGradeModes: Map; + userId: string; + role: Roles; + taskResults: Map; + } + + let { workbooks, workbookGradeModes: _, userId, role, taskResults }: Props = $props(); + + function getTaskResult(workbookId: number): TaskResults { + return taskResults?.get(workbookId) ?? []; + } + + +
+
+ + + 回答状況 + + 修了 + + + + + {#each workbooks as workbook} + {#if canRead(workbook.isPublished, userId, workbook.authorId)} + + + + + + + + +
+ +
+
+ +
+ +
+
+ +
+ {#if canEdit(userId, workbook.authorId, role, workbook.isPublished)} + 編集 + {/if} + + {#if canDelete(userId, workbook.authorId)} +
+ + + {/if} +
+
+
+ {/if} + {/each} +
+
+
diff --git a/src/features/workbooks/components/list/WorkBookList.svelte b/src/features/workbooks/components/list/WorkBookList.svelte index e1e2383ee..07f63061f 100644 --- a/src/features/workbooks/components/list/WorkBookList.svelte +++ b/src/features/workbooks/components/list/WorkBookList.svelte @@ -1,5 +1,6 @@ @@ -135,14 +158,14 @@ {/if} -{#if readableMainWorkbooksCount()} +{#if readableMainWorkbooksCount} + {@const TableComponent = tableComponents[workbookType]}
{#if workbookType === WorkBookType.CURRICULUM}
手引き
{/if} - - {#if workbookType === WorkBookType.CURRICULUM && readableReplenishedWorkbooksCount()} + {#if workbookType === WorkBookType.CURRICULUM && readableReplenishedWorkbooksCount}
@@ -182,8 +205,7 @@
{#if showReplenishmentWorkbooks} - > = {}): Omit { + return { + authorId: '1', + title: 'テスト問題集', + description: '', + editorialUrl: '', + isPublished: false, + isOfficial: false, + isReplenished: false, + workBookType: WorkBookType.CREATED_BY_USER, + urlSlug: null, + workBookTasks: [], + ...overrides, + }; +} + +describe('getWorkBookTasks', () => { + test('taskId / priority / comment を含むタスク配列を返す', () => { + const workBook = createWorkBook({ + workBookTasks: [ + { taskId: 'abc300_a', priority: 1, comment: 'コメント' }, + { taskId: 'abc300_b', priority: 2, comment: '' }, + ], + }); + expect(getWorkBookTasks(workBook)).toEqual([ + { taskId: 'abc300_a', priority: 1, comment: 'コメント' }, + { taskId: 'abc300_b', priority: 2, comment: '' }, + ]); + }); + + test('workBookTasks が空の場合は空配列を返す', () => { + expect(getWorkBookTasks(createWorkBook({ workBookTasks: [] }))).toEqual([]); + }); + + test('comment が空文字列でも含めて返す', () => { + const workBook = createWorkBook({ + workBookTasks: [{ taskId: 'abc300_a', priority: 1, comment: '' }], + }); + expect(getWorkBookTasks(workBook)).toEqual([{ taskId: 'abc300_a', priority: 1, comment: '' }]); + }); +}); + +describe('validateRequiredFields', () => { + test('正常ケース: 全フィールドが揃っている場合はエラーを投げない', () => { + const tasks: WorkBookTasksBase = [{ taskId: 'abc300_a', priority: 1, comment: '' }]; + expect(() => validateRequiredFields(tasks)).not.toThrow(); + }); + + test('空配列の場合はエラーを投げない', () => { + expect(() => validateRequiredFields([])).not.toThrow(); + }); + + test('taskId が空文字列の場合にエラーを投げる(index 0)', () => { + const tasks: WorkBookTasksBase = [{ taskId: '', priority: 1, comment: '' }]; + expect(() => validateRequiredFields(tasks)).toThrow('index 0'); + }); + + test('taskId が空文字列の場合にエラーを投げる(中間タスク)', () => { + const tasks: WorkBookTasksBase = [ + { taskId: 'abc300_a', priority: 1, comment: '' }, + { taskId: '', priority: 2, comment: '' }, + { taskId: 'abc300_c', priority: 3, comment: '' }, + ]; + expect(() => validateRequiredFields(tasks)).toThrow('index 1'); + }); + + test('taskId が空文字列の場合にエラーを投げる(最後のタスク)', () => { + const tasks: WorkBookTasksBase = [ + { taskId: 'abc300_a', priority: 1, comment: '' }, + { taskId: '', priority: 2, comment: '' }, + ]; + expect(() => validateRequiredFields(tasks)).toThrow('index 1'); + }); + + // NOTE: !task.priority は priority === 0 を falsy として扱うためエラーになる。 + // priority は実際には 1 以上の整数で設定されるため、0 は使用されない想定。 + test('priority が 0 の場合にエラーを投げる(index 0)', () => { + const tasks: WorkBookTasksBase = [{ taskId: 'abc300_a', priority: 0, comment: '' }]; + expect(() => validateRequiredFields(tasks)).toThrow('index 0'); + }); + + test('priority が 0 の場合にエラーを投げる(中間タスク)', () => { + const tasks: WorkBookTasksBase = [ + { taskId: 'abc300_a', priority: 1, comment: '' }, + { taskId: 'abc300_b', priority: 0, comment: '' }, + { taskId: 'abc300_c', priority: 3, comment: '' }, + ]; + expect(() => validateRequiredFields(tasks)).toThrow('index 1'); + }); + + test('priority が 0 の場合にエラーを投げる(最後のタスク)', () => { + const tasks: WorkBookTasksBase = [ + { taskId: 'abc300_a', priority: 1, comment: '' }, + { taskId: 'abc300_b', priority: 0, comment: '' }, + ]; + expect(() => validateRequiredFields(tasks)).toThrow('index 1'); + }); + + // 負数・小数は truthy なのでエラーにならない(ドキュメント目的) + test('priority が負数の場合はエラーを投げない(truthy 扱い)', () => { + const tasks: WorkBookTasksBase = [{ taskId: 'abc300_a', priority: -1, comment: '' }]; + expect(() => validateRequiredFields(tasks)).not.toThrow(); + }); + + test('priority が小数の場合はエラーを投げない(truthy 扱い)', () => { + const tasks: WorkBookTasksBase = [{ taskId: 'abc300_a', priority: 1.5, comment: '' }]; + expect(() => validateRequiredFields(tasks)).not.toThrow(); + }); +}); diff --git a/src/features/workbooks/services/workbooks.ts b/src/features/workbooks/services/workbooks.ts index d24aacccf..5e7a80696 100644 --- a/src/features/workbooks/services/workbooks.ts +++ b/src/features/workbooks/services/workbooks.ts @@ -1,9 +1,9 @@ -import { error } from '@sveltejs/kit'; import { default as db } from '$lib/server/database'; import type { WorkBook, WorkBooks, + WorkbooksWithAuthors, WorkBookTasksBase, WorkBookType, } from '$features/workbooks/types/workbook'; @@ -16,7 +16,6 @@ import * as userCrud from '$lib/services/users'; import { sanitizeUrl } from '$lib/utils/url'; import { parseWorkBookId, parseWorkBookUrlSlug } from '$features/workbooks/utils/workbook'; -import { BAD_REQUEST, NOT_FOUND } from '$lib/constants/http-response-status-codes'; export async function getWorkBooks(): Promise { const workbooks = await db.workBook.findMany({ @@ -35,6 +34,29 @@ export async function getWorkBooks(): Promise { return workbooks; } +export async function getWorkBooksWithAuthors(): Promise { + const workbooks = await db.workBook.findMany({ + orderBy: { + id: 'asc', + }, + include: { + user: { + select: { username: true }, + }, + workBookTasks: { + orderBy: { + priority: 'asc', + }, + }, + }, + }); + + return workbooks.map((workbook) => ({ + ...workbook, + authorName: workbook.user?.username ?? 'unknown', + })); +} + export async function getWorkBook(workBookId: number): Promise { const workBook = await db.workBook.findUnique({ where: { @@ -78,14 +100,10 @@ export async function getWorkBookByUrlSlug(urlSlug: string): Promise { +): Promise<{ workBook: WorkBook; isExistingAuthor: boolean } | null> { const workBookId = parseWorkBookId(slug); const workBookUrlSlug = parseWorkBookUrlSlug(slug); - if (workBookId === null && workBookUrlSlug === null) { - error(BAD_REQUEST, '不正な問題集idです。'); - } - let workBook: WorkBook | null = null; if (workBookId) { @@ -95,7 +113,7 @@ export async function getWorkbookWithAuthor( } if (workBook === null) { - error(NOT_FOUND, `問題集id: ${slug} は見つかりませんでした。`); + return null; } // Validate if the author of the workbook exists after the workbook has been created. @@ -145,13 +163,7 @@ async function isExistingUrlSlug(slug: string): Promise { } async function isExistingWorkBook(workBookId: number): Promise { - const workBook = await getWorkBook(workBookId); - - if (workBook) { - return true; - } else { - return false; - } + return (await db.workBook.count({ where: { id: workBookId } })) > 0; } export async function updateWorkBook(workBookId: number, workBook: WorkBook): Promise { diff --git a/src/features/workbooks/types/workbook.ts b/src/features/workbooks/types/workbook.ts index 99984e6c7..277335a98 100644 --- a/src/features/workbooks/types/workbook.ts +++ b/src/features/workbooks/types/workbook.ts @@ -33,6 +33,9 @@ export interface WorkbookList extends WorkBookBase { export type WorkbooksList = WorkbookList[]; +// Alias used as the return type of getWorkBooksWithAuthors(). +export type WorkbooksWithAuthors = WorkbooksList; + // HACK: enumを使うときは毎回書いているので、もっと簡略化できないか? export const WorkBookType: { [key in WorkBookTypeOrigin]: key } = { CREATED_BY_USER: 'CREATED_BY_USER', // (デフォルト) ユーザ作成: サービスの利用者がさまざまなコンセプトで作成 diff --git a/src/features/workbooks/utils/workbooks.test.ts b/src/features/workbooks/utils/workbooks.test.ts index 644427aee..5a3eb74d4 100644 --- a/src/features/workbooks/utils/workbooks.test.ts +++ b/src/features/workbooks/utils/workbooks.test.ts @@ -1,13 +1,15 @@ import { expect, test } from 'vitest'; import { Roles } from '$lib/types/user'; -import { TaskGrade, type Task } from '$lib/types/task'; +import { TaskGrade, type Task, type TaskResult } from '$lib/types/task'; import { type WorkbookList, WorkBookType } from '$features/workbooks/types/workbook'; import { canViewWorkBook, getUrlSlugFrom, calcWorkBookGradeModes, + getWorkBooksByType, + buildTaskResultsByWorkBookId, } from '$features/workbooks/utils/workbooks'; function createTask(taskId: string, grade: TaskGrade): Task { @@ -20,6 +22,23 @@ function createTask(taskId: string, grade: TaskGrade): Task { }; } +function createTaskResult(taskId: string): TaskResult { + return { + task_id: taskId, + contest_id: 'abc300', + task_table_index: 'A', + title: '', + grade: TaskGrade.Q10, + user_id: '1', + status_name: 'AC', + status_id: '1', + submission_status_image_path: '', + submission_status_label_name: 'AC', + is_ac: true, + updated_at: new Date(), + }; +} + function createWorkBookListBase(overrides: Partial = {}): WorkbookList { return { id: 1, @@ -42,21 +61,21 @@ describe('Workbooks', () => { describe('can view workbooks', () => { describe('when the user is admin', () => { test('admin can view published workbooks', () => { - expect(canViewWorkBook(Roles.ADMIN, true)).toBeTruthy(); + expect(canViewWorkBook(Roles.ADMIN, true)).toBe(true); }); test('admin can view workbooks privately', () => { - expect(canViewWorkBook(Roles.ADMIN, false)).toBeTruthy(); + expect(canViewWorkBook(Roles.ADMIN, false)).toBe(true); }); }); describe('when the user is not admin', () => { test('the user can view published workbooks', () => { - expect(canViewWorkBook(Roles.USER, true)).toBeTruthy(); + expect(canViewWorkBook(Roles.USER, true)).toBe(true); }); test('the user can not view workbooks privately', () => { - expect(canViewWorkBook(Roles.USER, false)).toBeFalsy(); + expect(canViewWorkBook(Roles.USER, false)).toBe(false); }); }); }); @@ -173,5 +192,127 @@ describe('Workbooks', () => { expect(result.get(10)).toBe(TaskGrade.Q8); expect(result.get(20)).toBe(TaskGrade.Q7); }); + + // タイブレーク: 同頻度グレードが2つある場合は最も易しいグレードを返す + // コメント例: 8Q: 2問、7Q: 5問、6Q: 5問 => 7Q + test('同頻度のグレードが2つある場合は最も易しいグレードを返す(7Q と 6Q → 7Q)', () => { + const tasksMapByIds: Map = new Map([ + ['abc300_a', createTask('abc300_a', TaskGrade.Q8)], + ['abc300_b', createTask('abc300_b', TaskGrade.Q8)], + ['abc300_c', createTask('abc300_c', TaskGrade.Q7)], + ['abc300_d', createTask('abc300_d', TaskGrade.Q7)], + ['abc300_e', createTask('abc300_e', TaskGrade.Q7)], + ['abc300_f', createTask('abc300_f', TaskGrade.Q7)], + ['abc300_g', createTask('abc300_g', TaskGrade.Q7)], + ['abc300_h', createTask('abc300_h', TaskGrade.Q6)], + ]); + const workbooks = [ + createWorkBookListBase({ + id: 1, + workBookTasks: [ + { taskId: 'abc300_a', priority: 1, comment: '' }, + { taskId: 'abc300_b', priority: 2, comment: '' }, + { taskId: 'abc300_c', priority: 3, comment: '' }, + { taskId: 'abc300_d', priority: 4, comment: '' }, + { taskId: 'abc300_e', priority: 5, comment: '' }, + { taskId: 'abc300_f', priority: 6, comment: '' }, + { taskId: 'abc300_g', priority: 7, comment: '' }, + { taskId: 'abc300_h', priority: 8, comment: '' }, + ], + }), + ]; + // Q7: 5問, Q6: 5問 で同頻度 → 易しい方の Q7 を返す + const result = calcWorkBookGradeModes(workbooks, tasksMapByIds); + expect(result.get(1)).toBe(TaskGrade.Q7); + }); + + test('同頻度のグレードが2つある場合は最も易しいグレードを返す(Q2 と Q1 → Q2)', () => { + const tasksMapByIds: Map = new Map([ + ['abc300_a', createTask('abc300_a', TaskGrade.Q2)], + ['abc300_b', createTask('abc300_b', TaskGrade.Q1)], + ]); + const workbooks = [ + createWorkBookListBase({ + id: 1, + workBookTasks: [ + { taskId: 'abc300_a', priority: 1, comment: '' }, + { taskId: 'abc300_b', priority: 2, comment: '' }, + ], + }), + ]; + // Q2 と Q1 で同頻度 → 易しい方の Q2 を返す + const result = calcWorkBookGradeModes(workbooks, tasksMapByIds); + expect(result.get(1)).toBe(TaskGrade.Q2); + }); + }); + + describe('getWorkBooksByType', () => { + test('指定タイプのみを返す', () => { + const workbooks = [ + createWorkBookListBase({ id: 1, workBookType: WorkBookType.CURRICULUM }), + createWorkBookListBase({ id: 2, workBookType: WorkBookType.SOLUTION }), + createWorkBookListBase({ id: 3, workBookType: WorkBookType.CURRICULUM }), + ]; + const result = getWorkBooksByType(workbooks, WorkBookType.CURRICULUM); + expect(result.map((workbook) => workbook.id)).toEqual([1, 3]); + }); + + test('空配列を渡すと空配列を返す', () => { + expect(getWorkBooksByType([], WorkBookType.CURRICULUM)).toEqual([]); + }); + + test('一致するタイプがない場合は空配列を返す', () => { + const workbooks = [createWorkBookListBase({ id: 1, workBookType: WorkBookType.SOLUTION })]; + expect(getWorkBooksByType(workbooks, WorkBookType.CURRICULUM)).toEqual([]); + }); + }); + + describe('buildTaskResultsByWorkBookId', () => { + test('タスク結果が存在する問題集は Map に含まれる', () => { + const taskResult = createTaskResult('abc300_a'); + const taskResultsByTaskId = new Map([['abc300_a', taskResult]]); + const workbooks = [ + createWorkBookListBase({ + id: 1, + workBookTasks: [{ taskId: 'abc300_a', priority: 1, comment: '' }], + }), + ]; + const result = buildTaskResultsByWorkBookId(workbooks, taskResultsByTaskId); + expect(result.get(1)).toEqual([taskResult]); + }); + + test('タスク結果が存在しない問題集は Map から除外される', () => { + const taskResultsByTaskId = new Map(); + const workbooks = [ + createWorkBookListBase({ + id: 1, + workBookTasks: [{ taskId: 'abc300_a', priority: 1, comment: '' }], + }), + ]; + const result = buildTaskResultsByWorkBookId(workbooks, taskResultsByTaskId); + expect(result.has(1)).toBe(false); + }); + + test('空の workbooks を渡すと空の Map を返す', () => { + const taskResultsByTaskId = new Map(); + const result = buildTaskResultsByWorkBookId([], taskResultsByTaskId); + expect(result.size).toBe(0); + }); + + test('複数タスクのうち一部のみ結果がある場合、存在するものだけ含まれる', () => { + const taskResult = createTaskResult('abc300_a'); + const taskResultsByTaskId = new Map([['abc300_a', taskResult]]); + const workbooks = [ + createWorkBookListBase({ + id: 1, + workBookTasks: [ + { taskId: 'abc300_a', priority: 1, comment: '' }, + { taskId: 'abc300_b', priority: 2, comment: '' }, + ], + }), + ]; + const result = buildTaskResultsByWorkBookId(workbooks, taskResultsByTaskId); + expect(result.get(1)).toEqual([taskResult]); + }); }); }); diff --git a/src/features/workbooks/utils/workbooks.ts b/src/features/workbooks/utils/workbooks.ts index e314016cf..715b17fce 100644 --- a/src/features/workbooks/utils/workbooks.ts +++ b/src/features/workbooks/utils/workbooks.ts @@ -1,6 +1,18 @@ import { Roles } from '$lib/types/user'; -import { TaskGrade, type Task, type TaskGrades } from '$lib/types/task'; -import type { WorkBook, WorkbookList, WorkBookTaskBase } from '$features/workbooks/types/workbook'; +import { + TaskGrade, + type Task, + type TaskGrades, + type TaskResult, + type TaskResults, +} from '$lib/types/task'; +import type { + WorkBook, + WorkbookList, + WorkbooksList, + WorkBookTaskBase, + WorkBookType, +} from '$features/workbooks/types/workbook'; import { isAdmin } from '$lib/utils/authorship'; import { calcGradeMode } from '$lib/utils/task'; @@ -22,6 +34,48 @@ export function getUrlSlugFrom(workbook: WorkbookList | WorkBook): string { return slug ? slug : workbook.id.toString(); } +/** + * Filters workbooks by type. + */ +export function getWorkBooksByType( + workbooks: WorkbooksList, + workBookType: WorkBookType, +): WorkbooksList { + return workbooks.filter((workbook: WorkbookList) => workbook.workBookType === workBookType); +} + +/** + * Builds a map from workbook ID to the task results for that workbook's tasks. + * Workbooks with no matching task results are omitted from the map. + */ +export function buildTaskResultsByWorkBookId( + workbooks: WorkbooksList, + taskResultsByTaskId: Map, +): Map { + const taskResultsWithWorkBookId = new Map(); + + workbooks.forEach((workbook: WorkbookList) => { + const taskResults: TaskResults = workbook.workBookTasks.reduce( + (array: TaskResults, workBookTask: WorkBookTaskBase) => { + const taskResult = taskResultsByTaskId.get(workBookTask.taskId); + + if (taskResult !== undefined) { + array.push(taskResult); + } + + return array; + }, + [], + ); + + if (taskResults.length > 0) { + taskResultsWithWorkBookId.set(workbook.id, taskResults); + } + }); + + return taskResultsWithWorkBookId; +} + /** * Calculates the grade modes for a list of workbooks in curriculum based on their tasks. * diff --git a/src/routes/workbooks/+page.server.ts b/src/routes/workbooks/+page.server.ts index a2e7b046a..754176a89 100644 --- a/src/routes/workbooks/+page.server.ts +++ b/src/routes/workbooks/+page.server.ts @@ -3,7 +3,6 @@ import { error } from '@sveltejs/kit'; import * as workBooksCrud from '$features/workbooks/services/workbooks'; import * as taskCrud from '$lib/services/tasks'; import * as taskResultsCrud from '$lib/services/task_results'; -import * as userCrud from '$lib/services/users'; import { getLoggedInUser, canDelete } from '$lib/utils/authorship'; import { parseWorkBookId } from '$features/workbooks/utils/workbook'; @@ -15,26 +14,11 @@ import { INTERNAL_SERVER_ERROR, } from '$lib/constants/http-response-status-codes'; -// See: -// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map export async function load({ locals }) { const loggedInUser = await getLoggedInUser(locals); - const workbooks = await workBooksCrud.getWorkBooks(); - const workbooksWithAuthors = await Promise.all( - workbooks.map(async (workbook) => { - const workbookAuthor = await userCrud.getUserById(workbook.authorId); - - if (workbookAuthor) { - return { ...workbook, authorName: workbookAuthor.username }; - } else { - // ユーザが問題集を作成したあとに、アカウントを削除した場合 - return { ...workbook, authorName: 'unknown' }; - } - }), - ); - try { + const workbooks = await workBooksCrud.getWorkBooksWithAuthors(); // 問題集を構成する問題のグレードの最頻値を取得するために使用 const tasksMapByIds = await taskCrud.getTasksByTaskId(); // ユーザの回答状況を表示するために使用 @@ -44,13 +28,13 @@ export async function load({ locals }) { ); return { - workbooks: workbooksWithAuthors, + workbooks: workbooks, tasksMapByIds: tasksMapByIds, taskResultsByTaskId: taskResultsByTaskId, loggedInUser: loggedInUser, }; } catch (e) { - console.error('Failed to fetch tasks or task results: ', e); + console.error('Failed to fetch workbooks, tasks or task results: ', e); error( INTERNAL_SERVER_ERROR, '問題もしくは回答の取得に失敗しました。しばらくしてから、もう一度試してください。', @@ -60,7 +44,6 @@ export async function load({ locals }) { export const actions = { delete: async ({ locals, request }) => { - console.log('form -> actions -> delete'); const loggedInUser = await getLoggedInUser(locals); const url = new URL(request.url); @@ -82,6 +65,7 @@ export const actions = { try { await workBooksCrud.deleteWorkBook(workBookId); + console.log(`Deleted workbook ${workBookId} by user ${loggedInUser?.id}`); } catch (e) { console.error(`Failed to delete WorkBook with id ${workBookId}:`, e); error( diff --git a/src/routes/workbooks/+page.svelte b/src/routes/workbooks/+page.svelte index 5cdd6cc0c..5a7e20d2d 100644 --- a/src/routes/workbooks/+page.svelte +++ b/src/routes/workbooks/+page.svelte @@ -3,13 +3,8 @@ import { Button, Tabs } from 'flowbite-svelte'; import { Roles } from '$lib/types/user'; - import { type Task, type TaskResult, type TaskResults } from '$lib/types/task'; - import { - type WorkbookList, - type WorkbooksList, - type WorkBookTaskBase, - WorkBookType, - } from '$features/workbooks/types/workbook'; + import { type Task, type TaskResult } from '$lib/types/task'; + import { type WorkbooksList, WorkBookType } from '$features/workbooks/types/workbook'; import { activeWorkbookTabStore } from '$features/workbooks/stores/active_workbook_tab'; @@ -17,7 +12,12 @@ import WorkbookTabItem from '$features/workbooks/components/list/WorkbookTabItem.svelte'; import WorkBookList from '$features/workbooks/components/list/WorkBookList.svelte'; - import { canViewWorkBook, calcWorkBookGradeModes } from '$features/workbooks/utils/workbooks'; + import { + canViewWorkBook, + calcWorkBookGradeModes, + getWorkBooksByType, + buildTaskResultsByWorkBookId, + } from '$features/workbooks/utils/workbooks'; let { data } = $props(); @@ -26,14 +26,6 @@ // HACK: loggedInUser.roleで比較すると、@prisma/clientと型が異なるため、やむを得ずasでキャスト let role = loggedInUser?.role as Roles; - // TODO: 単体テストをしやすくするため、utilsに移動させる + インポート - const getWorkBooksByType = (workbooks: WorkbooksList, workBookType: WorkBookType) => { - const filteredWorkbooks = workbooks.filter( - (workbook: WorkbookList) => workbook.workBookType === workBookType, - ); - return filteredWorkbooks; - }; - const workBookTabs = [ { title: 'カリキュラム', @@ -63,33 +55,6 @@ let taskResultsByTaskId = data.taskResultsByTaskId as Map; const workbookGradeModes = calcWorkBookGradeModes(data.workbooks as WorkbooksList, tasksMapByIds); - - // 計算量: 問題集の数をN、各問題集の問題の平均値をMとすると、O(N * M) - function fetchTaskResultsWithWorkBookId(workbooks: WorkbooksList, workBookType: WorkBookType) { - const workbooksByType = getWorkBooksByType(workbooks, workBookType); - const taskResultsWithWorkBookId = new Map(); - - workbooksByType.forEach((workbook: WorkbookList) => { - const taskResults: TaskResults = workbook.workBookTasks.reduce( - (array: TaskResults, workBookTask: WorkBookTaskBase) => { - const taskResult = taskResultsByTaskId.get(workBookTask.taskId); - - if (taskResult !== undefined) { - array.push(taskResult); - } - - return array; - }, - [], - ); - - if (taskResults.length > 0) { - taskResultsWithWorkBookId.set(workbook.id, taskResults); - } - }); - - return taskResultsWithWorkBookId; - }
@@ -122,11 +87,11 @@ workbookType={workBookTab.workBookType} workbooks={getWorkBooksByType(workbooks, workBookTab.workBookType)} {workbookGradeModes} - taskResultsWithWorkBookId={fetchTaskResultsWithWorkBookId( - workbooks, - workBookTab.workBookType, + taskResultsWithWorkBookId={buildTaskResultsByWorkBookId( + getWorkBooksByType(workbooks, workBookTab.workBookType), + taskResultsByTaskId, )} - {loggedInUser} + loggedInUser={loggedInUser as { id: string; role: Roles }} />
diff --git a/src/routes/workbooks/[slug]/+page.server.ts b/src/routes/workbooks/[slug]/+page.server.ts index af1e8b415..66c54474b 100644 --- a/src/routes/workbooks/[slug]/+page.server.ts +++ b/src/routes/workbooks/[slug]/+page.server.ts @@ -8,14 +8,24 @@ import { getWorkbookWithAuthor } from '$features/workbooks/services/workbooks'; import * as action from '$lib/actions/update_task_result'; import { getLoggedInUser, isAdmin, canRead } from '$lib/utils/authorship'; -import { FORBIDDEN } from '$lib/constants/http-response-status-codes'; +import { parseWorkBookId, parseWorkBookUrlSlug } from '$features/workbooks/utils/workbook'; +import { BAD_REQUEST, FORBIDDEN, NOT_FOUND } from '$lib/constants/http-response-status-codes'; export async function load({ locals, params }) { const loggedInUser = await getLoggedInUser(locals); const loggedInAsAdmin = isAdmin(loggedInUser?.role as Roles); const slug = params.slug.toLowerCase(); + if (!parseWorkBookId(slug) && !parseWorkBookUrlSlug(slug)) { + error(BAD_REQUEST, '不正な問題集idです。'); + } + const workbookWithAuthor = await getWorkbookWithAuthor(slug); + + if (!workbookWithAuthor) { + error(NOT_FOUND, `問題集id: ${slug} は見つかりませんでした。`); + } + const workBook = workbookWithAuthor.workBook; const isPublished = workBook.isPublished; const authorId = workBook.authorId; diff --git a/src/routes/workbooks/create/+page.server.ts b/src/routes/workbooks/create/+page.server.ts index f6c4a3630..421f2e039 100644 --- a/src/routes/workbooks/create/+page.server.ts +++ b/src/routes/workbooks/create/+page.server.ts @@ -40,7 +40,6 @@ export const load = async ({ locals }) => { export const actions = { default: async ({ locals, request }) => { - console.log('form -> actions -> create'); const author = await getLoggedInUser(locals); if (!author) { @@ -69,6 +68,7 @@ export const actions = { try { await workBooksCrud.createWorkBook(workBook); + console.log(`Created workbook "${workBook.title}" by user ${author.id}`); } catch (e) { console.error('Failed to create a workbook', e); return fail(BAD_REQUEST, { diff --git a/src/routes/workbooks/edit/[slug]/+page.server.ts b/src/routes/workbooks/edit/[slug]/+page.server.ts index 118787a92..c572156a7 100644 --- a/src/routes/workbooks/edit/[slug]/+page.server.ts +++ b/src/routes/workbooks/edit/[slug]/+page.server.ts @@ -1,4 +1,4 @@ -import { redirect, fail } from '@sveltejs/kit'; +import { error, redirect, fail } from '@sveltejs/kit'; import { superValidate } from 'sveltekit-superforms/server'; import { zod4 } from 'sveltekit-superforms/adapters'; @@ -9,9 +9,12 @@ import * as tasksCrud from '$lib/services/tasks'; import * as workBooksCrud from '$features/workbooks/services/workbooks'; import { getLoggedInUser, canEdit, isAdmin } from '$lib/utils/authorship'; +import { parseWorkBookId, parseWorkBookUrlSlug } from '$features/workbooks/utils/workbook'; import { + BAD_REQUEST, FORBIDDEN, + NOT_FOUND, TEMPORARY_REDIRECT, INTERNAL_SERVER_ERROR, } from '$lib/constants/http-response-status-codes'; @@ -20,8 +23,17 @@ export async function load({ locals, params }) { const loggedInUser = await getLoggedInUser(locals); const loggedInAsAdmin = isAdmin(loggedInUser?.role as Roles); const slug = params.slug.toLowerCase(); + + if (!parseWorkBookId(slug) && !parseWorkBookUrlSlug(slug)) { + error(BAD_REQUEST, '不正な問題集idです。'); + } + const workBookWithAuthor = await workBooksCrud.getWorkbookWithAuthor(slug); + if (!workBookWithAuthor) { + error(NOT_FOUND, `問題集id: ${slug} は見つかりませんでした。`); + } + const form = await superValidate(null, zod4(workBookSchema)); const workBook = { ...workBookWithAuthor.workBook, @@ -66,7 +78,6 @@ export async function load({ locals, params }) { export const actions = { default: async ({ request, params }) => { - console.log('form -> actions -> update'); const form = await superValidate(request, zod4(workBookSchema)); if (!form.valid) { @@ -82,10 +93,16 @@ export const actions = { const workBook = form.data; const slug = params.slug.toLowerCase(); const workBookWithAuthor = await workBooksCrud.getWorkbookWithAuthor(slug); + + if (!workBookWithAuthor) { + error(NOT_FOUND, `問題集id: ${slug} は見つかりませんでした。`); + } + const workBookId = workBookWithAuthor.workBook.id; try { await workBooksCrud.updateWorkBook(workBookId, { ...workBook, id: workBookId }); + console.log(`Updated workbook ${workBookId}`); } catch (e) { console.error(`Failed to update WorkBook with id ${workBookId}:`, e); From ec8fbf5cb7c721a3ae62901688c2d8967e275337 Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Thu, 19 Mar 2026 06:26:25 +0000 Subject: [PATCH 03/14] docs: Update workbook list refactoring plan (Phase 0-6 completed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 完了したフェーズのチェックボックスを更新し、残作業(Phase 7-8)を整理。 Co-Authored-By: Claude Sonnet 4.6 --- .../refactor-for-workbook-list/plan.md | 229 ++++-------------- 1 file changed, 49 insertions(+), 180 deletions(-) diff --git a/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md b/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md index cfc63d6de..fa15c549a 100644 --- a/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md +++ b/docs/dev-notes/2026-03-19/refactor-for-workbook-list/plan.md @@ -10,160 +10,71 @@ Issue #3269(管理者指定の並び順で問題集を表示)をスムーズ ## Findings -### テスト欠落(最優先) +### テスト -- `services/workbooks.ts` (202行) — テストなし -- `services/workbook_tasks.ts` (18行) — テストなし +- テストタイトルが日本語 → 英語で統一する(`testing.md` でルール化) +- 正常系・異常系が同一 `describe` に混在 → `valid inputs` / `invalid inputs` で分割する +- テストの並び順が実装の関数順と不一致 → 実装順に揃える -### +page.server.ts +### ドキュメント -- **N+1クエリ**: `getWorkBooks()` → ループ内で `getUserById()` (N回) → `include: { user }` で1クエリ化可能 -- try/catch が `workbooksWithAuthors` を囲っていない(著者取得失敗時も 500 が返らない) -- `console.log('form -> actions -> delete')` デバッグログ残留 +- エクスポートされた関数・型に TSDoc なし → `coding-style.md` で必須化する -### +page.svelte +### コンポーネント設計 -- `getWorkBooksByType()` — TODO コメントあり、utils に移すべき純粋関数 -- `fetchTaskResultsWithWorkBookId()` — 複雑なロジックがコンポーネント内に埋め込まれている +- 複数コンポーネント間で Props 型・ヘルパー関数・テンプレートブロックが重複 → 共通セルコンポーネントに抽出する +- コンポーネント内のヘルパー関数が純粋関数 → `utils/` に移動してテスト可能にする -### services/workbooks.ts +--- -- `getWorkbookWithAuthor()` がサービス層で SvelteKit の `error()` を呼ぶ(テスト不能、責務混在) -- `isExistingWorkBook()` が存在確認のために全フィールドを `findUnique` している(`count` で十分) +## 実装フェーズ -### WorkBookList.svelte +### Phase A: ルール更新(リスクなし) -- `loggedInUser: any` — 型安全性の欠如(使用フィールドは `.id` と `.role` のみ) -- `$derived(() => countReadableWorkbooks(...))` — Svelte 5 の誤用、`$derived(countReadableWorkbooks(...))` が正しい +- [ ] `testing.md`: テストタイトルは英語で書く旨を追記 +- [ ] `coding-style.md`: エクスポート対象(関数・型・クラス)には TSDoc を必須とする旨を追記 +- [ ] `AGENTS.md`: `@quramy/prisma-fabbrica` の用途を明確化(seed / E2E グローバルセットアップ専用。service 層ユニットテストは `vi.mock` を使う) -### WorkBookBaseTable.svelte +### Phase B: テスト修正(低リスク) -- FIXME コメント: 「問題集の種類別にコンポーネントを分ける」 -- CURRICULUM / SOLUTION / CREATED_BY_USER でヘッダー・ボディ両方に if 分岐が散在 +_Phase A 完了後_ -### utils/workbooks.ts テストギャップ +- [ ] `workbook_tasks.test.ts`: テストタイトルを英語に変換 +- [ ] `workbook_tasks.test.ts`: `describe('validateRequiredFields')` を `valid inputs` / `invalid inputs` に分割 +- [ ] `workbooks.test.ts`: テストタイトルを英語に変換 +- [ ] `workbooks.test.ts`: `describe` 順序を実装順(`getWorkBooksByType` → `buildTaskResultsByWorkBookId` → `calcWorkBookGradeModes`)に変更 +- [ ] `workbooks.ts`: `canViewWorkBook` に TSDoc 追加 +- [ ] `workbook_tasks.ts`: `getWorkBookTasks` / `validateRequiredFields` に TSDoc 追加 -- `calcWorkBookGradeModes` のタイブレーク動作(同頻度グレードの場合)が未テスト +### Phase C: テーブルコンポーネントのリファクタリング(中リスク) ---- +_Phase B と独立して実施可能。完了後に `pnpm check` で確認。_ -## 実装フェーズ +- [ ] `utils/workbooks.ts` に `getGradeMode(id, map)` / `getTaskResult(id, map)` を追加(テスト付き) +- [ ] `types/workbook.ts` に `WorkbookTableProps` を定義(3テーブル共通 Props) +- [ ] `GradeTableBodyCell.svelte` を作成(GradeLabel ラッパー、CurriculumTable 専用) +- [ ] `WorkbookProgressCell.svelte` を作成(ThermometerProgressBar + AcceptedCounter) +- [ ] `WorkbookCompletionCell.svelte` を作成(CompletedTasks トロフィー) +- [ ] `WorkbookAuthorActionsCell.svelte` を作成(編集/削除フォーム) + +**命名の判断根拠** + +- `GradeTableBodyCell` — 既存の `TitleTableBodyCell` パターンに合わせる +- `WorkbookProgressCell` と `WorkbookCompletionCell` を分割した理由: ThermometerProgressBar + AcceptedCounter は「現在の進捗」、CompletedTasks トロフィーは「全タスク完了という達成」で意味が異なるため責務を分ける +- `WorkbookAuthorActionsCell` — 編集(`canEdit`)・削除(`canDelete`)はいずれも著者または管理者権限を要する操作。`ManageCell`(曖昧)・`CrudCell`(実装手段の露出)・`AdminActionsCell`(管理者専用に見える)を退けた +- [ ] 3テーブルコンポーネントを更新して上記を import +- [ ] `pnpm check && pnpm dev` で3タブの表示を確認 -### Phase 0: 未テストサービスへのテスト追加(純粋追加、リスクなし)✅ - -- [x] `services/workbook_tasks.test.ts` を新規作成 - - `getWorkBookTasks`: 通常ケース、空配列、comment フィールドの取り扱い - - `validateRequiredFields`: 正常ケース、taskId 欠損、priority 欠損(index 0/中/末/負/小数)、空配列 - - `priority === 0` は `!task.priority` で falsy になりエラーとなる動作を確認・文書化。priority は実際 1 以上なので意図的動作として許容。 - -### Phase 1: +page.svelte から utils へ純粋関数を抽出(低リスク)✅ - -- [x] `utils/workbooks.ts` に `getWorkBooksByType(workbooks, type)` を追加(TODO コメント対応) - - テスト追加(通常フィルタ、空配列、type が一致しない場合) -- [x] `utils/workbooks.ts` に `buildTaskResultsByWorkBookId(workbooks, taskResultsByTaskId)` を追加 - - 現在 +page.svelte の `fetchTaskResultsWithWorkBookId()` に相当 - - テスト追加(task 結果あり/なし、空配列) -- [x] `+page.svelte` を更新: 両関数を import に置き換え - -### Phase 2: +page.server.ts の N+1 修正とクリーンアップ(中リスク)✅ - -- [x] `services/workbooks.ts` に `getWorkBooksWithAuthors()` を追加 - - `include: { user: { select: { username: true } }, workBookTasks: { orderBy: { priority: 'asc' } } }` を使用 - - 戻り値型 `WorkbooksWithAuthors` を `types/workbook.ts` に追加(`authorName: string` を含む) - - 著者削除済みの場合は `user?.username ?? 'unknown'` で対応 -- [x] `+page.server.ts` の `load()` を更新: - - `getWorkBooks()` + N+1 ループ → `getWorkBooksWithAuthors()` に置き換え - - try/catch の範囲を拡大(workbooks 取得も含める) -- [x] CRUD アクションのログを「受信時」→「成功後」に移動(3ファイル) - -**判断根拠**: 現在の `console.log('form -> actions -> ...')` はバリデーション**前**(アクション受信時)に発火するため、不正なリクエストでも記録される。また「どのリソースを誰が操作したか」が分からず本番デバッグに役立たない。成功を確認できる位置に移動し、識別情報を含めることで意味のあるログにする。 - -```ts -// Before(受信時、情報量が低い) -console.log('form -> actions -> delete'); -// ... バリデーション ... -await workBooksCrud.deleteWorkBook(workBookId); - -// After(操作成功後、識別情報を含む) -await workBooksCrud.deleteWorkBook(workBookId); -console.log(`Deleted workbook ${workBookId} by user ${loggedInUser?.id}`); -``` - -対象ファイルと成功後ログの内容: - -| ファイル | ログ内容 | -| ---------------------------------------------- | ---------------------------------------------------------------------------------- | -| `routes/workbooks/+page.server.ts` (delete) | `Deleted workbook ${workBookId} by user ${loggedInUser?.id}` | -| `routes/workbooks/create/+page.server.ts` | `Created workbook "${workBook.title}" by user ${author.id}` | -| `routes/workbooks/edit/[slug]/+page.server.ts` | `Updated workbook ${workBookId}`(edit action に `locals` がなくユーザー情報なし) | - -### Phase 3: サービス層から SvelteKit error() を除去(中リスク) - -`getWorkbookWithAuthor()` の error() 移動 — 方針: - -- スラッグバリデーション(`parseWorkBookId/parseWorkBookUrlSlug`)は**入力バリデーション = ルートの責務** -- サービスは DB アクセスのみ、存在しない場合は `null` を返す -- 2種類のエラー(BAD_REQUEST vs NOT_FOUND)をルートが区別できる構造にする - -``` -// Before: サービスが error() を直接呼ぶ -export async function getWorkbookWithAuthor(slug) { - if (workBookId === null && workBookUrlSlug === null) { - error(BAD_REQUEST, '...'); // ← SvelteKit 依存 - } - if (workBook === null) { - error(NOT_FOUND, '...'); // ← SvelteKit 依存 - } -} - -// After: ルートが責務を持つ -// service: null を返すだけ -// route: slug バリデーション → null チェック → error() 呼び出し -const workBookId = parseWorkBookId(slug); -if (!workBookId) { error(BAD_REQUEST, '...'); } -const workbook = await getWorkbookWithAuthor(slug); -if (!workbook) { error(NOT_FOUND, '...'); } -``` - -- [x] `getWorkbookWithAuthor()` から `error()` 呼び出しを削除、`null` を返すように変更 -- [x] 呼び出し元のルートハンドラーを更新(`src/routes/workbooks/[slug]/+page.server.ts` 等): - - slug バリデーションを呼び出し元に移動 - - null チェック後に `error(BAD_REQUEST/NOT_FOUND, ...)` を呼び出す -- [x] `isExistingWorkBook()` を `db.workBook.count({ where: { id } }) > 0` に変更 - -### Phase 4: services/workbooks.ts の統合テスト追加(Phase 3 後) - -- [ ] `services/workbooks.test.ts` を新規作成(`@quramy/prisma-fabbrica` ファクトリー使用) - - `getWorkBook`: 存在する/しない - - `getWorkBooksWithAuthors`: 著者あり/著者削除済み - - `createWorkBook`: 正常、重複スラッグ - - `updateWorkBook`: 正常、存在しない ID - - `deleteWorkBook`: 正常、存在しない ID - -### Phase 5: WorkBookList.svelte 型修正と Svelte 5 パターン修正(低リスク)✅ - -- [x] `loggedInUser: any` → `interface LoggedInUser { id: string; role: Roles }` を定義(インライン) - - `$lib/types/user.ts` の `User` は `userId` フィールドで不一致のため、最小 interface をコンポーネント内に定義 - - `+page.svelte` から渡す際に `as { id: string; role: Roles }` キャスト(Prisma Roles vs $lib Roles の型差異) -- [x] `$derived(() => countReadableWorkbooks(...))` を `$derived(countReadableWorkbooks(...))` に修正(2箇所) - -### Phase 6: WorkBookBaseTable.svelte を3種コンポーネントに分割(中リスク)✅ - -- [x] 各コンポーネントに同じ `interface Props` を定義(`workbooks`, `workbookGradeModes`, `userId`, `role`, `taskResults`) -- [x] 3コンポーネントを作成: - - `CurriculumTable.svelte` — グレード + タイトル列 - - `SolutionTable.svelte` — タイトル列(padding 広め) - - `CreatedByUserTable.svelte` — 作者 + タイトル列 -- [x] `WorkBookList.svelte` を更新: `Record` ルックアップで切り替え - - `{@const TableComponent}` は `{#if}` の直接の子として配置(HTML 要素内では使用不可) - - 廃止型(TEXTBOOK/GENRE/THEME/OTHERS)は近似の既存コンポーネントにフォールバック -- [x] `WorkBookBaseTable.svelte` を削除 - -### Phase 7: コーナーケーステストの強化(純粋追加)✅ - -- [x] `utils/workbooks.ts` `calcWorkBookGradeModes`: - - タイブレーク: 同頻度グレードが2つある場合は最も易しいグレードを返す(7Q vs 6Q → 7Q、Q2 vs Q1 → Q2) -- [x] 既存テストの `toBeTruthy()`/`toBeFalsy()` を `toBe(true)`/`toBe(false)` に置き換え(coding-style.md 準拠) +### Phase 4: services/workbooks.ts の統合テスト追加 + +_Phase C と独立して実施可能。_ + +- [ ] `services/workbooks.test.ts` を新規作成(`crud.test.ts` と同じモック方式: `vi.mock('$lib/server/database', ...)`) + - `getWorkBook`: found / not found + - `getWorkBooksWithAuthors`: with author / deleted author (`'unknown'` fallback) + - `createWorkBook`: success, duplicate slug + - `updateWorkBook`: success, not found id + - `deleteWorkBook`: success, not found id ### Phase 8: E2E テスト追加(純粋追加) @@ -187,45 +98,3 @@ if (!workbook) { error(NOT_FOUND, '...'); } - [ ] 問題集行に「編集」リンクと「削除」ボタンが表示される **補足**: グレードラベルの文字列(`10Q` 等)は `getTaskGradeLabel()` の戻り値に依存するため、実装時に `workbook_order.test.ts` の既存セレクター(`{ name: '10Q' }`)と一致していることを確認すること。 - ---- - -## 変更ファイル一覧 - -### 新規作成 - -- `src/features/workbooks/services/workbook_tasks.test.ts` -- `src/features/workbooks/services/workbooks.test.ts` -- `src/features/workbooks/components/list/CurriculumTable.svelte` -- `src/features/workbooks/components/list/SolutionTable.svelte` -- `src/features/workbooks/components/list/CreatedByUserTable.svelte` -- `tests/workbooks_list.test.ts` - -### 変更 - -- `src/features/workbooks/services/workbooks.ts` — `getWorkBooksWithAuthors()` 追加、`isExistingWorkBook()` 修正、`getWorkbookWithAuthor()` から error() 除去 -- `src/features/workbooks/types/workbook.ts` — `WorkbooksWithAuthors` 型追加 -- `src/features/workbooks/utils/workbooks.ts` — `getWorkBooksByType()`, `buildTaskResultsByWorkBookId()` 追加 -- `src/features/workbooks/utils/workbooks.test.ts` — 追加テスト -- `src/features/workbooks/components/list/WorkBookList.svelte` — 型修正・Svelte 5 パターン修正・3コンポーネント使用 -- `src/routes/workbooks/+page.svelte` — 関数 import 化 -- `src/routes/workbooks/+page.server.ts` — N+1 修正・ログを成功後に移動 -- `src/routes/workbooks/create/+page.server.ts` — ログを成功後に移動 -- `src/routes/workbooks/edit/[slug]/+page.server.ts` — ログを成功後に移動 -- `src/routes/workbooks/[slug]/+page.server.ts` 等 — error() 呼び出し元更新 - -### 削除 - -- `src/features/workbooks/components/list/WorkBookBaseTable.svelte`(Phase 6 後) - ---- - -## 検証方法 - -```bash -pnpm test:unit # Phase 0, 4, 7 のテスト確認 -pnpm check # Svelte 型チェック(Phase 5, 6) -pnpm lint # ESLint チェック -pnpm test:integration # Phase 8 の E2E テスト確認 -pnpm dev # ローカルで3タブ(カリキュラム/解法別/ユーザ作成)が正常表示されること -``` From 0d43d03e91226a118487bf5ba0a48e60eecd3598 Mon Sep 17 00:00:00 2001 From: Kato Hiroki Date: Thu, 19 Mar 2026 07:51:36 +0000 Subject: [PATCH 04/14] refactor: Workbook list Phase 7-8 + component extraction and test coverage (#3280) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Phase 7: テスト強化(workbooks.test.ts追加、workbooks.test.ts・workbook_tasks.test.ts拡充) - Phase 8: E2Eテスト追加(tests/workbooks_list.test.ts、loginAsAdminをhelpers/auth.tsに抽出) - テーブルコンポーネントから共通セルを抽出(GradeTableBodyCell、WorkbookAuthorActionsCell、WorkbookCompletionCell、WorkbookProgressCell) - coding-style.mdにTSDocルール追加 - testing.mdにルール追加 Co-Authored-By: Claude Sonnet 4.6 --- .claude/rules/coding-style.md | 9 + .claude/rules/testing.md | 18 ++ AGENTS.md | 2 +- .../refactor-for-workbook-list/plan.md | 72 +++---- .../components/list/CreatedByUserTable.svelte | 85 ++------ .../components/list/CurriculumTable.svelte | 100 ++------- .../components/list/GradeTableBodyCell.svelte | 19 ++ .../components/list/SolutionTable.svelte | 97 +++------ .../components/list/WorkBookList.svelte | 1 + .../list/WorkbookAuthorActionsCell.svelte | 34 +++ .../list/WorkbookCompletionCell.svelte | 21 ++ .../list/WorkbookProgressCell.svelte | 26 +++ .../workbooks/services/workbook_tasks.test.ts | 118 ++++++----- .../workbooks/services/workbook_tasks.ts | 5 + .../workbooks/services/workbooks.test.ts | 179 ++++++++++++++++ src/features/workbooks/types/workbook.ts | 11 + .../workbooks/utils/workbooks.test.ts | 196 +++++++++++------- src/features/workbooks/utils/workbooks.ts | 15 +- tests/helpers/auth.ts | 23 ++ tests/workbook_order.test.ts | 18 +- tests/workbooks_list.test.ts | 105 ++++++++++ 21 files changed, 750 insertions(+), 404 deletions(-) create mode 100644 src/features/workbooks/components/list/GradeTableBodyCell.svelte create mode 100644 src/features/workbooks/components/list/WorkbookAuthorActionsCell.svelte create mode 100644 src/features/workbooks/components/list/WorkbookCompletionCell.svelte create mode 100644 src/features/workbooks/components/list/WorkbookProgressCell.svelte create mode 100644 src/features/workbooks/services/workbooks.test.ts create mode 100644 tests/helpers/auth.ts create mode 100644 tests/workbooks_list.test.ts diff --git a/.claude/rules/coding-style.md b/.claude/rules/coding-style.md index 591e9be71..9281f08ed 100644 --- a/.claude/rules/coding-style.md +++ b/.claude/rules/coding-style.md @@ -8,6 +8,15 @@ - **`any`**: before using `any`, check the value's origin — adding a missing `@types/*` or `devDependency` often provides the correct type. - **UI labels**: if a label does not match actual behavior, update it or add an inline comment explaining the intentional mismatch. +## TSDoc + +Add TSDoc comments to every exported function, type, and class. The minimum required fields are `@param` (for non-obvious parameters) and `@returns` (when the return value is not evident from the type). One-liner `/** ... */` is sufficient for simple cases; use multi-line only when behaviour needs explanation. + +```typescript +/** Returns the URL slug for a workbook, falling back to the workbook ID. */ +export function getUrlSlugFrom(workbook: WorkbookList): string { ... } +``` + ## Syntax - **Braces**: always use braces for single-statement `if` blocks. Never `if () return;` — write `if () { return; }`. diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index cd9b06c5e..44d48cf29 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -9,6 +9,10 @@ paths: # Testing +## Test Titles + +Write all test titles in English. Use descriptive sentences that state the expected behavior (e.g., `'returns empty array when workbooks is empty'`). Japanese is only acceptable in inline comments or fixture strings that represent real user-facing content. + ## Test Integrity - Never delete, comment out, or weaken assertions (e.g. `toEqual` → `toBeDefined`) to make tests pass @@ -81,3 +85,17 @@ Stop the split if internal helpers (e.g. `fetchUnplacedWorkbooks`) would be frag ## HTTP Mocking Use Nock for external HTTP calls. See `src/test/lib/clients/` for examples. + +## Flowbite Toggle in E2E Tests + +Flowbite's `Toggle` renders an `sr-only` `` inside a `