diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 900be2ca..0b0cd5dd 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -146,6 +146,9 @@ CI 定義: `.github/workflows/ci.yml` - **`IntegrityError` 後の再 SELECT は `None` を判定する**: ユニーク制約衝突後の再取得で他セッションが先に commit したケースを想定し、`None` ならば明示的に `RuntimeError` を上げる。戻り値型が non-Optional な関数で握りつぶさないこと。 - **タスクハンドラの「黙って return」は禁止**: 失敗パスでは `NonRetryableError` / `RetryableError` を `raise` し、worker に `dead_letter` / `retrying` 遷移と通知発行を任せる。早期 return は呼び出し側に completed として観測される。 - **lint 失敗時は当該ファイルだけ確認**: `make lint-backend` が他ファイルの I001 等で落ちる場合、自分の変更分は `nix develop --command bash -c "cd backend && .venv/bin/python -m ruff check "` で個別検証してから進める（既存違反を巻き込まない）。 +- **Router には「エンドポイント定義・依存性解決・HTTP 変換」のみ**: 外部 API 呼び出し・DB クエリ（`db.query(...)` 直書き）・ビジネスロジックを router に書かない。外部 API の例外は service 層で処理し、router では `raise_app_error` への変換のみ行う。詳細・Bad/Good 例: `.claude/rules/backend/layers.md` +- **ORM model には「テーブル定義・リレーション」のみ**: ソート・フォーマット等の表示ロジックを `@property` として model に持たせない。`sort_utils` のような presentation 層ユーティリティを model に import しない。ソートは `relationship(order_by=...)` か service 層で行う。詳細: `.claude/rules/backend/layers.md` +- **300 行超のコンポーネント・500 行超のサービスモジュールは分割を検討する**: 行数は目安（強制閾値ではない）だが、超過したら責務が複数混在していないかを確認する。モーダル状態や更新ハンドラ群は専用フックに切り出す。詳細・Good パターン例: `.claude/rules/frontend/component-design.md` ## 命名規約 diff --git a/.claude/rules/backend/layers.md b/.claude/rules/backend/layers.md new file mode 100644 index 00000000..94e1b46a --- /dev/null +++ b/.claude/rules/backend/layers.md @@ -0,0 +1,128 @@ +--- +paths: + - backend/** +--- + +# Backend 層の境界ルール + +`backend/architecture.md` が「何があるか」を示すのに対し、このファイルは「各層で何を書いてはいけないか」の禁止事項を補完する。 +新しい負債を発見したら本ファイルの Bad/Good 例と `CLAUDE.md`「失敗から学んだ知見」の両方を更新すること。 + +## 層ごとの責務と禁止事項 + +| 層 | 書いてよいこと | 書いてはいけないこと | +|---|---|---| +| `routers/` | エンドポイント定義・`Depends` による依存性解決・`raise_app_error` / `HTTPException` への変換・rate limit デコレータ | 外部 API 呼び出し・`db.query(...)` 直書き・ビジネスロジック・collector / fetcher の直接 import | +| `services/` | ビジネスロジック・外部 API 呼び出し・ドメイン例外の定義と raise・トランザクション管理 | `HTTPException`（HTTP 知識を service に持ち込まない）・プレゼンテーション整形 | +| `repositories/` | ORM クエリ・CRUD・`db.commit()` / `db.rollback()` | ビジネスロジック・外部 API 呼び出し・HTTP 知識 | +| `models/` | テーブル定義・リレーション・制約・DB レベルの型変換（`format_year_month` 等） | ソート・フォーマット等の表示ロジック・`sort_utils` のような presentation 層ユーティリティの import | + +## 禁止パターンと修正例 + +### パターン A — router への外部 API 例外処理の漏れ + +```python +# Bad: router が collector を直接 import し、外部 API 例外を自前で処理している +# routers/blog/accounts.py +from ...services.blog.collector import ( + BlogPlatformRequestError, + UnsupportedBlogPlatformError, + normalize_username, + verify_user_exists, +) + +@router.post("/accounts") +async def add_account(body: BlogAccountCreate, ...): + try: + normalized = normalize_username(body.platform, body.username) + except UnsupportedBlogPlatformError as exc: + raise HTTPException(status_code=400, ...) from exc + try: + user_exists = await verify_user_exists(body.platform, normalized) + except BlogPlatformRequestError as exc: + raise HTTPException(status_code=502, ...) from exc +``` + +```python +# Good: service 層が外部 API 例外を吸収。router は raise_app_error への変換のみ +# services/blog/account_service.py +async def add_account(self, platform: str, username: str) -> BlogAccount: + normalized = normalize_username(platform, username) # ドメイン例外を raise + user_exists = await verify_user_exists(platform, normalized) # ドメイン例外を raise + if not user_exists: + raise BlogAccountNotFoundError(...) + return self._account_repo.upsert(platform, normalized) + +# routers/blog/accounts.py +@router.post("/accounts") +async def add_account(body: BlogAccountCreate, ...): + service = BlogAccountService(db, user.id) + try: + return await service.add_account(body.platform, body.username) + except BlogPlatformRequestError as exc: + raise_app_error(ErrorCode.EXTERNAL_API_ERROR, ...) from exc + except BlogAccountNotFoundError as exc: + raise_app_error(ErrorCode.NOT_FOUND, ...) from exc +``` + +### パターン B — router 内への DB クエリ直書き + +```python +# Bad: router のモジュールレベル関数が db.query を直接実行している +# routers/github_link.py +def _get_or_create_cache(db: Session, user_id: str) -> GitHubLinkCache: + cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + if not cache: + cache = GitHubLinkCache(user_id=user_id) + db.add(cache) + db.flush() + return cache +``` + +```python +# Good: repository 層に移設。router からは service 経由で呼ぶ +# repositories/github_link.py +class GitHubLinkCacheRepository: + def get_or_create(self, user_id: str) -> GitHubLinkCache: + cache = self.db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + if not cache: + cache = GitHubLinkCache(user_id=user_id) + self.db.add(cache) + self.db.flush() + # IntegrityError 後の再 SELECT が None を返す場合は RuntimeError を上げる + # （CLAUDE.md「失敗から学んだ知見」参照） + return cache +``` + +### パターン C — ORM model への表示ロジック混入 + +```python +# Bad: model が sort_utils を import し @property でソート済みリストを返す +# models/resume.py +from ..services.shared.sort_utils import sort_by_period_desc + +class Resume(Base): + @property + def experiences(self) -> list["ResumeExperience"]: + return sort_by_period_desc(list(self.experience_rows)) +``` + +```python +# Good 案1: relationship に order_by を指定（DB レベルで解決できる場合） +# models/resume.py +experience_rows: Mapped[list["ResumeExperience"]] = relationship( + back_populates="resume", + cascade="all, delete-orphan", + order_by="ResumeExperience.start_date.desc()", +) + +# Good 案2: service 層でソート（動的な条件が必要な場合） +# services/shared/resume_format.py（既存）で sort_by_period_desc を呼ぶ +``` + +## 例外変換の責務ルール + +- 外部 API 固有例外（`BlogPlatformRequestError` 等）は発生するモジュール（collector / fetcher 等）内で定義し、service が raise する +- router では `raise_app_error(...)` か `HTTPException` への変換のみ行う +- service 層は `HTTPException` を import しない。HTTP ステータスコードを service に持ち込まない +- ドメイン例外クラスの定義場所: 例外を raise するモジュールと同じファイルに置く diff --git a/.claude/rules/frontend/component-design.md b/.claude/rules/frontend/component-design.md new file mode 100644 index 00000000..5ee0bfd8 --- /dev/null +++ b/.claude/rules/frontend/component-design.md @@ -0,0 +1,81 @@ +--- +paths: + - frontend/** +--- + +# Frontend コンポーネント設計ルール + +`frontend/architecture.md` が「何があるか」を示すのに対し、このファイルは「コンポーネント設計の判断基準」を補完する。 +行数はあくまで目安（強制閾値ではない）。超過した場合に責務が複数混在していないかを確認するトリガーとして使う。 + +## 行数の目安 + +| 対象 | 目安 | 判断 | +|---|---|---| +| コンポーネント（`.tsx`） | 300 行超 | 分割を検討する | +| コンポーネント（`.tsx`） | 500 行超 | 責務が複数混在している可能性が高い。必ず分割する | +| カスタムフック（`.ts`） | 150 行超 | 分割を検討する | + +- ページコンポーネント（`pages/`）は薄いラッパーを保つ。ロジックはカスタムフックへ移動する +- 「行数が少ないから問題ない」ではなく「責務が1つに絞られているか」を本質的な判断基準とする + +## props drilling の定義と Context 導入の判断基準 + +**props drilling の定義**: 中間コンポーネントが実際には使わない props を、下位コンポーネントへ「素通し」で渡す構造。 + +```tsx +// Bad: ParentForm → ChildSection → GrandchildEditor で同じハンドラ群を素通し +// ChildSection は onUpdateField / onAddItem / focusLocator を自分では使わず下に渡すだけ + +// ChildSection の中身 + +``` + +**Context 導入の判断基準**: +- 同じ props を **3 層以上素通し**する場合は Context または専用フックによる解消を検討する +- 2 層までの素通しは許容（過剰な Context 導入を避ける） +- 「素通し」か「実際に使っている」かを区別する。中間コンポーネントが props を使っていれば drilling ではない + +**Context を導入すべき条件**: +- drilling が 3 層以上 AND 複数の並列コンポーネントが同じ状態・ハンドラを参照する場合 + +## モーダル管理パターン + +親コンポーネントに `useState` でモーダル開閉状態が 3 個以上になったら専用フックに切り出す。 + +```tsx +// Bad: 親コンポーネントに複数のモーダル状態が並ぶ +const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); +const [showSaveConfirm, setShowSaveConfirm] = useState(false); +const [editingField, setEditingField] = useState<"career_summary" | "self_pr" | null>(null); +// さらに PdfPreviewModal の状態も... +``` + +```tsx +// Good: 専用フックに切り出す（useProjectModalState パターンを参照） +// hooks/career/useProjectModalState.ts の設計に倣う +const { isOpen, openModal, closeModal, modalProject } = useProjectModalState(formState); + +// 複数のモーダルを1フックにまとめる（関連度が高い場合） +const { deleteConfirm, saveConfirm, markdownField, openDeleteConfirm, ... } = useCareerFormModals(); +``` + +## 既存の良いパターンへの参照 + +新規実装時は以下を規範として参照する。 + +| パターン | ファイル | 用途 | +|---|---|---| +| フォーム CRUD 共通化 | `hooks/useDocumentForm.ts` | loading/saving/error/cache 状態を一元管理 | +| モーダル状態の切り出し | `hooks/career/useProjectModalState.ts` | モーダル開閉・対象オブジェクト管理の規範例 | +| 更新ハンドラ群の切り出し | `hooks/career/useCareerExperienceMutators.ts` | 複数の mutation ハンドラをフックに集約 | +| 非同期タスク進捗 | `hooks/useTaskPolling.ts` | ポーリングロジックをフックに切り出した例 | +| 汎用 UI コンポーネント | `components/ui/` | 新規共通 UI の置き場（toast/, Skeleton 等が既存） | diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..95d517b1 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,15 @@ +{ + "hooks": { + "Stop": [ + { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "command -v nix >/dev/null 2>&1 && make dupe-check || true" + } + ] + } + ] + } +} diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 00000000..66062683 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,31 @@ +## 変更概要 + + + +## セルフレビューチェックリスト + +### 必須確認 + +- [ ] `make ci` が pass している（lint + test + build-frontend） +- [ ] コメント・ドキュメント・エラーメッセージは日本語で記述した + +### 条件付き確認（該当する場合のみ N/A と記入） + +- [ ] `app/schemas/` または `app/routers/` を変更した場合: `make codegen-types` を実行し `frontend/src/api/generated.ts` の差分をコミットした +- [ ] 新しいページ・認証・ナビゲーション・レイアウトを変更した場合: E2E を実行した（`nix develop --command bash -c "cd frontend && npm run test:e2e"`） +- [ ] 新規環境変数を追加した場合: `env_keys.py` / `docs/api.md` / `infra/modules/cloud_run/main.tf` / `docker-compose.yml` の 4 箇所を同期した +- [ ] `frontend/src/` で日本語メッセージを `frontend/src/constants/messages.ts` の定数経由で参照した（リテラル直書きなし） + +### 破壊的変更 + +- [ ] 破壊的変更なし（API 契約・DB スキーマ・既存の公開インターフェースに変更なし） +- [ ] 破壊的変更あり → 概要: + +### ADR（設計判断を伴う変更の場合のみ） + +- [ ] 新しいライブラリ採用・アーキテクチャ変更を伴う場合、ADR を作成した（または既存 ADR が対応している） + +--- + +PR タイトル形式: `: <内容>`（日本語） +type: `feat` / `fix` / `docs` / `refactor` / `test` / `chore` / `infra` diff --git a/.report/refactoring-plan-20260612.md b/.report/refactoring-plan-20260612.md new file mode 100644 index 00000000..dd173959 --- /dev/null +++ b/.report/refactoring-plan-20260612.md @@ -0,0 +1,309 @@ +# DevForge 全体リファクタリング計画書 + +- **作成日**: 2026-06-12 +- **対象**: backend / frontend / infra / CI / docs の全領域 +- **目的**: 蓄積した技術的負債をフェーズ分けで段階的に解消し、保守性・テスト容易性・CI 効率を改善する +- **調査方法**: 全領域のソースコード静的調査（ファイルサイズ・責務分離・重複・テスト状態・デッドコード） + +--- + +## エグゼクティブサマリー + +調査の結論として、このコードベースは**基盤は健全**（セキュリティ・エラーハンドリング・型生成・ADR 運用は整備済み、TODO/FIXME はほぼゼロ、明確なデッドコードなし）だが、以下の 4 系統の負債が蓄積している。 + +| 系統 | 代表例 | 深刻度 | +|---|---|---| +| **巨大モジュールの責務混在** | FE `CareerResumeForm.tsx`（534 行・4 モーダル同居）、BE `pdf/generators/resume_generator.py`（408 行・HTML+CSS+WeasyPrint 混在） | 高 | +| **層の境界侵食** | BE router へのビジネスロジック漏れ（`blog/accounts.py`・`github_link.py`）、ORM model への表示ロジック混在（`models/resume.py`） | 高 | +| **CI/インフラの構造的重複** | デプロイジョブ 6 個がほぼ同一（dev/stg/prod × FE/BE）、WeasyPrint ライブラリの重複インストール | 中 | +| **テストの偏り** | BE: 1 ファイル 900 行超の巨大テスト・`_extended` 分裂、FE: API 層・Redux slice・E2E エラーパスが手薄 | 中 | + +全 6 フェーズ構成。**各フェーズは独立して PR 化でき、途中で止めても価値が残る**順序で並べている。総工数目安は 40〜60 時間。 + +```text +Phase 0 ─→ Phase 1 ─→ Phase 2 (BE) ─┬─→ Phase 4 (テスト補強)─→ Phase 5 (CI/インフラ) ─→ Phase 6 (仕上げ) + Phase 3 (FE) ─┘ + ※ Phase 2 と 3 は並行可能 +``` + +--- + +## Phase 0: 計測基盤とベースライン確定（前提作業） + +**目的**: 「リファクタの効果を測れる状態」を先に作る。以降の全フェーズの判断基準になる。 + +**工数目安**: 2〜3h ／ **リスク**: なし（コード変更を伴わない） + +### 作業項目 + +| # | 作業 | 対象 | +|---|---|---| +| 0-1 | `make dupe-check` を実行して jscpd ベースラインを確定。`report/dupe/jscpd-report.json` の重複率を本計画書に追記し、各フェーズ完了時に比較する | `.jscpd.json` / `report/dupe/` | +| 0-2 | テストカバレッジの現状値を記録（BE: pytest-cov、FE: vitest coverage）。未導入なら計測だけでも一度実行して数値を控える | `backend/tests/` / `frontend/src/` | +| 0-3 | 行数ベースラインの記録: BE app/ 10,266 行・tests/ 8,454 行、FE src/ 約 17,900 行（generated.ts 除く）を起点として記録 | 本ドキュメント | +| 0-4 | 各フェーズの PR 粒度を確認: 1 PR = 1 フェーズ内の 1〜3 項目を上限とし、`make ci` green を必須ゲートとする | 運用ルール | + +### 完了条件 + +- jscpd ベースライン数値が記録されている +- 以降のフェーズで「重複率・行数・カバレッジが悪化していないこと」を機械的に確認できる + +--- + +## Phase 1: 即効性の高い負債解消（クイックウィン） + +**目的**: リスクが低く独立性の高い項目を先に片付け、後続フェーズのノイズを減らす。 + +**工数目安**: 4〜6h ／ **リスク**: 低（1-1 のみ本番シークレット操作を伴うため要注意） + +### 作業項目 + +| # | 作業 | 対象 | 備考 | +|---|---|---|---| +| 1-1 | **FIELD_ENCRYPTION_KEY の棚卸し TODO 解消**: `infra/modules/cloud_run/main.tf:14-16` に「廃止予定」と明記されたまま残置。Secret Manager・Cloud Run env・`env_keys.py` の 4 箇所同期手順に従って削除、または廃止しない判断なら TODO コメントを更新 | `infra/modules/cloud_run/main.tf` / `backend/app/core/env_keys.py` / `docs/api.md` / `docker-compose.yml` | **破壊的変更の可能性**。本番影響を確認してから着手 | +| 1-2 | **BE テストファイルの統合**: `test_blog_collector.py`（237 行）+ `test_blog_collector_extended.py`（364 行）を 1 ファイルに統合し、重複 fixture を削除 | `backend/tests/` | Phase 2 の collector 分割の前提整理 | +| 1-3 | **ADR の整理**: ADR-0009 の番号重複（textlint / toast の 2 件）の整理方針決定、ADR-0004 に superseded by 0008 の明記 | `docs/adr/` | docs のみ | +| 1-4 | **cleanup スクリプトの導線整備**: `scripts/cleanup_docker_images.sh` / `cleanup_secret_versions.sh` が Makefile にもドキュメントにも未記載。`docs/runbooks/` に手順を記載するか Makefile ターゲット化 | `scripts/` / `Makefile` / `docs/` | | +| 1-5 | **`response_mapper.py`（15 行）の妥当性判断**: 薄すぎるモジュール。`github_link_service.py` への吸収を検討（Rule of Three 観点で利用箇所が 1 つなら吸収） | `backend/app/services/intelligence/` | | + +### 検証 + +```bash +make ci # 1-2, 1-5 +make infra-validate # 1-1 +``` + +### 完了条件 + +- インフラの TODO コメントがゼロ +- `_extended` 命名のテストファイルがゼロ +- ADR の番号体系と supersede 関係が一貫 + +--- + +## Phase 2: Backend 責務分離（コア） + +**目的**: 層の境界（router / service / repository / model）を回復し、巨大モジュールを分割する。本計画の本丸その 1。 + +**工数目安**: 10〜14h ／ **リスク**: 中（契約変更を伴わない内部リファクタに限定すれば低い） + +**実行手段**: `BE_refacter` スキルで詳細レビュー → `BE_apply` で適用、の既存フローに乗せることを推奨。 + +### 2A. 巨大モジュールの分割 + +| # | 作業 | 現状 | 分割案 | +|---|---|---|---| +| 2A-1 | **PDF generator の 3 分割** | `services/pdf/generators/resume_generator.py`（408 行）に HTML 組み立て・CSS ロード・WeasyPrint 統合が混在 | `resume_generator_html.py`（HTML 組み立て）/ CSS・フォント定義モジュール / `resume_generator.py`（WeasyPrint 統合・公開 API）。公開 API のシグネチャは変えない | +| 2A-2 | **blog/collector の 2 層分割** | `services/blog/collector.py`（311 行）に Zenn/note/Qiita の HTTP fetch・正規化・存在確認が同居 | `blog/fetcher.py`（プラットフォーム別 HTTP fetch）+ `blog/account_service.py`（既存 53 行を拡張: 正規化・存在確認・登録オーケストレーション） | +| 2A-3 | **contributions.py の分割** | `services/intelligence/github/contributions.py`（249 行）に GitHub API 呼び出しと分析ロジックが混在 | `github/contributions.py`（API 呼び出しのみ）+ `intelligence/contribution_analyzer.py`（分析ロジック） | + +### 2B. 層の境界回復 + +| # | 作業 | 現状の問題 | 修正方針 | +|---|---|---|---| +| 2B-1 | **blog/accounts router からロジック除去** | `routers/blog/accounts.py:75-96` で `normalize_username()` / `verify_user_exists()` を collector から直接 import し、外部 API の例外→HTTPException 変換が router に露出 | 2A-2 の `account_service` 経由に変更。例外→HTTP コード変換は `core/errors.py` の既存機構（`raise_app_error`）に寄せる | +| 2B-2 | **github_link router のキャッシュ初期化を repository へ** | `routers/github_link.py` の `_get_or_create_cache()` が router 内に DB クエリを持つ | repository（または `dispatch_service`）へ移設。`IntegrityError` 後の再 SELECT `None` 判定ルール（CLAUDE.md「失敗から学んだ知見」）を遵守 | +| 2B-3 | **Resume model から表示ロジック除去** | `models/resume.py`（336 行）が `sort_utils` を import し、`@property` でソート済みリストを返す | ソートは利用側（schema 変換 or service）で `sort_utils` を直接呼ぶ。model は ORM 定義に専念 | +| 2B-4 | **blog repository の `upsert_many` 簡素化** | `repositories/blog.py` の `upsert_many()` が正規化＋複数アカウント横断バッチを抱える | 正規化は service 層（2A-2 の account_service / sync_service）へ引き上げ、repository は単純な upsert に縮小 | + +### 注意事項 + +- `app/services/agent/` と `tasks/` は調査の結果**設計良好**（ADR-0010 準拠・責務分離済み）。**このフェーズでは触らない**。agent 配下を触る場合は `.claude/rules/backend/agent.md` の事前読了が必須 +- `app/schemas/` / `app/routers/` のシグネチャ・docstring に触れた場合は **`make codegen-types` → `frontend/src/api/generated.ts` のコミットが必須**（codegen-drift CI） +- タスクハンドラの黙殺 return 禁止・例外握りつぶし禁止ルールを分割時に維持する + +### 検証 + +```bash +make codegen-types && git diff frontend/src/api/generated.ts # スキーマ/ルーター変更時 +make ci +``` + +### 完了条件 + +- 400 行超の BE モジュール（テスト・マイグレーション除く）がゼロ +- router 内の DB クエリ・外部 API 例外ハンドリングがゼロ +- model が `sort_utils` を import していない +- jscpd 重複率が Phase 0 ベースラインから悪化していない + +--- + +## Phase 3: Frontend 責務分離（コア・Phase 2 と並行可能） + +**目的**: ページコンポーネントの肥大化と props drilling を解消する。本丸その 2。 + +**工数目安**: 10〜14h ／ **リスク**: 中（UI 挙動の回帰リスク → E2E 必須） + +**実行手段**: `FE_refacter` スキルで詳細レビュー → `FE_apply` で適用を推奨。 + +### 3A. 巨大コンポーネント・フックの分割 + +| # | 作業 | 現状 | 分割案 | +|---|---|---|---| +| 3A-1 | **CareerResumeForm の分割** | `components/forms/CareerResumeForm.tsx`（534 行）に 10+ フック・3 モーダル状態・4 モーダルコンポーネント・セクション組立・ドラフト復元が同居 | モーダル群を `CareerResumeFormModals.tsx`（仮）へ分離し、開閉状態は専用フック（`useCareerFormModals` 等）に集約。目標 350 行以下 | +| 3A-2 | **useCareerDirty の分割** | `hooks/career/useCareerDirty.ts`（273 行）が experience/client/project/qualification の 4 型の dirty 判定を 5 層ネストで一元処理 | 階層別フック（`useExperienceDirty` / `useProjectDirty` 等）に分割。deep equal 比較の単体テストを各階層に付ける | +| 3A-3 | **useCareerExperienceMutators の分割** | `hooks/career/useCareerExperienceMutators.ts`（210 行）が 13 個のハンドラを返す | client 系ミューテータを `useClientMutators` へ分離 | +| 3A-4 | **AgentChatWidget の UI/ロジック分離** | `components/forms/AgentChatWidget.tsx`（351 行）にパネル操作（ドラッグ・リサイズ）とチャット送受信が混在 | パネル操作を独立フック/コンポーネントへ。チャットロジックは既存 `useAgentChat` へ寄せる | + +### 3B. props drilling の解消 + +| # | 作業 | 現状 | 修正方針 | +|---|---|---|---| +| 3B-1 | **CareerFormContext の導入** | `CareerExperienceEditor.tsx`（327 行・12+ props）→ `ClientEditor` へ 6+ ハンドラを素通しで 3 層 drilling | mutation ハンドラ群を Context で一括提供。3A-3 の分割後に実施すると Context の形が決めやすい | + +### 3C. バリデーションの階層化 + +| # | 作業 | 現状 | 修正方針 | +|---|---|---|---| +| 3C-1 | **payloadBuilders のバリデータ分割** | `payloadBuilders.ts`（448 行）の `validateCareerForm()` が 5 層を all-in-one 検証 | experience / client / project 層別のバリデータへ分割。`payloadBuilders.test.ts`（838 行）も対応して分割 | + +### 注意事項 + +- **E2E 必須トリガーに該当**（レイアウト・フォームフロー変更）。`nix develop --command bash -c "cd frontend && npm run test:e2e"` を各 PR で実行 +- メッセージは引き続き `constants/messages.ts` 経由。リテラル直書きは `make lint-frontend-messages` で検知される +- `api/generated.ts` は触らない（codegen 管理） + +### 完了条件 + +- 400 行超の FE コンポーネント・フック（テスト・generated 除く）がゼロ +- `CareerExperienceEditor` → `ClientEditor` のハンドラ素通し props がゼロ +- E2E 全シナリオ green + +--- + +## Phase 4: テスト補強（Phase 2・3 の変更を固定化） + +**目的**: リファクタ後の構造を回帰テストで固定し、調査で判明した手薄領域を埋める。 + +**工数目安**: 8〜10h ／ **リスク**: 低 + +### Backend + +| # | 作業 | 対象 | +|---|---|---| +| 4-1 | **巨大テストの責務分割**: `test_agent.py`（921 行）・`test_endpoints.py`（427 行）・`test_schemas.py`（425 行）を対象モジュール単位に分割。テスト名と実装の対応を回復 | `backend/tests/` | +| 4-2 | **drift 検知テストの横展開**: `test_scope_limits_match_resume_schema` 型の schema↔model 整合テストを他の model/schema ペアに拡張 | `backend/tests/` | + +### Frontend + +| # | 作業 | 対象 | +|---|---|---| +| 4-3 | **API ドメイン別テスト追加**: 現状 `client.test.ts` のみ。`resumes` / `blog` / `githubLink` 各 API の成功・失敗・リトライをテスト | `frontend/src/api/` | +| 4-4 | **Redux formCacheSlice のテスト追加**: cache/clear/persist のテストが皆無。ページ遷移時のデータ喪失を回帰防止 | `frontend/src/store/` | +| 4-5 | **CareerFormEditors のコンポーネントテスト**: Experience/Client エディタはテストゼロ。Phase 3 の分割後の形でテストを書く | `frontend/src/components/forms/CareerFormEditors/` | +| 4-6 | **E2E エラーパス追加**: バリデーション失敗・ネットワークエラー・タイムアウトのシナリオ（現状ゴールデンパスのみ） | `frontend/e2e/` | + +### 注意事項 + +- **DB をモックしない**（CLAUDE.md: 統合テストは実 DB のテスト用 SQLite セッションに当てる） +- 契約を変えた箇所は旧契約を固定化したテストの assert・テスト名の両方を見直す + +### 完了条件 + +- 500 行超のテストファイルがゼロ（分割困難な統合テストは例外として明記） +- FE API 層・Redux slice にテストが存在 +- カバレッジが Phase 0 ベースラインから向上 + +--- + +## Phase 5: CI / インフラ最適化 + +**目的**: CI の構造的重複を除去し、実行時間を短縮する。アプリコードと独立なので最後でよいが、効果は全 PR に波及する。 + +**工数目安**: 6〜8h ／ **リスク**: 中（デプロイパイプライン変更のため stg で先行検証） + +**実行手段**: infra 部分は `INFRA_refacter` → `INFRA_apply` フローを推奨。 + +### CI（.github/workflows/ci.yml: 591 行・15 ジョブ） + +| # | 作業 | 現状 | 修正方針 | +|---|---|---|---| +| 5-1 | **デプロイジョブの統合** | `deploy-frontend` / `-stg` / `-prod` と `deploy-backend` / `-stg` / `-prod` の 6 ジョブがほぼ同一（差分は env/secrets のみ） | reusable workflow（`workflow_call` + environment 入力）または matrix 化で 2 ジョブに集約。**dev → stg → prod の順に 1 環境ずつ検証してから展開** | +| 5-2 | **WeasyPrint インストールの共通化** | `test-backend`（L199-204）と `codegen-drift`（L249-254）で同一の apt-get install を重複実行 | composite action 化 + apt キャッシュ。flake.nix（L38-46）のライブラリ一覧との二重管理はコメントで相互参照を明記 | +| 5-3 | **Playwright ブラウザのキャッシュ** | `test-e2e` で毎回 `npx playwright install chromium --with-deps` | `~/.cache/ms-playwright` を actions/cache でキャッシュ | + +### インフラ（OpenTofu） + +| # | 作業 | 現状 | 修正方針 | +|---|---|---|---| +| 5-4 | **cloud_run env ブロックの整理** | `infra/modules/cloud_run/main.tf`（190 行）で env ブロック 30 行・secret_env 6 個が逐次列挙 | `dynamic "env"` + locals の map 化で宣言的に。`env_keys.py` / `docs/api.md` との 4 箇所同期の正本関係をコメントで明記 | +| 5-5 | **symlink 整合性の自動検証** | `environments/{dev,stg,prod}` の symlink 統合は手動運用。新規ファイル追加時に壊れても気づけない | pre-commit hook または CI ステップで symlink 先の存在チェックを追加。`.jscpd.json` の ignore 追記漏れも同時に検知 | + +### 注意事項 + +- デプロイジョブ変更は **必ず dev 環境で 1 度デプロイを通してから** stg / prod に展開 +- セキュリティ上の意図がある「デプロイジョブで npm キャッシュを復元しない」設計は維持する（統合時に消さないこと） + +### 完了条件 + +- ci.yml のデプロイ系ジョブ定義が 6 → 2（+ 呼び出し）に削減 +- WeasyPrint install の記述が 1 箇所 +- `make infra-validate` green、dev 環境での実デプロイ成功 + +--- + +## Phase 6: 仕上げ・再発防止 + +**目的**: リファクタ成果を制度として固定し、負債の再蓄積を防ぐ。 + +**工数目安**: 3〜4h ／ **リスク**: 低 + +| # | 作業 | 内容 | +|---|---|---| +| 6-1 | **jscpd threshold の引き上げ** | `.jscpd.json` は現在 Phase 1 運用（`threshold` warn-only）。Phase 0 比で改善したベースラインを元に fail 閾値を設定し、CI の `detect-duplication` を enforcing に切り替え | +| 6-2 | **ファイルサイズの lint 化検討** | 「400 行超で警告」のような機械チェックを ESLint（`max-lines`）/ ruff 系で導入するか判断。導入しない場合も判断理由を記録 | +| 6-3 | **ADR の起票** | 本リファクタで行った構造判断（CareerFormContext 導入、CI reusable workflow 化など）のうち ADR に値するものを `CONTRIBUTING.md` の運用ルールに従って起票 | +| 6-4 | **docs 更新** | `docs/development.md` / `.claude/rules/` のうち、分割後のディレクトリ構成・新モジュール配置に言及している箇所を更新 | +| 6-5 | **効果測定** | Phase 0 のベースライン（重複率・行数・カバレッジ・CI 実行時間）と最終値を比較し、本ドキュメント末尾に結果を追記 | + +--- + +## 触らないと決めたもの（スコープ外） + +調査の結果、以下は健全と判断したため**意図的にスコープ外**とする。「動いているものを壊さない」ため。 + +| 領域 | 理由 | +|---|---| +| `backend/app/services/agent/`・`tasks/` | ADR-0010 準拠で責務分離済み。worker のセッション管理は複雑だが Hrana 失効対策としてコメント・テスト完備 | +| `backend/app/core/`（errors / settings / env_keys / security） | エラーコード一元化・環境変数管理は SSoT として機能している | +| `frontend/src/api/client.ts`・`constants/messages.ts` 系 | 401 リフレッシュ・CSRF・メッセージ SSoT は設計良好。ESLint 監視も機能 | +| `infra/environments/` の symlink 統合 | 物理統合済み。Phase 5-5 の自動検証追加のみ | +| `monitoring/` モジュール | 責務別ファイル分割済み | +| タスクハンドラ抽象（handlers/base.py） | 実装が GITHUB_LINK 1 種でややオーバーエンジニアリングだが、削るコストの方が高い。新規タスク追加時に再評価 | +| PDF/Markdown generator の期間フォーマット差異 | 出力形式依存の意図的な分離（偶発的重複ではない） | + +--- + +## 運用ルール（全フェーズ共通） + +1. **1 PR = 1〜3 作業項目**。フェーズをまたぐ PR は作らない +2. 各 PR で `make ci` green が必須。schema/router 変更時は `make codegen-types`、UI フロー変更時は E2E を追加実行 +3. ブランチは `refactor/` を `origin/main` 起点で作成 +4. 各フェーズ完了時に `make dupe-check` を実行し、重複率がベースラインから悪化していないことを確認 +5. 既存の skill フロー（`BE_refacter`→`BE_apply`、`FE_refacter`→`FE_apply`、`INFRA_refacter`→`INFRA_apply`、領域横断は `XR_refacter`）に乗せられる項目は乗せる +6. 「形は同じだが変更理由が違う」コードは抽出しない（`.claude/rules/common/duplication.md` の偶発的重複ポリシー遵守） + +## 工数サマリー + +| フェーズ | 内容 | 工数目安 | 依存 | +|---|---|---|---| +| Phase 0 | 計測基盤・ベースライン | 2〜3h | なし | +| Phase 1 | クイックウィン | 4〜6h | Phase 0 | +| Phase 2 | Backend 責務分離 | 10〜14h | Phase 1 | +| Phase 3 | Frontend 責務分離 | 10〜14h | Phase 1（Phase 2 と並行可） | +| Phase 4 | テスト補強 | 8〜10h | Phase 2・3 | +| Phase 5 | CI/インフラ最適化 | 6〜8h | なし（いつでも可、推奨は Phase 4 後） | +| Phase 6 | 仕上げ・再発防止 | 3〜4h | Phase 5 | +| **合計** | | **43〜59h** | | + +## ベースライン記録欄（Phase 0 で記入） + +| 指標 | Phase 0 時点 | 最終 | +|---|---|---| +| jscpd 重複率（全体） | （未計測） | | +| BE app/ 行数 | 10,266 | | +| BE tests/ 行数 | 8,454 | | +| FE src/ 行数（generated 除く） | 約 17,900 | | +| BE テストケース数 | 409 | | +| FE 単体テストファイル数 | 40 | | +| CI 所要時間（main push） | （未計測） | | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 31dafd09..b1640920 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,18 +4,19 @@ | ブランチ | 用途 | |---|---| -| `main` | 本番リリース済みコード | -| `develop` | 開発統合ブランチ | +| `main` | 本番リリース済みコード（PR のマージ先） | | `feature/*` | 機能開発 | | `fix/*` | バグ修正 | | `docs/*` | ドキュメント変更 | | `refactor/*` | リファクタリング | | `infra/*` | インフラ変更 | +> **Note**: 以前は `develop` ブランチを開発統合ブランチとして使っていたが、dev 環境廃止に伴い廃止済み。すべてのブランチを `main` へ直接マージする運用。詳細は `CLAUDE.md`「新規ブランチは `origin/main` 起点で切る」を参照。 + ## PR の作り方 -- `develop` ブランチに向けて PR を作成する -- PR タイトルは `: <内容>` の形式（例: `feat: GitHub 連携スコア計算の追加`） +- `main` ブランチに向けて PR を作成する +- PR タイトルは `: <内容>` の形式（例: `feat: GitHub 連携スコア計算の追加`） - セルフレビュー後にマージする ## ADR（Architecture Decision Record） @@ -74,4 +75,8 @@ docs/adr/XXXX-kebab-case-title.md | [ADR-0004](docs/adr/0004-llm-provider-abstraction.md) | LLM プロバイダ抽象化（Ollama/Vertex AI） | Superseded by ADR-0008 | | [ADR-0005](docs/adr/0005-cloudrun-single-instance.md) | Cloud Run single instance 構成の採用 | Accepted | | [ADR-0006](docs/adr/0006-tanstack-query.md) | TanStack Query 導入検討 | Proposed | -| [ADR-0008](docs/adr/0008-remove-llm-to-rule-based-design.md) | LLM プロバイダ抽象化の撤去とルールベース設計への統一 | Accepted | +| [ADR-0007](docs/adr/0007-openapi-typescript-codegen.md) | OpenAPI → TypeScript 型生成（codegen-drift CI） | Accepted | +| [ADR-0008](docs/adr/0008-remove-llm-to-rule-based-design.md) | LLM プロバイダ抽象化の撤去とルールベース設計への統一 | Superseded by ADR-0010 | +| [ADR-0009](docs/adr/0009-frontend-toast-notification.md) | フロントエンドのトースト通知統一 | Accepted | +| [ADR-0011](docs/adr/0011-frontend-textlint-proofread.md) | フロントエンド完結型文章校正（textlint） | Accepted | +| [ADR-0010](docs/adr/0010-devforge-agent.md) | DevForge Agent 機能の導入 | Accepted | diff --git a/backend/app/models/resume.py b/backend/app/models/resume.py index f9b53e6c..5eda3fd1 100644 --- a/backend/app/models/resume.py +++ b/backend/app/models/resume.py @@ -16,7 +16,6 @@ from ..core.date_utils import format_year_month from ..db import Base -from ..services.shared.sort_utils import sort_by_date_asc, sort_by_period_desc class Resume(Base): @@ -59,13 +58,13 @@ class Resume(Base): @property def experiences(self) -> list["ResumeExperience"]: - """経歴を在籍期間の降順でソートして返す。""" - return sort_by_period_desc(list(self.experience_rows)) + """経歴を返す（save 時に sort_order で在籍期間降順が確定済み）。""" + return list(self.experience_rows) @property def qualifications(self) -> list["ResumeQualification"]: - """資格を取得日の昇順でソートして返す。""" - return sort_by_date_asc(list(self.qualification_rows), date_key="acquired_date_value") + """資格を返す（save 時に sort_order で取得日昇順が確定済み）。""" + return list(self.qualification_rows) class ResumeQualification(Base): @@ -166,8 +165,8 @@ class ResumeClient(Base): @property def projects(self) -> list["ResumeProject"]: - """プロジェクトを期間の降順でソートして返す。""" - return sort_by_period_desc(list(self.project_rows)) + """プロジェクトを返す（save 時に sort_order で期間降順が確定済み）。""" + return list(self.project_rows) @property def vacation_start_date(self) -> str: diff --git a/backend/app/routers/blog/accounts.py b/backend/app/routers/blog/accounts.py index 5a7a3c8a..250a6de5 100644 --- a/backend/app/routers/blog/accounts.py +++ b/backend/app/routers/blog/accounts.py @@ -16,13 +16,14 @@ BlogAccountUpdate, BlogArticleResponse, ) -from ...services.blog.account_service import BlogAccountService +from ...services.blog.account_service import ( + BlogAccountAlreadyRegisteredError, + BlogAccountService, +) from ...services.blog.collector import ( BlogAccountNotFoundError, BlogPlatformRequestError, UnsupportedBlogPlatformError, - normalize_username, - verify_user_exists, ) logger = logging.getLogger(__name__) @@ -51,16 +52,14 @@ async def add_account( """連携アカウントを登録する。 同じプラットフォームは1つまで。ユーザー存在チェックあり。 """ - repo = BlogAccountRepository(db, user.id) - existing = repo.get_by_platform(body.platform) - if existing: + service = BlogAccountService(db, user.id) + try: + return await service.add_account(body.platform, body.username) + except BlogAccountAlreadyRegisteredError as exc: raise HTTPException( status_code=409, detail=get_error("blog.account_already_registered"), - ) - - try: - normalized_username = normalize_username(body.platform, body.username) + ) from exc except UnsupportedBlogPlatformError as exc: raise HTTPException( status_code=400, @@ -71,29 +70,16 @@ async def add_account( status_code=404, detail=get_error("blog.account_not_found"), ) from exc - - # 外部プラットフォーム上にユーザーが存在するか検証 - try: - user_exists = await verify_user_exists(body.platform, normalized_username) - except UnsupportedBlogPlatformError as exc: - raise HTTPException( - status_code=400, - detail=get_error("blog.platform_not_supported"), - ) from exc except BlogPlatformRequestError as exc: raise HTTPException( status_code=502, detail=get_error("blog.account_check_failed"), ) from exc - - if not user_exists: + except BlogAccountNotFoundError as exc: raise HTTPException( status_code=404, detail=get_error("blog.account_not_found"), - ) - - account = repo.upsert(body.platform, normalized_username) - return account + ) from exc @router.patch("/accounts/{platform}", response_model=BlogAccountResponse) diff --git a/backend/app/routers/github_link.py b/backend/app/routers/github_link.py index 48abdb43..55183401 100644 --- a/backend/app/routers/github_link.py +++ b/backend/app/routers/github_link.py @@ -23,6 +23,7 @@ ProgressResponse, ) from ..schemas.shared import TaskAcceptedResponse, TaskStatusResponse +from ..services.intelligence.github_link_service import get_or_create_github_link_cache from ..services.tasks import AsyncTaskCacheService, TaskType logger = logging.getLogger(__name__) @@ -40,16 +41,6 @@ def _raise_dispatch_failed() -> None: ) -def _get_or_create_cache(db: Session, user_id: str) -> GitHubLinkCache: - """ユーザーのキャッシュレコードを取得、なければ作成する。""" - cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() - if not cache: - cache = GitHubLinkCache(user_id=user_id) - db.add(cache) - db.flush() - return cache - - def require_github_user(user: User = Depends(get_current_user)) -> User: """GitHub 連携には GitHub ログイン（``github_id`` 保持）が必須。未連携なら 403。 @@ -126,7 +117,7 @@ async def start_github_link( github_username = user.username # 進行中のタスクがあればそのステータスを返す - cache = _get_or_create_cache(db, user.id) + cache = get_or_create_github_link_cache(db, user.id) service = AsyncTaskCacheService(db, cache) # DB 最新状態を取得しつつ pending へアトミック遷移。進行中なら早期リターン diff --git a/backend/app/services/blog/account_service.py b/backend/app/services/blog/account_service.py index 403beccf..59371ed7 100644 --- a/backend/app/services/blog/account_service.py +++ b/backend/app/services/blog/account_service.py @@ -1,4 +1,4 @@ -"""ブログ連携アカウントの更新サービス。""" +"""ブログ連携アカウントの登録・更新サービス。""" from sqlalchemy.orm import Session @@ -13,6 +13,10 @@ ) +class BlogAccountAlreadyRegisteredError(ValueError): + """同じプラットフォームのアカウントが既に登録済みの場合の例外。""" + + class BlogAccountService: """ブログ連携アカウントの更新処理を扱う。""" @@ -25,6 +29,24 @@ def __init__(self, db: Session, user_id: str) -> None: def get_by_platform(self, platform: str) -> BlogAccount | None: return self._account_repo.get_by_platform(platform) + async def add_account(self, platform: str, username: str) -> BlogAccount: + """新規ブログアカウントを登録する。 + + 既に同じプラットフォームが登録済みなら BlogAccountAlreadyRegisteredError を raise する。 + 外部プラットフォームにユーザーが存在しない場合は BlogAccountNotFoundError を raise する。 + """ + existing = self._account_repo.get_by_platform(platform) + if existing: + raise BlogAccountAlreadyRegisteredError(platform) + + normalized_username = normalize_username(platform, username) + + user_exists = await verify_user_exists(platform, normalized_username) + if not user_exists: + raise BlogAccountNotFoundError(f"アカウントが見つかりません: {platform}/{username}") + + return self._account_repo.upsert(platform, normalized_username) + async def update_username(self, platform: str, username: str) -> BlogAccount: account = self._account_repo.get_by_platform(platform) if not account: diff --git a/backend/app/services/intelligence/github_link_service.py b/backend/app/services/intelligence/github_link_service.py index 8fec367d..52e0992c 100644 --- a/backend/app/services/intelligence/github_link_service.py +++ b/backend/app/services/intelligence/github_link_service.py @@ -10,6 +10,8 @@ from datetime import datetime, timezone +from sqlalchemy.orm import Session + from ...core.encryption import decrypt_field from ...core.logging_utils import get_logger from ...core.messages import get_error @@ -31,6 +33,16 @@ def _now() -> datetime: return datetime.now(timezone.utc) +def get_or_create_github_link_cache(db: Session, user_id: str) -> GitHubLinkCache: + """ユーザーの GitHubLinkCache レコードを取得し、存在しなければ作成する。""" + cache = db.query(GitHubLinkCache).filter_by(user_id=user_id).first() + if not cache: + cache = GitHubLinkCache(user_id=user_id) + db.add(cache) + db.flush() + return cache + + async def run_github_link(session_factory: SessionFactory, payload: dict) -> None: """GitHub 連携パイプラインを実行し、結果をキャッシュに保存する。 diff --git a/backend/tests/blog/test_accounts.py b/backend/tests/blog/test_accounts.py index 72341eb1..ca28fe8b 100644 --- a/backend/tests/blog/test_accounts.py +++ b/backend/tests/blog/test_accounts.py @@ -10,7 +10,7 @@ from conftest import auth_header # テスト中は外部 API 呼び出しをモックし、常にユーザーが存在する扱いにする -_VERIFY_PATCH = "app.routers.blog.accounts.verify_user_exists" +_VERIFY_PATCH = "app.services.blog.account_service.verify_user_exists" def test_add_blog_account(client: TestClient) -> None: diff --git a/backend/tests/blog/test_sync.py b/backend/tests/blog/test_sync.py index c56e29d3..deacadf0 100644 --- a/backend/tests/blog/test_sync.py +++ b/backend/tests/blog/test_sync.py @@ -9,7 +9,7 @@ from conftest import auth_header -_VERIFY_PATCH = "app.routers.blog.accounts.verify_user_exists" +_VERIFY_PATCH = "app.services.blog.account_service.verify_user_exists" def test_sync_requires_auth(client: TestClient) -> None: diff --git a/backend/tests/security/test_idor.py b/backend/tests/security/test_idor.py index fe07d5ef..526d3288 100644 --- a/backend/tests/security/test_idor.py +++ b/backend/tests/security/test_idor.py @@ -96,7 +96,7 @@ def test_blog_account_patch_does_not_touch_other_user_data( headers_b = auth_header(client, "idor-blog-patch-b") # 早期 404 のため verify_user_exists には到達しないが、念のためモック with patch( - "app.routers.blog.accounts.verify_user_exists", + "app.services.blog.account_service.verify_user_exists", new_callable=AsyncMock, return_value=True, ): diff --git a/backend/tests/security/test_mass_assignment.py b/backend/tests/security/test_mass_assignment.py index cb3f06d0..e3addd91 100644 --- a/backend/tests/security/test_mass_assignment.py +++ b/backend/tests/security/test_mass_assignment.py @@ -11,7 +11,7 @@ from conftest import auth_header, make_resume_payload -_ACCOUNT_VERIFY_PATCH = "app.routers.blog.accounts.verify_user_exists" +_ACCOUNT_VERIFY_PATCH = "app.services.blog.account_service.verify_user_exists" _SERVICE_VERIFY_PATCH = "app.services.blog.account_service.verify_user_exists" diff --git a/docs/adr/0009-frontend-textlint-proofread.md b/docs/adr/0011-frontend-textlint-proofread.md similarity index 100% rename from docs/adr/0009-frontend-textlint-proofread.md rename to docs/adr/0011-frontend-textlint-proofread.md diff --git a/frontend/src/components/forms/CareerResumeForm.tsx b/frontend/src/components/forms/CareerResumeForm.tsx index 3af815f3..ac157326 100644 --- a/frontend/src/components/forms/CareerResumeForm.tsx +++ b/frontend/src/components/forms/CareerResumeForm.tsx @@ -1,6 +1,8 @@ import { CSSProperties, FormEvent, useCallback, useEffect, useMemo, useRef, useState } from "react"; import type { Dispatch, SetStateAction } from "react"; +import { useCareerFormModals } from "../../hooks/career/useCareerFormModals"; + import { createCareerResume, deleteCareerResume, @@ -49,13 +51,8 @@ import { CareerSelfPrSection } from "./sections/CareerSelfPrSection"; export function CareerResumeForm({ isAuthenticated }: { isAuthenticated: boolean }) { // 未ログインで要ログイン機能を使おうとしたときに開く共通モーダル。 const requestLogin = useLoginPrompt(); - const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); - // 保存時の変更点確認ダイアログの表示状態。 - const [showSaveConfirm, setShowSaveConfirm] = useState(false); // PDF 原本ビュー（右カラム）の折りたたみ状態。折りたたむと入力フォームが全幅に広がる。 const [pdfCollapsed, setPdfCollapsed] = useState(false); - // 自己PR / 職務要約の入力モーダルの対象フィールド（null で閉じている）。 - const [editingField, setEditingField] = useState<"career_summary" | "self_pr" | null>(null); const assist = useResumeImportAssist(); const splitRef = useRef(null); const { width: pdfWidth, startResize } = useImportPanelLayout(splitRef); @@ -97,6 +94,17 @@ export function CareerResumeForm({ isAuthenticated }: { isAuthenticated: boolean skipLoad: !isAuthenticated, }); + const { + showDeleteConfirm, + setShowDeleteConfirm, + showSaveConfirm, + setShowSaveConfirm, + editingField, + setEditingField, + handleDelete, + handleConfirmSave, + } = useCareerFormModals({ save, deleteDoc }); + // ログイン後（往復から復帰）に退避ドラフトを復元する情報トースト用メッセージ。 const [restoreMessage, setRestoreMessage] = useState(null); @@ -260,20 +268,9 @@ export function CareerResumeForm({ isAuthenticated }: { isAuthenticated: boolean setShowSaveConfirm(true); }; - /** 確認ダイアログで「この内容で保存」を押したときの確定処理。 */ - const handleConfirmSave = async () => { - await save(); - setShowSaveConfirm(false); - }; - const focusLocator = focusTarget?.locator ?? null; const focusNonce = focusTarget?.nonce ?? 0; - const handleDelete = async () => { - await deleteDoc(); - setShowDeleteConfirm(false); - }; - /** * 要ログイン機能（プレビュー / PDF / Markdown 出力）のハンドラ。 * 未ログインならログイン促進モーダルを開き、ログイン済みなら本来の処理を行う。 diff --git a/frontend/src/hooks/career/useCareerFormModals.test.ts b/frontend/src/hooks/career/useCareerFormModals.test.ts new file mode 100644 index 00000000..cf3b90a1 --- /dev/null +++ b/frontend/src/hooks/career/useCareerFormModals.test.ts @@ -0,0 +1,88 @@ +import { renderHook, act } from "@testing-library/react"; +import { describe, it, expect, vi } from "vitest"; + +import { useCareerFormModals } from "./useCareerFormModals"; + +describe("useCareerFormModals", () => { + const makeDeps = () => ({ + save: vi.fn().mockResolvedValue(undefined), + deleteDoc: vi.fn().mockResolvedValue(undefined), + }); + + it("削除確認モーダルの開閉", () => { + const { result } = renderHook(() => useCareerFormModals(makeDeps())); + + expect(result.current.showDeleteConfirm).toBe(false); + + act(() => { + result.current.setShowDeleteConfirm(true); + }); + expect(result.current.showDeleteConfirm).toBe(true); + + act(() => { + result.current.setShowDeleteConfirm(false); + }); + expect(result.current.showDeleteConfirm).toBe(false); + }); + + it("保存確認モーダルの開閉", () => { + const { result } = renderHook(() => useCareerFormModals(makeDeps())); + + act(() => { + result.current.setShowSaveConfirm(true); + }); + expect(result.current.showSaveConfirm).toBe(true); + }); + + it("handleDelete: deleteDoc を呼び、モーダルを閉じる", async () => { + const deps = makeDeps(); + const { result } = renderHook(() => useCareerFormModals(deps)); + + act(() => { + result.current.setShowDeleteConfirm(true); + }); + expect(result.current.showDeleteConfirm).toBe(true); + + await act(async () => { + await result.current.handleDelete(); + }); + expect(deps.deleteDoc).toHaveBeenCalledTimes(1); + expect(result.current.showDeleteConfirm).toBe(false); + }); + + it("handleConfirmSave: save を呼び、モーダルを閉じる", async () => { + const deps = makeDeps(); + const { result } = renderHook(() => useCareerFormModals(deps)); + + act(() => { + result.current.setShowSaveConfirm(true); + }); + + await act(async () => { + await result.current.handleConfirmSave(); + }); + expect(deps.save).toHaveBeenCalledTimes(1); + expect(result.current.showSaveConfirm).toBe(false); + }); + + it("editingField: 自己PR / 職務要約の切り替えと閉じる", () => { + const { result } = renderHook(() => useCareerFormModals(makeDeps())); + + expect(result.current.editingField).toBeNull(); + + act(() => { + result.current.setEditingField("self_pr"); + }); + expect(result.current.editingField).toBe("self_pr"); + + act(() => { + result.current.setEditingField("career_summary"); + }); + expect(result.current.editingField).toBe("career_summary"); + + act(() => { + result.current.setEditingField(null); + }); + expect(result.current.editingField).toBeNull(); + }); +}); diff --git a/frontend/src/hooks/career/useCareerFormModals.ts b/frontend/src/hooks/career/useCareerFormModals.ts new file mode 100644 index 00000000..ab6c8db1 --- /dev/null +++ b/frontend/src/hooks/career/useCareerFormModals.ts @@ -0,0 +1,38 @@ +import { useState } from "react"; + +/** + * CareerResumeForm のモーダル開閉状態とその操作ハンドラをまとめるカスタムフック。 + * 削除確認・保存確認・マークダウンフィールド編集の 3 モーダルを 1 フックで集約する。 + */ +export function useCareerFormModals({ + save, + deleteDoc, +}: { + save: (...args: never[]) => Promise; + deleteDoc: (...args: never[]) => Promise; +}) { + const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); + const [showSaveConfirm, setShowSaveConfirm] = useState(false); + const [editingField, setEditingField] = useState<"career_summary" | "self_pr" | null>(null); + + const handleDelete = async () => { + await deleteDoc(); + setShowDeleteConfirm(false); + }; + + const handleConfirmSave = async () => { + await save(); + setShowSaveConfirm(false); + }; + + return { + showDeleteConfirm, + setShowDeleteConfirm, + showSaveConfirm, + setShowSaveConfirm, + editingField, + setEditingField, + handleDelete, + handleConfirmSave, + }; +}