Skip to content

feat: 返品申請機能を追加#6838

Open
ttokoro20240902 wants to merge 24 commits into
4.4from
feature/refund-request
Open

feat: 返品申請機能を追加#6838
ttokoro20240902 wants to merge 24 commits into
4.4from
feature/refund-request

Conversation

@ttokoro20240902

@ttokoro20240902 ttokoro20240902 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

マイページから返品申請を行い、管理画面で承認・却下できる機能を追加します。

closes #6820

主な変更内容

データ層

  • RefundRequest / RefundRequestFile エンティティ、RefundRequestStatus マスタを追加
  • RefundRequestRepository による検索・ページネーション
  • マイグレーション(テーブル作成・初期データ投入)

ビジネスロジック

  • RefundRequestService — 申請作成・添付ファイル保存・メール送信
  • RefundRequestStateMachine — Symfony Workflow によるステータス遷移(申請中→承認/却下/取消)
  • RefundRequestEvent / EccubeEvents 定数追加

フロント(マイページ)

  • 注文履歴から返品申請フォームへ遷移
  • 商品選択・数量・理由入力・画像添付(最大3枚)
  • 確認画面 → 完了画面の流れ
01-mypage-history-with-refund-button 02-mypage-refund-input 03-mypage-refund-confirm 04-mypage-refund-complete 05-mypage-refund-history

管理画面

  • 返品申請一覧(検索・ページネーション・CSVエクスポート)
  • 返品申請詳細(ステータス変更・管理者メモ)
  • ナビゲーションに「返品申請管理」を追加
06-admin-refund-list 07-admin-refund-detail

フォーム

  • RefundRequestType(フロント用)/ RefundRequestEditType(管理用)/ SearchRefundRequestType(検索用)
  • RefundRequestStatusType(マスタ選択)

メール

  • 返品申請受付通知メールテンプレート

テスト

  • PHPUnit: Entity / Repository / Service / StateMachine / FormType / Controller(フロント・管理)
  • E2E(Playwright): フロント申請フロー・管理画面操作

Test plan

  • マイページ → 注文履歴 → 返品申請 → 確認 → 完了の一連フロー
  • 管理画面 → 返品申請一覧 → 詳細 → ステータス変更(承認/却下)
  • CSVエクスポート
  • 添付ファイルアップロード(画像3枚まで)
  • バリデーション(必須項目、数量上限、ファイルサイズ/形式)
  • メール送信確認
  • PHPUnit全テストパス
  • E2Eテストパス

🤖 Generated with Claude Code

Summary by CodeRabbit

リリースノート

  • New Features
    • マイページで配達済み注文の商品ごとに返品申請・申請履歴、証拠ファイル添付/ダウンロードに対応しました(入力バリデーション、確認/完了画面、履歴表示)。
    • 管理画面で返品申請の一覧/検索/編集、ステータス遷移、CSVエクスポート、通知メール送信を追加し、ナビゲーションにも反映しました。
  • Tests
    • フロント/管理画面の返品申請E2Eと、フォーム/サービス/コントローラ/リポジトリ/状態遷移の各テストを追加しました。

ttokoro20240902 and others added 9 commits June 15, 2026 16:22
- RefundRequest / RefundRequestFile / Master\RefundRequestStatus エンティティを追加
- Product に返品可否フラグ refund_allowed (default true) を追加
- 各 Repository(検索・注文明細別件数の一括取得=N+1回避)を追加
- 新規インストール用 CSV(mtb_refund_request_status, dtb_mail_template id=10, dtb_csv refund_allowed)
- 既存環境アップデート用 INSERT マイグレーション(冪等・down 実装)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- EccubeEvents に返品申請関連の定数を追加(フロント/管理/ステータス変更/メール)
- RefundRequestEvent を追加(ステータス変更時に変更前後のステータスを保持)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- refund_request ステートマシン設定(NEW→PROCESSING→ACCEPTED/DECLINED/INFO_REQUESTED)
- RefundRequestStateMachine(遷移可否判定・遷移実行・利用可能遷移の取得)
- RefundRequestService(申請作成+エビデンスファイル保存+ステータス遷移)
- MailService に管理者向け返品申請通知メール送信を追加
- エビデンスファイルは非公開の var/ 配下に保存(配信はコントローラ経由に限定)
- eccube.yaml に保存先・通知メールテンプレートIDの設定を追加

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Front/RefundRequestType: 数量・理由・エビデンスファイル(最大3件/15MB/MIME制限)
- Admin/SearchRefundRequestType: 複合検索・ステータス・作成日・更新日
- Admin/RefundRequestEditType: 管理者メモ・ステータス遷移(利用可能な遷移のみ表示)
- Master/RefundRequestStatusType: マスタセレクト

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Mypage/RefundRequestController: 申請入力/確認/完了/商品別履歴/ファイル配信(所有チェック付き)
- Admin/Order/RefundRequestController: 一覧検索/詳細編集/ステータス変更API/ファイル配信
- テンプレート7ファイル(フロント4 + 管理2 + メール通知1)
- history.twig: 発送済み注文に返品申請・履歴リンクを追加
- ナビ: 受注管理配下に「返品申請管理」メニュー追加
- 翻訳: messages/validators の ja/en に返品申請関連キーを追加

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FormViewの`_token`プロパティへのアクセスが不正だったため、
form要素のnovalidate属性でCSRFトークンを自動注入するように修正。
これにより管理画面返品申請一覧が正常に表示されるようになった。

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- Admin/RefundRequestController::export(): 検索条件に基づくCSVエクスポート
- 出力カラム: 申請ID/注文番号/会員名/商品名/ステータス/数量/理由/申請日時/更新日時
- 一覧テンプレートにCSVエクスポートボタン追加
- BOM付きUTF-8でExcel互換

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
StateMachine・Service・Repository・FormType・Controller(フロント/管理)の
各レイヤの単体・機能テスト計91件を追加。MailServiceのMailHistory保存で
存在しないsetMailTemplate()呼び出しを修正。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
フロント(申請フォーム表示・入力→確認→完了・履歴表示・バリデーション)と
管理画面(一覧・検索・詳細・メモ保存・ステータス遷移・CSV表示)のE2Eテストを追加。
テスト用フィクスチャ(発送済み注文を持つ会員)とCIマトリクスも追加。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

EC-CUBE に返品申請機能を全面的に新規実装した。マイページからの申請フォーム(入力→確認→完了)、申請履歴、管理画面での一覧・検索・編集・CSV エクスポート、Symfony Workflow ベースのステートマシン、エビデンスファイルの非公開ローカル保存、管理者向けメール通知、Doctrine マイグレーション、PHPUnit テスト、Playwright E2E テストを一括追加した。

Changes

返品申請機能(Refund Request Feature)

Layer / File(s) Summary
エンティティ・設定・マイグレーション
src/Eccube/Entity/RefundRequest.php, src/Eccube/Entity/RefundRequestFile.php, src/Eccube/Entity/Master/RefundRequestStatus.php, src/Eccube/Entity/Product.php, src/Eccube/Repository/Master/RefundRequestStatusRepository.php, src/Eccube/Repository/RefundRequestFileRepository.php, app/DoctrineMigrations/Version20260615000000.php, app/config/eccube/packages/refund_request_state_machine.php, app/config/eccube/packages/eccube.yaml, app/config/eccube/packages/eccube_nav.yaml
RefundRequest(注文明細単位の申請)・RefundRequestFile(エビデンス)・RefundRequestStatus(NEW/PROCESSING/ACCEPTED/DECLINED/INFO_REQUESTED)エンティティを新規追加。Product.refund_allowed フラグをデフォルト true で追加。マイグレーションで 5 種類のステータス、テンプレート ID=10、CSV ID=215 を投入。ステートマシン・ファイル保存パス・メール設定・ナビゲーション項目を追加。
ステートマシン・イベント・リポジトリ
src/Eccube/Service/RefundRequestStateMachine.php, src/Eccube/Event/EccubeEvents.php, src/Eccube/Event/RefundRequestEvent.php, src/Eccube/Repository/RefundRequestRepository.php
RefundRequestStateMachine(Symfony Workflow ラッパー)と RefundRequestStateMachineContext で 5 つの遷移(start_processing / accept / decline / request_info / resume_processing)を管理。EccubeEvents に 11 個の返品申請イベント定数を追加。RefundRequestRepository に複合検索・注文明細別取得・顧客別集計クエリを実装。
RefundRequestService・MailService
src/Eccube/Service/RefundRequestService.php, src/Eccube/Service/MailService.php
RefundRequestService で申請作成(ステータス NEW 設定・ファイル保存・永続化)・ステータス変更(ステートマシン経由)・遷移可否判定・ファイル保存(random_bytes による名前付け・MIME 記録)を実装。MailService に管理者向けメール送信メソッド追加。
フォームタイプ・ローカライズ
src/Eccube/Form/Type/Front/RefundRequestType.php, src/Eccube/Form/Type/Admin/RefundRequestEditType.php, src/Eccube/Form/Type/Admin/SearchRefundRequestType.php, src/Eccube/Form/Type/Master/RefundRequestStatusType.php, src/Eccube/Resource/locale/messages.*.yaml, src/Eccube/Resource/locale/validators.*.yaml
フロント申請フォーム(quantity: 0 超・上限検証、reason: 必須・4000 文字上限、files: 最大 3 件・各 15MB・許可 MIME タイプ)、管理編集フォーム(admin_note・利用可能遷移の選択肢)、管理検索フォーム(複合テキスト・ステータス複数選択・日時範囲)を追加。日英メッセージ・バリデーション文言を 53 行追加。
マイページ返品申請コントローラ
src/Eccube/Controller/Mypage/RefundRequestController.php
index(フォーム生成・セッション一時ファイル保存)、confirm(確認画面プレビュー・申請作成)、complete(完了画面)、itemHistory(履歴一覧)、downloadFile/downloadTempFile(ファイル配信・所有者チェック・realpath によるパストラバーサル防止)の各ルートを実装。getValidOrder(配送済み検証・所有者チェック)、getValidOrderItem(返品許可検証)ヘルパを提供。
管理画面返品申請コントローラ
src/Eccube/Controller/Admin/Order/RefundRequestController.php
index(ページネーション・セッション検索保存・復元)、edit(管理メモ・遷移適用・通知・リダイレクト)、updateStatus(PUT JSON API・遷移バリデーション)、export(SearchQueryBuilder・UTF-8 BOM 付き CSV ストリーミング・CSVインジェクション対策)、downloadFile(.. 検査・realpath 検証)の各エンドポイントを実装。
Twig テンプレート群
src/Eccube/Resource/template/default/Mypage/refund_request*.twig, src/Eccube/Resource/template/default/Mypage/history.twig, src/Eccube/Resource/template/admin/Order/refund_request*.twig, src/Eccube/Resource/template/default/Mail/refund_request_notify.twig
マイページ申請(フォーム・数量/理由/ファイル入力)、確認(注文明細・数量/理由再確認・一時ファイルプレビュー)、完了、履歴(ステータス・理由・添付ファイル)テンプレート。管理一覧(検索・ページング・CSV エクスポート)、編集(詳細・ファイル表示・管理メモ・ステータス遷移)テンプレート。メール通知テンプレート。購入履歴テンプレートに DELIVERED + 返品許可時のリンクを追加。
ユニット・統合テスト
tests/Eccube/Tests/Form/Type/..., tests/Eccube/Tests/Repository/RefundRequestRepositoryTest.php, tests/Eccube/Tests/Service/RefundRequestService*.php, tests/Eccube/Tests/Service/RefundRequestStateMachineTest.php
フォームバリデーション(quantity・reason・files 複合)、リポジトリ検索(空・ID・注文番号・ステータス・日時範囲)、ステートマシン遷移可否(全 5 遷移×5 ステータス組み合わせ)、サービス申請作成・ステータス変更・メール送信・ファイル添付を PHPUnit で検証。
Web テスト・E2E テスト・CI
tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php, tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php, e2e/tests/admin-refund-request.spec.ts, e2e/tests/front-refund-request.spec.ts, e2e/setup-fixtures.php, .github/workflows/e2e-test.yml
管理/マイページコントローラの HTTP 統合テスト(一覧・編集・ステータス更新・エクスポート・ファイル配信・アクセス制御・バリデーション・パストラバーサル防止)。Playwright E2E テスト(フロント:ログイン→申請→確認→完了→履歴、管理:一覧→編集→メモ保存→ステータス変更→CSV エクスポート、バリデーション)。E2E スイート登録。

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • EC-CUBE/ec-cube#6829: .github/workflows/e2e-test.ymlmatrix.suite に Playwright スイートを追加する変更が本 PR と同じファイル・同じ箇所に対して行われている。

Suggested reviewers

  • nanasess
  • dotani1111

Poem

🐇 うさぎがぴょんと申請を出す
返品のフォーム、するするっと流れ
ステートマシンが遷移をさばき
管理画面はCSVを吐き出す
ファイルはひっそり var の奥に宿る
テストも揃って、安心ぴょん! 🎉

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/refund-request

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (3)
tests/Eccube/Tests/Repository/RefundRequestRepositoryTest.php (1)

58-69: ⚡ Quick win

検索テストが偽陽性になり得るため、期待ID/件数まで固定してください。

assertNotEmpty() だけだと、無関係データが返っても成功します。multi/status/create_date それぞれで「期待件数」と「期待ID(または全件が条件一致)」まで検証すると、クエリ退行を確実に検出できます。

Also applies to: 71-101

🤖 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 `@tests/Eccube/Tests/Repository/RefundRequestRepositoryTest.php` around lines
58 - 69, The test method testGetQueryBuilderBySearchDataMultiByOrderNo uses only
assertNotEmpty() to verify search results, which can pass even when unrelated
data is returned, creating a false positive. Replace the assertNotEmpty()
assertion with more specific validations that check both the expected count of
returned results and the expected IDs of those results (or verify all results
match the search condition). This ensures the query correctly filters by the
multi parameter and will catch regressions. Apply the same fix pattern to all
similar search test methods in the test class that test multi, status, and
create_date search criteria.
tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php (1)

43-75: ⚡ Quick win

検索系テストがHTTP 200のみで、検索条件の有効性を検証できていません。

testIndexWithSearch / testIndexWithStatusSearch は成功ステータスだけを見ているため、絞り込みが壊れても通過します。未使用の $crawler を使って、結果内容まで検証してください。

🔧 例: 検索結果の中身まで検証
-        $this->assertTrue($this->client->getResponse()->isSuccessful());
+        $this->assertTrue($this->client->getResponse()->isSuccessful());
+        $content = (string) $this->client->getResponse()->getContent();
+        $this->assertStringContainsString((string) $RefundRequest->getId(), $content);
🤖 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 `@tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php` around
lines 43 - 75, Both testIndexWithSearch and testIndexWithStatusSearch methods
only verify HTTP 200 response status but do not validate that the search filters
actually work. Instead of leaving $crawler unused, use it to verify the contents
of the search results. For testIndexWithSearch, use the crawler to assert that
the result contains the RefundRequest with the matching ID. For
testIndexWithStatusSearch, use the crawler to assert that only RefundRequests
with the NEW status are present in the results. This ensures the search
functionality is properly validated beyond just a successful response code.

Source: Linters/SAST tools

src/Eccube/Controller/Mypage/RefundRequestController.php (1)

218-220: ⚡ Quick win

realpath の配下判定はディレクトリ境界付きで比較してください。

str_starts_with($realPath, $realTopDir) だけだと、prefix が同じ別ディレクトリを誤許可する余地があります。末尾セパレータを含めた厳密比較にしてください(同等ロジックの管理画面側も同時修正推奨)。

修正例
-        if ($realPath === false || $realTopDir === false || !str_starts_with($realPath, $realTopDir)) {
+        $baseDir = rtrim($realTopDir ?: '', DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR;
+        if ($realPath === false || $realTopDir === false || !str_starts_with($realPath, $baseDir)) {
             throw new NotFoundHttpException();
         }
🤖 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 `@src/Eccube/Controller/Mypage/RefundRequestController.php` around lines 218 -
220, The realpath subdirectory validation using `str_starts_with($realPath,
$realTopDir)` lacks proper directory boundary checking and could incorrectly
allow paths from sibling or similarly-named directories. Modify the comparison
to include the directory separator (typically a forward slash) to ensure strict
boundary matching, for example by appending a separator to $realTopDir before
the comparison or by checking that the character immediately following the
prefix in $realPath is a directory separator. This same pattern should also be
applied to equivalent path validation logic in the admin panel side to ensure
consistency across the application.
🤖 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.

Inline comments:
In `@app/DoctrineMigrations/Version20260615000000.php`:
- Around line 90-93: The down() method's DELETE statements are using only id as
a filter, which is too broad and could accidentally delete existing production
records during rollback if those IDs are already in use. Modify each of the
three DELETE statements (dtb_csv, dtb_mail_template, and
mtb_refund_request_status) to include additional WHERE conditions using
deployment-time identifiable keys such as file_name, entity_name, field_name, or
discriminator_type to narrow the deletion criteria and ensure only the newly
added migration data is removed.

In `@e2e/setup-fixtures.php`:
- Around line 217-237: The issue is that when the refund test customer already
exists, the order creation logic (from the $Delivery retrieval through the
shipping date foreach loop) is only executed in the if block and gets skipped in
the else branch. This means on subsequent test runs where the customer exists,
no shipped order is guaranteed to exist, causing test instability. Move the
order creation and delivery status setting logic outside of the if/else
conditional block so that it executes regardless of whether the customer is
newly created or already existed. Ensure that after the customer existence
check, the code always verifies and creates a delivered order for the refund
test customer if one doesn't already exist.

In `@e2e/tests/admin-refund-request.spec.ts`:
- Around line 37-50: The test uses conditional `if` statements that skip
verification when prerequisite data is not found, allowing the test to pass even
when required data is missing. Replace the conditional checks (the `if
(hasResults)` pattern and similar ones at the other affected sites) with
mandatory `expect` assertions that require the elements to exist before
proceeding. This ensures that if the data is missing, the test fails immediately
rather than silently skipping the validation. Update all affected locations:
lines 37-50 (the editBtn.isVisible check), lines 63-80, 104-120, 144-158,
171-173, and 186-195 in the same file to use expect assertions instead of
conditional branching for prerequisite data validation.

In `@src/Eccube/Controller/Admin/Order/RefundRequestController.php`:
- Around line 275-286: The CSV export in the RefundRequestController is
vulnerable to formula injection attacks. When customer names, product names, and
reasons are exported, if they start with formula characters (=, +, -, @), they
will be evaluated as formulas when opened in spreadsheet applications. Sanitize
the potentially dangerous fields (the customer name concatenation, product name
from getProductName(), and the reason from getReason()) by prepending a single
quote to any value that starts with these characters before they are added to
the row array that is passed to fputcsv(). This prevents the spreadsheet
application from interpreting them as formulas.

In `@src/Eccube/Controller/Mypage/RefundRequestController.php`:
- Around line 104-105: The UploadedFile objects obtained from
`$form->get('files')->getData()` in the refund request flow are not persisted
across the confirmation screen, causing the file array to be empty when
processing completes. Instead of directly passing the UploadedFile objects to
createRefundRequest, implement temporary file storage before the confirmation
screen by saving the uploaded files to a temporary location and generating a
token or identifier to track them. Pass this token/identifier through the
confirmation screen (via session, hidden form field, or similar mechanism), then
retrieve and use the temporarily stored files from this identifier in the
complete action instead of relying on form data that will have been lost.

In `@src/Eccube/Entity/RefundRequest.php`:
- Around line 222-224: The `removeRefundRequestFile()` method in the
RefundRequest entity only removes the RefundRequestFile from the collection but
does not clear the bidirectional relationship. To properly maintain
bidirectional relationship synchronization, after removing the element from the
RefundRequestFiles collection, you must also set the refundRequest property of
the removed RefundRequestFile object to null. This ensures Doctrine correctly
reflects the relationship removal and prevents data inconsistency.

In `@src/Eccube/Form/Type/Admin/RefundRequestEditType.php`:
- Around line 65-69: The `refund_request` option is defined as optional (with
null as the default value in setDefaults and null allowed in setAllowedTypes) in
the configureOptions method at lines 65-69, but the code at line 39 immediately
uses it as if it's always a RefundRequest object, which causes a runtime error
when the option is null or missing. Make the `refund_request` option mandatory
by removing the null default from the setDefaults call (remove the
'refund_request' => null line or set required to true) and remove 'null' from
the allowed types in setAllowedTypes so it only accepts RefundRequest::class,
ensuring the form contract matches the actual usage requirements.

In
`@src/Eccube/Resource/template/default/Mypage/refund_request_item_history.twig`:
- Around line 77-80: The image link in the refund request file display lacks an
accessible name because the img element has an empty alt attribute, making it a
nameless link for assistive technologies. Add meaningful alt text to the img
element inside the link that describes the attached file (such as the filename
or a descriptive label like "View attached file"), or add an aria-label
attribute to the a element to provide context. This will ensure assistive
technologies can properly identify and announce the purpose of the link to
users.

In `@src/Eccube/Service/MailService.php`:
- Around line 916-918: In the catch block for TransportExceptionInterface in the
MailService class around line 916-918, the current error log only includes the
RefundRequest ID but omits the actual exception message. Add the exception
message from the caught TransportExceptionInterface variable $e to the log
context by including $e->getMessage() in the array passed to log_error, so that
debugging information about the actual mail sending failure is preserved in the
logs.

In `@src/Eccube/Service/RefundRequestService.php`:
- Around line 60-63: The instanceof UploadedFile check in the foreach loop is
causing PHPStan failures because the parameter type hint already declares
$uploadedFiles as UploadedFile[], making every element guaranteed to be an
UploadedFile instance. Remove the redundant instanceof UploadedFile condition in
the foreach loop that iterates over $uploadedFiles and directly execute the
addRefundRequestFile call without the conditional wrapper. This will eliminate
the static analysis error by removing the unnecessary type check.

In `@tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php`:
- Around line 37-125: The RefundRequestTypeTest class is missing test cases for
file attachment validation requirements. Add new test methods following the
existing pattern (testValidData, testInvalidQuantity*, etc.) to cover file
attachment scenarios: testValidFileSubmission (within limits),
testInvalidFilesExceedMaxCount (more than 3 files), testInvalidFileSizeExceeded
(file exceeding size limit), and testInvalidMimeType (invalid MIME types). Each
test should submit form data with file attachments and assert the validation
result using isValid(), ensuring the form properly enforces the specification
constraints for maximum 3 files, file size limits, and MIME type validation.

In `@tests/Eccube/Tests/Service/RefundRequestServiceTest.php`:
- Around line 63-83: The current test testCreateRefundRequestWithoutFiles only
covers the scenario where a RefundRequest is created with no files (passing an
empty array). Add a new test method that verifies the "with files" scenario by
creating a RefundRequest with at least one attached file and then asserting that
the RefundRequestFile objects are correctly created, properly associated to the
RefundRequest, and contain all necessary metadata. This ensures the
createRefundRequest method properly handles and persists file attachments.

In `@tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php`:
- Around line 245-291: The validation tests testValidationQuantityZero() and
testValidationReasonEmpty() only assert that the response is successful but do
not verify that validation actually failed. Add explicit assertions to each test
to confirm that validation errors are displayed or that the confirmation screen
is not reached, such as checking that the crawler still contains the input form
elements or does not contain confirmation screen content, ensuring that invalid
inputs (quantity of 0 and empty reason respectively) are properly rejected.

---

Nitpick comments:
In `@src/Eccube/Controller/Mypage/RefundRequestController.php`:
- Around line 218-220: The realpath subdirectory validation using
`str_starts_with($realPath, $realTopDir)` lacks proper directory boundary
checking and could incorrectly allow paths from sibling or similarly-named
directories. Modify the comparison to include the directory separator (typically
a forward slash) to ensure strict boundary matching, for example by appending a
separator to $realTopDir before the comparison or by checking that the character
immediately following the prefix in $realPath is a directory separator. This
same pattern should also be applied to equivalent path validation logic in the
admin panel side to ensure consistency across the application.

In `@tests/Eccube/Tests/Repository/RefundRequestRepositoryTest.php`:
- Around line 58-69: The test method
testGetQueryBuilderBySearchDataMultiByOrderNo uses only assertNotEmpty() to
verify search results, which can pass even when unrelated data is returned,
creating a false positive. Replace the assertNotEmpty() assertion with more
specific validations that check both the expected count of returned results and
the expected IDs of those results (or verify all results match the search
condition). This ensures the query correctly filters by the multi parameter and
will catch regressions. Apply the same fix pattern to all similar search test
methods in the test class that test multi, status, and create_date search
criteria.

In `@tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php`:
- Around line 43-75: Both testIndexWithSearch and testIndexWithStatusSearch
methods only verify HTTP 200 response status but do not validate that the search
filters actually work. Instead of leaving $crawler unused, use it to verify the
contents of the search results. For testIndexWithSearch, use the crawler to
assert that the result contains the RefundRequest with the matching ID. For
testIndexWithStatusSearch, use the crawler to assert that only RefundRequests
with the NEW status are present in the results. This ensures the search
functionality is properly validated beyond just a successful response code.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bdd39dc0-0128-4e9e-8d83-0f9f4ae8b496

📥 Commits

Reviewing files that changed from the base of the PR and between 2dc6a22 and a2e8df9.

⛔ Files ignored due to path filters (6)
  • src/Eccube/Resource/doctrine/import_csv/en/dtb_csv.csv is excluded by !**/*.csv
  • src/Eccube/Resource/doctrine/import_csv/en/dtb_mail_template.csv is excluded by !**/*.csv
  • src/Eccube/Resource/doctrine/import_csv/en/mtb_refund_request_status.csv is excluded by !**/*.csv
  • src/Eccube/Resource/doctrine/import_csv/ja/dtb_csv.csv is excluded by !**/*.csv
  • src/Eccube/Resource/doctrine/import_csv/ja/dtb_mail_template.csv is excluded by !**/*.csv
  • src/Eccube/Resource/doctrine/import_csv/ja/mtb_refund_request_status.csv is excluded by !**/*.csv
📒 Files selected for processing (45)
  • .github/workflows/e2e-test.yml
  • app/DoctrineMigrations/Version20260615000000.php
  • app/config/eccube/packages/eccube.yaml
  • app/config/eccube/packages/eccube_nav.yaml
  • app/config/eccube/packages/refund_request_state_machine.php
  • e2e/setup-fixtures.php
  • e2e/tests/admin-refund-request.spec.ts
  • e2e/tests/front-refund-request.spec.ts
  • src/Eccube/Controller/Admin/Order/RefundRequestController.php
  • src/Eccube/Controller/Mypage/RefundRequestController.php
  • src/Eccube/Entity/Master/RefundRequestStatus.php
  • src/Eccube/Entity/Product.php
  • src/Eccube/Entity/RefundRequest.php
  • src/Eccube/Entity/RefundRequestFile.php
  • src/Eccube/Event/EccubeEvents.php
  • src/Eccube/Event/RefundRequestEvent.php
  • src/Eccube/Form/Type/Admin/RefundRequestEditType.php
  • src/Eccube/Form/Type/Admin/SearchRefundRequestType.php
  • src/Eccube/Form/Type/Front/RefundRequestType.php
  • src/Eccube/Form/Type/Master/RefundRequestStatusType.php
  • src/Eccube/Repository/Master/RefundRequestStatusRepository.php
  • src/Eccube/Repository/RefundRequestFileRepository.php
  • src/Eccube/Repository/RefundRequestRepository.php
  • src/Eccube/Resource/locale/messages.en.yaml
  • src/Eccube/Resource/locale/messages.ja.yaml
  • src/Eccube/Resource/locale/validators.en.yaml
  • src/Eccube/Resource/locale/validators.ja.yaml
  • src/Eccube/Resource/template/admin/Order/refund_request.twig
  • src/Eccube/Resource/template/admin/Order/refund_request_edit.twig
  • src/Eccube/Resource/template/default/Mail/refund_request_notify.twig
  • src/Eccube/Resource/template/default/Mypage/history.twig
  • src/Eccube/Resource/template/default/Mypage/refund_request.twig
  • src/Eccube/Resource/template/default/Mypage/refund_request_complete.twig
  • src/Eccube/Resource/template/default/Mypage/refund_request_confirm.twig
  • src/Eccube/Resource/template/default/Mypage/refund_request_item_history.twig
  • src/Eccube/Service/MailService.php
  • src/Eccube/Service/RefundRequestService.php
  • src/Eccube/Service/RefundRequestStateMachine.php
  • tests/Eccube/Tests/Form/Type/Admin/SearchRefundRequestTypeTest.php
  • tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php
  • tests/Eccube/Tests/Repository/RefundRequestRepositoryTest.php
  • tests/Eccube/Tests/Service/RefundRequestServiceTest.php
  • tests/Eccube/Tests/Service/RefundRequestStateMachineTest.php
  • tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php
  • tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php

Comment thread app/DoctrineMigrations/Version20260615000000.php Outdated
Comment thread e2e/setup-fixtures.php Outdated
Comment thread e2e/tests/admin-refund-request.spec.ts Outdated
Comment thread src/Eccube/Controller/Admin/Order/RefundRequestController.php
Comment thread src/Eccube/Controller/Mypage/RefundRequestController.php Outdated
Comment thread src/Eccube/Service/MailService.php
Comment thread src/Eccube/Service/RefundRequestService.php Outdated
Comment thread tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php
Comment thread tests/Eccube/Tests/Service/RefundRequestServiceTest.php
Comment thread tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php (2)

270-292: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

バリデーション失敗を明示的に検証してください。

testValidationReasonEmpty() も同様に、理由が空の場合にバリデーションが実際に失敗することを明示的に検証してください。

🤖 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 `@tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php` around lines
270 - 292, The testValidationReasonEmpty() method needs to explicitly verify
that validation actually fails when the reason field is empty. Instead of only
checking that the response is successful, add assertions to verify that
validation errors are present in the response or form. Check for the presence of
validation error messages, form error constraints, or other validation failure
indicators when submitting the refund request with an empty reason value to
confirm the validation is working as expected.

246-268: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

バリデーション失敗を明示的に検証してください。

testValidationQuantityZero()isSuccessful() のみを確認していますが、バリデーションが実際に失敗したこと(入力画面に留まる、またはエラーメッセージが表示される)を検証していません。無効な入力で誤って確認画面に進んでも通過してしまいます。

🔧 検証の追加例
 $this->assertTrue($this->client->getResponse()->isSuccessful());
+$content = (string) $this->client->getResponse()->getContent();
+// 確認画面へ進まないことを確認
+$this->assertStringNotContainsString('返品申請内容確認', $content);
+// または入力フォームが残っていることを確認
+$this->assertGreaterThan(0, $crawler->filter('`#refund_request_quantity`')->count());
🤖 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 `@tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php` around lines
246 - 268, The testValidationQuantityZero() method only verifies that the
response is successful but does not explicitly check that validation actually
failed for the quantity zero input. To fix this, add assertions that verify the
validation failure is clearly evident, such as checking for validation error
messages in the response content, verifying that the page remains on the input
form rather than progressing to the confirmation screen, or confirming that
specific error text related to the invalid quantity is present in the rendered
page. This ensures the test actually validates that the zero quantity validation
rule is enforced.
🧹 Nitpick comments (6)
tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php (4)

152-175: ⚡ Quick win

フォーム再生成の一貫性を保ってください。

testInvalidFilesExceedMaxCount() でも同様にフォームを再構築しています。既存テストとの一貫性のため $this->form の再利用を検討してください。

🤖 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 `@tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php` around lines
152 - 175, The testInvalidFilesExceedMaxCount method creates a form inline using
$this->formFactory->createBuilder instead of reusing $this->form like other
tests in the class. For consistency with existing tests, refactor the method to
reuse the $this->form property instead of inline form creation. If necessary,
configure the form with the required options (csrf_protection and max_quantity)
before submission to maintain consistency with the test class pattern.

128-150: ⚡ Quick win

フォーム再生成の一貫性を保ってください。

testValidWithFiles() では新たにフォームを構築していますが、既存テスト(38–116行目)では setUp() で用意した $this->form を再利用しています。一貫性のため、既存パターンに合わせて $this->form->submit(...) を使用することを推奨します。

ただし、ファイルアップロードテストでフォーム状態の分離が必要な技術的理由がある場合は、コメントで明示してください。

🤖 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 `@tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php` around lines
128 - 150, The testValidWithFiles() method creates a new form instance rather
than reusing the $this->form that is initialized in setUp() like other tests do
(lines 38-116). For consistency with the existing test pattern, either refactor
testValidWithFiles() to use $this->form->submit() instead of creating a new form
builder, or if form state isolation is technically necessary for file upload
testing, add a clear comment to the method explaining why the new form instance
approach is required here.

128-198: ⚡ Quick win

一時ファイルのクリーンアップを追加してください。

3つの新規テストメソッドで tempnam()file_put_contents() で作成した一時ファイルが削除されていません。テスト終了後に unlink() でクリーンアップすることを推奨します。

🔧 クリーンアップ例
 public function testValidWithFiles(): void
 {
     $tmpFile = tempnam(sys_get_temp_dir(), 'refund_type_').'.gif';
     file_put_contents($tmpFile, "GIF89a...");
+    try {
         $form = $this->formFactory->createBuilder(...)->getForm();
         $form->submit([...]);
         $this->assertTrue($form->isValid());
+    } finally {
+        if (file_exists($tmpFile)) {
+            unlink($tmpFile);
+        }
+    }
 }
🤖 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 `@tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php` around lines
128 - 198, Add cleanup code to remove temporary files created during test
execution in the three test methods: testValidWithFiles(),
testInvalidFilesExceedMaxCount(), and testInvalidFileDisallowedMimeType(). In
testValidWithFiles() and testInvalidFileDisallowedMimeType(), add
unlink($tmpFile) at the end of each method to delete the temporary file created
with tempnam(). In testInvalidFilesExceedMaxCount(), add cleanup logic at the
end to iterate through the temporary files created in the loop and delete each
one using unlink() to ensure all temporary files are properly removed after each
test completes.

177-198: ⚡ Quick win

フォーム再生成の一貫性を保ってください。

testInvalidFileDisallowedMimeType() でも同様です。既存テストパターンに合わせることで、コードの可読性と保守性が向上します。

🤖 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 `@tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php` around lines
177 - 198, The testInvalidFileDisallowedMimeType() method should follow the same
form creation and submission pattern used by other similar test methods in this
file. Review the existing test methods (likely other testInvalid* methods) to
identify the established pattern for form setup and submission, then refactor
testInvalidFileDisallowedMimeType() to match that same structure and approach
for consistency and maintainability.
tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php (1)

347-368: ⚡ Quick win

テストヘルパーメソッドが重複しています。

createTestFile() メソッドが Admin と Mypage の両コントローラテストで同一実装として重複しています。共通の trait または基底クラスに抽出することで、保守性が向上します。

  • tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php#L347-L368: このメソッドを共通 trait(例: RefundRequestTestHelperTrait)に抽出する。
  • tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php#L402-L423: 同じく共通 trait を use して重複を削除する。
🤖 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 `@tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php` around
lines 347 - 368, The `createTestFile()` method is duplicated across two test
classes: RefundRequestControllerTest in the Admin/Order directory (lines
347-368) and RefundRequestControllerTest in the Mypage directory (lines
402-423). Create a new trait called RefundRequestTestHelperTrait that contains
the shared `createTestFile()` method implementation, then add a use statement
for this trait in both test classes at
tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php and
tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php, and remove the
duplicate method implementations from both classes.
tests/Eccube/Tests/Service/RefundRequestServiceTest.php (1)

180-244: ⚡ Quick win

一時ファイルのクリーンアップを追加してください。

testCreateRefundRequestWithFiles() および testCreateRefundRequestWithMultipleFiles() で作成した一時ファイルが削除されていません。テスト終了後に明示的にクリーンアップすることを推奨します。

🔧 クリーンアップ例
 public function testCreateRefundRequestWithFiles(): void
 {
     // ... setup code ...
     $tmpFile = tempnam(sys_get_temp_dir(), 'refund_test_');
     file_put_contents($tmpFile, str_repeat("\x00", 100));
     $uploadedFile = new UploadedFile($tmpFile, 'test.jpg', 'image/jpeg', null, true);
+    try {
         $result = $this->refundRequestService->createRefundRequest($RefundRequest, [$uploadedFile]);
         // ... assertions ...
+    } finally {
+        if (file_exists($tmpFile)) {
+            unlink($tmpFile);
+        }
+    }
 }
🤖 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 `@tests/Eccube/Tests/Service/RefundRequestServiceTest.php` around lines 180 -
244, The temporary files created using tempnam() in the
testCreateRefundRequestWithFiles() method are not being deleted after the test
completes. Add a cleanup section after the assertions to explicitly delete the
$tmpFile using unlink(). Similarly, in the
testCreateRefundRequestWithMultipleFiles() method, add cleanup code after the
assertions that iterates through the $files array and deletes each temporary
file. This ensures no temporary files are left behind in the system after tests
complete.
🤖 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.

Duplicate comments:
In `@tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php`:
- Around line 270-292: The testValidationReasonEmpty() method needs to
explicitly verify that validation actually fails when the reason field is empty.
Instead of only checking that the response is successful, add assertions to
verify that validation errors are present in the response or form. Check for the
presence of validation error messages, form error constraints, or other
validation failure indicators when submitting the refund request with an empty
reason value to confirm the validation is working as expected.
- Around line 246-268: The testValidationQuantityZero() method only verifies
that the response is successful but does not explicitly check that validation
actually failed for the quantity zero input. To fix this, add assertions that
verify the validation failure is clearly evident, such as checking for
validation error messages in the response content, verifying that the page
remains on the input form rather than progressing to the confirmation screen, or
confirming that specific error text related to the invalid quantity is present
in the rendered page. This ensures the test actually validates that the zero
quantity validation rule is enforced.

---

Nitpick comments:
In `@tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php`:
- Around line 152-175: The testInvalidFilesExceedMaxCount method creates a form
inline using $this->formFactory->createBuilder instead of reusing $this->form
like other tests in the class. For consistency with existing tests, refactor the
method to reuse the $this->form property instead of inline form creation. If
necessary, configure the form with the required options (csrf_protection and
max_quantity) before submission to maintain consistency with the test class
pattern.
- Around line 128-150: The testValidWithFiles() method creates a new form
instance rather than reusing the $this->form that is initialized in setUp() like
other tests do (lines 38-116). For consistency with the existing test pattern,
either refactor testValidWithFiles() to use $this->form->submit() instead of
creating a new form builder, or if form state isolation is technically necessary
for file upload testing, add a clear comment to the method explaining why the
new form instance approach is required here.
- Around line 128-198: Add cleanup code to remove temporary files created during
test execution in the three test methods: testValidWithFiles(),
testInvalidFilesExceedMaxCount(), and testInvalidFileDisallowedMimeType(). In
testValidWithFiles() and testInvalidFileDisallowedMimeType(), add
unlink($tmpFile) at the end of each method to delete the temporary file created
with tempnam(). In testInvalidFilesExceedMaxCount(), add cleanup logic at the
end to iterate through the temporary files created in the loop and delete each
one using unlink() to ensure all temporary files are properly removed after each
test completes.
- Around line 177-198: The testInvalidFileDisallowedMimeType() method should
follow the same form creation and submission pattern used by other similar test
methods in this file. Review the existing test methods (likely other
testInvalid* methods) to identify the established pattern for form setup and
submission, then refactor testInvalidFileDisallowedMimeType() to match that same
structure and approach for consistency and maintainability.

In `@tests/Eccube/Tests/Service/RefundRequestServiceTest.php`:
- Around line 180-244: The temporary files created using tempnam() in the
testCreateRefundRequestWithFiles() method are not being deleted after the test
completes. Add a cleanup section after the assertions to explicitly delete the
$tmpFile using unlink(). Similarly, in the
testCreateRefundRequestWithMultipleFiles() method, add cleanup code after the
assertions that iterates through the $files array and deletes each temporary
file. This ensures no temporary files are left behind in the system after tests
complete.

In `@tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php`:
- Around line 347-368: The `createTestFile()` method is duplicated across two
test classes: RefundRequestControllerTest in the Admin/Order directory (lines
347-368) and RefundRequestControllerTest in the Mypage directory (lines
402-423). Create a new trait called RefundRequestTestHelperTrait that contains
the shared `createTestFile()` method implementation, then add a use statement
for this trait in both test classes at
tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php and
tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php, and remove the
duplicate method implementations from both classes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1a0ecaaf-4fdc-46a9-bb16-51824c083d89

📥 Commits

Reviewing files that changed from the base of the PR and between a2e8df9 and 8d9bc8e.

📒 Files selected for processing (4)
  • tests/Eccube/Tests/Form/Type/Front/RefundRequestTypeTest.php
  • tests/Eccube/Tests/Service/RefundRequestServiceTest.php
  • tests/Eccube/Tests/Web/Admin/Order/RefundRequestControllerTest.php
  • tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php

ttokoro20240902 and others added 3 commits June 16, 2026 09:32
- PHPStan: 冗長なinstanceofチェックを削除(RefundRequestService)
- Rector: 未使用catch変数の削除、#[\Override]属性の追加、
  (string)キャスト追加、assertInstanceOf追加、
  名前付き引数のDataProvider、EccubeConfig::class化

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Rector: 2つ目のcatch未使用変数を修正
- CSV数式インジェクション対策(=+\-@始まりをサニタイズ)
- マイグレーション down() の DELETE条件を安全化
- Entity双方向関連の削除同期を追加
- RefundRequestEditType の refund_request オプション必須化
- テンプレート画像リンクの alt/aria-label 追加
- MailService ログに例外メッセージを含める

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
確認画面のテンプレが `form_rest(form)` で全フィールドを再描画していたため、
<input type="file"> は HTML 仕様上 value を再送できず、複数ステップを跨いだ申請完了時に
エビデンスファイルが 1 件も保存されない問題を修正。
合わせて確認画面に編集可能な input/textarea/file が並んでしまう UX 不具合も解消する。

Enterprise 版 (FilePond + S3) の「一時アップロード→セッション参照→確定時に永続領域へ移動」
パターンをローカル FS で再現:

- Mypage/RefundRequestController: index (入力 POST) と confirm (確認 POST) を分離。
  入力 POST でファイルを一時保存 → セッションに参照キー格納 → /confirm へリダイレクト。
  confirm POST で CSRF 検証後、永続領域へ rename して RefundRequest を確定。
- RefundRequestService: saveTempFile/getTempFilePath/removeTempFile を追加。
  createRefundRequest を (RefundRequest, list $tempFiles, string $sessionId) に変更。
- eccube.yaml: eccube_temp_refund_request_file_dir を追加。
- refund_request.twig / refund_request_confirm.twig: form_rest を撤去し編集欄を完全排除。
  EC-CUBE 標準フォーム (Entry/index.twig 等) に倣い ec-mypageRole + ec-input/ec-halfInput で
  レイアウトを統一し、textarea/file input/数量入力が全幅で表示されるよう修正。
- downloadFile: Content-Type を DB の mime_type で明示、X-Content-Type-Options: nosniff を付与。

テスト:
- PHPUnit: ファイル添付付き 2 段階フローで RefundRequestFile が永続化されることを assert
  (testConfirmPostWithFilePersistsFile, testCreateRefundRequestWithTempFiles)。
- E2E: 確認画面に編集欄が残らないこと/履歴に file リンクが出ること/セッション喪失時の挙動を追加。
- TraceableEventDispatcher 依存を EventDispatcherInterface に修正 (pre-existing バグ)。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php (1)

371-415: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

無効入力テストが成功条件を十分に検証できていません。

Line 371 と Line 394 のテストは isSuccessful() のみを見ており、無効値でも確認画面へ進んだ回帰を検知できません。入力画面に留まること(例: 確認見出しが出ない / 入力フォーム要素が残る)を明示的に検証してください。

修正例
@@
     public function testValidationQuantityZero(): void
@@
-        $this->assertTrue($this->client->getResponse()->isSuccessful());
+        $this->assertTrue($this->client->getResponse()->isSuccessful());
+        $content = (string) $this->client->getResponse()->getContent();
+        $this->assertStringNotContainsString('返品申請内容確認', $content);
+        $this->assertGreaterThan(0, $this->client->getCrawler()->filter('`#refund_request_quantity`')->count());
@@
     public function testValidationReasonEmpty(): void
@@
-        $this->assertTrue($this->client->getResponse()->isSuccessful());
+        $this->assertTrue($this->client->getResponse()->isSuccessful());
+        $content = (string) $this->client->getResponse()->getContent();
+        $this->assertStringNotContainsString('返品申請内容確認', $content);
+        $this->assertGreaterThan(0, $this->client->getCrawler()->filter('`#refund_request_reason`')->count());
🤖 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 `@tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php` around lines
371 - 415, The testValidationQuantityZero() and testValidationReasonEmpty() test
methods only verify that the response is successful but do not explicitly
confirm that invalid input causes the form to remain on the input screen rather
than proceeding to the confirmation page. Add explicit assertions in both
methods to verify that the input screen is retained when validation fails, such
as checking that the input form elements remain present or that confirmation
screen elements (like a confirmation heading) are absent. This ensures that
invalid values with quantity equal to zero and empty reason fields are properly
rejected and do not allow progression to the confirmation screen.
🧹 Nitpick comments (1)
src/Eccube/Service/RefundRequestService.php (1)

127-142: ⚡ Quick win

rename() は異なるファイルシステム間で失敗します。

Docker や分散ストレージ環境で一時ディレクトリと本保存ディレクトリが異なるマウントポイントにある場合、rename() が失敗します。copy() + unlink() へのフォールバックを追加するとより堅牢になります。

♻️ 修正案
             $finalPath = $finalDir.'/'.$info['key'];
-            if (!`@rename`($tempPath, $finalPath)) {
-                throw new \RuntimeException(sprintf('Failed to move temp file: %s', $info['key']));
+            if (!`@rename`($tempPath, $finalPath)) {
+                // rename fails across filesystems; fallback to copy + unlink
+                if (!`@copy`($tempPath, $finalPath) || !`@unlink`($tempPath)) {
+                    throw new \RuntimeException(sprintf('Failed to move temp file: %s', $info['key']));
+                }
             }
🤖 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 `@src/Eccube/Service/RefundRequestService.php` around lines 127 - 142, The
rename() function fails when moving files between different filesystems (common
in Docker or distributed storage environments). In the foreach loop in
RefundRequestService.php where tempFiles are being moved, replace the simple
rename() call with a more robust approach that attempts rename() first, then
falls back to copy() followed by unlink() if rename() fails. This ensures the
file move operation succeeds across filesystem boundaries. Update the error
handling to throw an exception only if both the rename and the fallback
copy+unlink operations fail.
🤖 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.

Inline comments:
In `@src/Eccube/Controller/Mypage/RefundRequestController.php`:
- Around line 174-176: The return value of the isTokenValid() method is not
being checked in the POST request handler of RefundRequestController, which
bypasses CSRF token validation. Modify the code to check the boolean return
value of isTokenValid() and prevent further processing if the token is invalid.
If isTokenValid() returns false, the refund request processing should not
continue. Add a conditional check after calling isTokenValid() to verify it
returned true before proceeding with the refund application logic.

---

Duplicate comments:
In `@tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php`:
- Around line 371-415: The testValidationQuantityZero() and
testValidationReasonEmpty() test methods only verify that the response is
successful but do not explicitly confirm that invalid input causes the form to
remain on the input screen rather than proceeding to the confirmation page. Add
explicit assertions in both methods to verify that the input screen is retained
when validation fails, such as checking that the input form elements remain
present or that confirmation screen elements (like a confirmation heading) are
absent. This ensures that invalid values with quantity equal to zero and empty
reason fields are properly rejected and do not allow progression to the
confirmation screen.

---

Nitpick comments:
In `@src/Eccube/Service/RefundRequestService.php`:
- Around line 127-142: The rename() function fails when moving files between
different filesystems (common in Docker or distributed storage environments). In
the foreach loop in RefundRequestService.php where tempFiles are being moved,
replace the simple rename() call with a more robust approach that attempts
rename() first, then falls back to copy() followed by unlink() if rename()
fails. This ensures the file move operation succeeds across filesystem
boundaries. Update the error handling to throw an exception only if both the
rename and the fallback copy+unlink operations fail.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: da791865-a2f8-4f9e-af24-a84241ba7df0

📥 Commits

Reviewing files that changed from the base of the PR and between 9d4e99e and c7c7ffd.

⛔ Files ignored due to path filters (1)
  • e2e/fixtures/evidence.png is excluded by !**/*.png
📒 Files selected for processing (8)
  • app/config/eccube/packages/eccube.yaml
  • e2e/tests/front-refund-request.spec.ts
  • src/Eccube/Controller/Mypage/RefundRequestController.php
  • src/Eccube/Resource/template/default/Mypage/refund_request.twig
  • src/Eccube/Resource/template/default/Mypage/refund_request_confirm.twig
  • src/Eccube/Service/RefundRequestService.php
  • tests/Eccube/Tests/Service/RefundRequestServiceTest.php
  • tests/Eccube/Tests/Web/Mypage/RefundRequestControllerTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Eccube/Resource/template/default/Mypage/refund_request.twig

Comment thread src/Eccube/Controller/Mypage/RefundRequestController.php
ttokoro20240902 and others added 9 commits June 16, 2026 10:28
完了画面・履歴画面が ec-orderRole > ec-orderRole__detail (注文表示用・左寄せ)
を使っていたため左に寄って見栄えが悪かった。入力・確認画面と同じく
ec-mypageRole 直下にコンテンツを配置し、ボタンは ec-RegisterRole__actions で
中央寄せにする。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
受注一覧 (admin/Order/index.twig) と同じ書式に揃え、
.card + text-center で中央寄せ表示にする。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
SearchRefundRequestType の status (EntityType + multiple: true) は未選択でも
空の ArrayCollection を返すため、Repository の `is_iterable && !empty` チェックを
通過してしまい、IN 句が "IN (NULL)" となって何にもマッチしなくなっていた。
count() による要素数チェックを追加して空コレクションを除外する。

回帰防止テスト testGetQueryBuilderBySearchDataEmptyStatusCollection を追加。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
受注一覧 (admin/Order/index.twig) と同じ件数切替セレクタを追加。
pager.twig に 'routes' を渡しておらず初期表示で 500 になっていた問題も修正。

- refund_request.twig:
  - CSV エクスポートボタン横に表示件数セレクタを追加 (10/20/30/.../100件)
  - pager include に 'routes': 'admin_refund_request_page' を追加
- PHPUnit: testIndexPageCountPersistsInSession / testIndexInitialShowsAllRefundRequests を追加
- E2E (admin-refund-request.spec.ts): 件数セレクタの選択と URL 反映を検証、
  status IN (NULL) 回帰防止の初期表示テストを追加

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- 外側コンテナを c-outsideBlock--small → c-outsideBlock に変更し
  画面いっぱい使えるよう左右余白を統一
- ページャを row.justify-content-md-center.pb-4.mb-4 でラップして中央寄せ
- pager include に pagination.totalItemCount > 0 のガードを追加

admin/Order/index.twig と同じ構造に揃えた。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
受注一覧 (admin/Order/index.twig) は結果テーブル領域も
c-outsideBlock__contents (padding 15px) で囲んでいるが、返品申請管理は
カードを直接 c-outsideBlock に置いていたため左右に余白が無かった。
標準と同じ構造に揃える。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
c-conversionAreaをc-outsideBlock--smallの外に移動し、受注編集画面と同じレイアウト構造に統一。

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CodeRabbit:
- 返品申請確認の POST で isTokenValid() の戻り値を確認するよう修正(CSRF 保護)
- 入力バリデーションテストで確認画面遷移しないことを明示
- E2E setup-fixtures.php で既存会員でも発送済み注文と返品申請(NEW)を必ず保証
- E2E admin-refund-request.spec.ts の条件分岐スキップを expect で必須化

Rector:
- RefundRequest::removeRefundRequestFile の setRefundRequest(null) を引数省略形に
- RefundRequestService::saveTempFile の (string) キャストを除去
- RefundRequestRepositoryTest で ArrayCollection を use 文に整理

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mtb_refund_request_status.csv を追加していたが import_csv/{ja,en}/definition.yml
への登録漏れがあり、eccube:fixtures:load が "mtb_refund_request_status.csv is
undefined in definition.yml" で失敗していた。これにより dockerbuild の
新規インストールフロー全 PHP バージョンで起動失敗していた問題を修正。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.38710% with 90 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.48%. Comparing base (2dc6a22) to head (53e1f26).
⚠️ Report is 30 commits behind head on 4.4.

Files with missing lines Patch % Lines
...cube/Controller/Mypage/RefundRequestController.php 83.41% 33 Missing ⚠️
...Controller/Admin/Order/RefundRequestController.php 91.14% 17 Missing ⚠️
src/Eccube/Service/MailService.php 73.80% 11 Missing ⚠️
src/Eccube/Service/RefundRequestService.php 89.23% 7 Missing ⚠️
src/Eccube/Repository/RefundRequestRepository.php 89.28% 6 Missing ⚠️
src/Eccube/Entity/RefundRequest.php 87.17% 5 Missing ⚠️
src/Eccube/Entity/RefundRequestFile.php 76.19% 5 Missing ⚠️
src/Eccube/Event/RefundRequestEvent.php 25.00% 3 Missing ⚠️
src/Eccube/Controller/Mypage/MypageController.php 80.00% 1 Missing ⚠️
.../Eccube/Repository/RefundRequestFileRepository.php 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##              4.4    #6838      +/-   ##
==========================================
- Coverage   74.82%   74.48%   -0.34%     
==========================================
  Files         463      477      +14     
  Lines       24029    24804     +775     
==========================================
+ Hits        17979    18476     +497     
- Misses       6050     6328     +278     
Flag Coverage Δ
Unit 74.48% <88.38%> (-0.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

詳細画面には「申請内容」「操作」の2つの .card-body があり、
条件分岐スキップを expect に置き換えた結果アサーション本体が
常時実行されるようになって strict-mode violation が顕在化した。
申請内容カードに限定する .first() を入れて解消。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@dotani1111 dotani1111 added this to the 4.4.0 milestone Jun 17, 2026
@dotani1111 dotani1111 self-assigned this Jun 17, 2026
- マイページ注文履歴の返品履歴ボタンを, 返品申請がある明細にのみ表示する(getRefundRequestCountsByCustomer を利用してN+1回避, 受入基準に準拠)。

- 管理のステータス更新で canApplyTransition による遷移ガードを追加し, 未定義・適用不可の遷移を確実に400で返す。

- 管理の添付ファイル配信に Content-Type / X-Content-Type-Options: nosniff / 末尾区切り付きパスガードを追加し, フロント配信と挙動を統一。

- アップロードのサイズ超過(15M)テストと, 履歴ボタン条件表示テストを追加。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[4.4] 返品申請機能 — 実装設計

2 participants