refactor(frontend): メッセージ SSoT 統一と職務経歴 mutator の重複解消#317
Conversation
- 削除確認ダイアログ/保存ボタンの日本語リテラルを UI_MESSAGES に集約 （ConfirmDialog・CareerResumeForm・useDocumentForm の隠れた SSoT 違反を解消） - useCareerExperienceMutators に updateExperienceAt/updateClientAt を導入し 三階層 immutable 更新の重複を集約（戻り値契約は不変） - 取引先配下のハンドラ Props を ClientMutationHandlers 型に切り出し CareerExperienceEditor/ClientEditor で共有 - 未ログイン保存導線（onSubmit 匿名分岐）の component テストを追加 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 40 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR centralizes UI message constants, introduces a shared type for client mutation handlers, refactors the career experience mutator hook with immutable update helpers, consolidates related component props, and adds test coverage for the unauthenticated resume save flow. ChangesMessage Constants and Type Consolidation with Hook Refactoring
Sequence DiagramssequenceDiagram
participant Test
participant CareerResumeForm
participant LoginPromptContext
Test->>CareerResumeForm: render with empty name
Test->>CareerResumeForm: click save button
CareerResumeForm->>Test: show FULL_NAME_REQUIRED validation
Test->>LoginPromptContext: verify requestLogin not called
Test->>CareerResumeForm: enter full name
Test->>CareerResumeForm: click save button
CareerResumeForm->>LoginPromptContext: call requestLogin
Test->>LoginPromptContext: verify requestLogin called once
Test->>CareerResumeForm: verify validation message absent
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/forms/CareerResumeForm.test.tsx (1)
69-69: ⚡ Quick winAvoid hardcoded placeholder literal in the test selector.
getByPlaceholderText("例: 山田 太郎")reintroduces a direct message literal and makes the test fragile to copy-only changes. Prefer a non-literal selector (role/label/test-id) or a centralized constant.As per coding guidelines, “In frontend code, follow message management rules from
.claude/rules/frontend/messages.md- message literals are forbidden and must use Single Source of Truth (SSoT)”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/forms/CareerResumeForm.test.tsx` at line 69, The test hardcodes the placeholder string in the selector (fireEvent.change(screen.getByPlaceholderText("例: 山田 太郎"), ...)), which violates message SSoT rules; replace that literal with a stable selector—either import the placeholder constant from your messages/SSoT and use it, or better switch to a non-literal selector such as getByLabelText or getByRole (or a data-testid like getByTestId('career-resume-name')) that targets the input rendered by the CareerResumeForm component so the test no longer depends on the placeholder text.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/components/forms/CareerResumeForm.test.tsx`:
- Line 69: The test hardcodes the placeholder string in the selector
(fireEvent.change(screen.getByPlaceholderText("例: 山田 太郎"), ...)), which violates
message SSoT rules; replace that literal with a stable selector—either import
the placeholder constant from your messages/SSoT and use it, or better switch to
a non-literal selector such as getByLabelText or getByRole (or a data-testid
like getByTestId('career-resume-name')) that targets the input rendered by the
CareerResumeForm component so the test no longer depends on the placeholder
text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bb484513-bdd4-4ee5-b4f6-d4fed19f2dfd
📒 Files selected for processing (9)
frontend/src/components/ConfirmDialog.tsxfrontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsxfrontend/src/components/forms/CareerFormEditors/ClientEditor.tsxfrontend/src/components/forms/CareerResumeForm.test.tsxfrontend/src/components/forms/CareerResumeForm.tsxfrontend/src/constants/messages.tsfrontend/src/formTypes.tsfrontend/src/hooks/career/useCareerExperienceMutators.tsfrontend/src/hooks/useDocumentForm.ts
レビュー指摘対応。getByPlaceholderText のリテラル結合を避け、
getByRole("textbox", { name: /氏名/ }) でラベル経由に特定して
プレースホルダ文言変更に強くする。
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
concurrently 経由の推移依存 shell-quote@1.8.3 が GHSA-w7jw-789q-3m8p (critical) に該当し npm audit --audit-level=high が CI で fail していた。 package.json の overrides で patch 版 1.8.4 を強制し、concurrently の 破壊的ダウングレード(--force)を避けて解消する。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
概要
FE_refacterレビュー（report/FE_report_20260609_2245.md）に基づく frontend 保守性リファクタ。Medium 全件 + Low（Props 型合成）+ テスト追加を適用。破壊的変更なし（表示文言の値・公開 API・戻り値契約は不変）。変更内容
メッセージ SSoT 統一（Medium）
UI_MESSAGESに集約。ESLint をすり抜けていた隠れた SSoT 違反を解消。ConfirmDialog.tsx（"削除中..."/"キャンセル"）CareerResumeForm.tsx（削除確認message/confirmLabel）useDocumentForm.ts（保存ボタン文言"保存中..."/"更新する"/"保存する"）nested-update ヘルパ抽出（Medium）
useCareerExperienceMutatorsにupdateExperienceAt/updateClientAtを導入し、三階層 immutable 更新の同形ボイラープレート 6 箇所を集約。戻り値契約は不変。Props 型合成（Low）
formTypes.tsのClientMutationHandlers型へ切り出し、CareerExperienceEditor/ClientEditorで共有。テスト追加
CareerResumeForm.test.tsx: 未ログイン保存導線（onSubmit匿名分岐）を守る 2 ケース。requestLoginを 1 回呼ぶ検証
make lint-frontend: ✅make lint-frontend-messages: ✅make test-frontend: ✅（vitest 299 / 38 files、node:test 4）make build-frontend: ✅（tsc -b + vite build）make dupe-check: 対象重複の解消を確認（残る非テスト clone はアイコン SVG / setAtPath / Rule of Three 保留のhandleSelectionのみ）保留（Follow-up）
MarkdownDocumentView↔PdfDocumentViewのhandleSelection共通化は現状 2 箇所で Rule of Three 未達のため保留。3 箇所目が出た時点でuseTextSelectionFill化。🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Refactor