受注の一括削除が別の受注を削除するため、一括削除を撤去する by ttokoro20240902 · Pull Request #7209 · EC-CUBE/ec-cube

ttokoro20240902 · 2026-10-02T07:26:17Z

概要(Overview・Refs Issue)

管理画面の受注の一括削除 (/admin/order/bulk_delete) を撤去します。

受注一覧のチェックボックスの値は出荷 (Shipping) の ID ですが、OrderController::bulkDelete() は受け取った ID を受注の ID として OrderRepository::find() に渡していました。複数配送の受注が 1 件でもあると以降の ID がずれ、選んだものとは別の受注が物理削除されます。

方針(Policy)

一括削除のボタンは注文一括削除ボタンを非表示に修正 #3709 以来コメントアウトされており、標準の画面からは操作できません。出荷単位で削除する案 (#350 【受注一覧】削除処理を行っても受注が削除されない #3596) もマージされていません。
Issue のコメントのとおり、誤作動を避けるため機能としては提供せず、ルート・確認モーダル・JavaScript を削除します。直接のリクエストでも動かなくなります。

実装に関する補足(Appendix)

OrderController::bulkDelete() と admin_order_bulk_delete ルートを削除しました。不要になった use Eccube\Common\Constant; も削除しています。
admin/Order/index.twig から、#btn_bulk_delete のハンドラ、{# TODO 削除処理は将来バージョンで対応 ... #} のコメント、#bulkDeleteModal を削除しました。
- モーダル内の <ul id="bulkErrors"> は、ステータス一括変更のモーダルと ID が重複していました。confirmationModal_js.twig が参照するのはステータス一括変更側の要素なので、動作に影響はありません。
参照元がなくなった翻訳キー admin.order.delete__confirm_title / admin.order.delete__confirm_message (ja / en) を削除しました。
zap/containing_urls.txt に /admin/order/bulk_delete が残っています。ZAP のシナリオ整備 (ZAP: zst シナリオを 4.4 に移行し、4.4 の新機能をスキャン対象に加える #7190) の範囲なので、この PR では変更していません (今後は 404 になります)。

テスト（Test)

OrderControllerTest::testBulkDelete (受注の ID を送っていたため不具合を検出できていなかったもの) を、testBulkDeleteIsNotAvailable に置き換えました。
- 受注の ID と出荷の ID を CSRF トークン付きで POST すると 404 になり、受注が削除されないことを確認します。
- 修正前のコントローラでは 302 is identical to 404 で失敗することを確認しています。
E2E の order_受注削除 (EA0401-UC08-T01) を削除しました (非表示のモーダルを JS で開いてボタンを押していたテストです)。
ローカルの実行結果
- vendor/bin/phpunit tests/Eccube/Tests/Web/Admin/Order/OrderControllerTest.php: OK (18 tests)
- vendor/bin/phpstan analyse src: No errors
- vendor/bin/php-cs-fixer / vendor/bin/rector --dry-run (変更ファイル): 差分なし
- npx playwright test --project=setup --project=admin-tests admin-order.spec.ts: 18 passed (一括メール送信・ステータス一括変更・納品書の一括出力を含む)

相談（Discussion）

なし

マイナーバージョン互換性保持のための制限事項チェックリスト

4.4 はメジャー更新のため、次の破壊的変更を含みます。

既存機能の仕様変更はありません — ルート admin_order_bulk_delete を削除しています
フックポイントの呼び出しタイミングの変更はありません
フックポイントのパラメータの削除・データ型の変更はありません
twigファイルに渡しているパラメータの削除・データ型の変更はありません
Serviceクラスの公開関数の、引数の削除・データ型の変更はありません
入出力ファイル(CSVなど)のフォーマット変更はありません

移行時の注意: app/template/admin/Order/index.twig をコアからコピーして上書きしている場合、url('admin_order_bulk_delete') が残っているとルートが見つからず受注一覧がエラーになります。上書きしたテンプレートから、#btn_bulk_delete のハンドラと #bulkDeleteModal を削除してください。admin_order_bulk_delete を参照しているプラグインも同様です。

レビュワー確認項目

動作確認
コードレビュー
E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
互換性が保持されているか
セキュリティ上の問題がないか
- 権限を超えた操作が可能にならないか
- 不要なファイルアップロードがないか
- 外部へ公開されるファイルや機能の追加ではないか
- テンプレートでのエスケープ漏れがないか

🤖 Generated with Claude Code

Summary by CodeRabbit

機能変更
- 管理画面の受注一覧から、一括削除機能を削除しました。削除確認モーダルと一括操作メニューの削除項目も表示されなくなります。
- 受注の一括削除リクエストは受け付けなくなりました。

受注一覧のチェックボックスの値は出荷の ID だが、一括削除は受け取った ID を受注の ID として扱っていた。複数配送の受注があると ID がずれ、選んだものと別の受注を削除する。ボタンは #3709 以来コメントアウトされ標準の画面からは使えないため、機能として提供せず、ルート・確認モーダル・JavaScript を削除する。直接のリクエストは 404 になることをテストで固定する。 Fixes #7208 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

coderabbitai · 2026-10-02T07:26:36Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (2)

.claude/skills/eccube-phpunit/SKILL.md — Agent Skill
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: be9892a0-8246-4df0-8932-11ed4a1e22e1

📥 Commits

Reviewing files that changed from the base of the PR and between e8a9bb0 and 6b922f8.

📒 Files selected for processing (6)

e2e/tests/admin-order.spec.ts
src/Eccube/Controller/Admin/Order/OrderController.php
src/Eccube/Resource/locale/messages.en.yaml
src/Eccube/Resource/locale/messages.ja.yaml
src/Eccube/Resource/template/admin/Order/index.twig
tests/Eccube/Tests/Web/Admin/Order/OrderControllerTest.php

💤 Files with no reviewable changes (5)

src/Eccube/Resource/locale/messages.ja.yaml
src/Eccube/Resource/locale/messages.en.yaml
e2e/tests/admin-order.spec.ts
src/Eccube/Controller/Admin/Order/OrderController.php
src/Eccube/Resource/template/admin/Order/index.twig

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

Walkthrough

管理画面の受注一括削除機能を削除しました。ルート、一覧画面の操作要素、確認用翻訳を除去しました。HTTPテストは、エンドポイントが404を返し、受注が残ることを検証します。E2Eの受注削除テストも削除しました。

Changes

受注一括削除の削除

Layer / File(s)	Summary
削除ルートと画面要素の削除 `src/Eccube/Controller/Admin/Order/OrderController.php`, `src/Eccube/Resource/template/admin/Order/index.twig`, `src/Eccube/Resource/locale/messages.*.yaml`	一括削除ルート、一覧画面の削除操作と確認モーダル、確認用翻訳を削除しました。
一括削除テストの更新 `tests/Eccube/Tests/Web/Admin/Order/OrderControllerTest.php`, `e2e/tests/admin-order.spec.ts`	HTTPテストは一括削除リクエストが404を返し、対象の受注が残ることを確認します。E2Eの受注削除テストを削除しました。

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: nanasess

Merge Risk: ⚪ Minimal · up to 6b922

Bulk order deletion is intentionally unavailable, and the updated HTTP test verifies that requests return 404 without deleting orders. The checked-in ZAP cleanup workflow does not depend on the old endpoint, so no concrete merge-blocking risk remains.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 6b922

The change removes the faulty bulk-deletion operation rather than adding another deletion mechanism. The compared implementation reduces destructive reachability, and the updated test asserts that the former request returns 404 while orders remain.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The directly affected sensitive assets are order records reachable through the retired administrative operation. Its deletion authority is reduced: the compared head controller no longer translates supplied IDs into entity removals through this route.

Security Findings and Attack Paths

inferred — The pre-existing path from request-controlled IDs to physical order deletion is retired, not newly exposed. The examined controller and template changes do not introduce a replacement destructive path.

Trust Boundaries and Controls

observed — The old handler called isTokenValid() before processing IDs. The PR removes both the protected operation and its sensitive sink; it does not replace token enforcement with an unprotected deletion implementation.

Resilience and Maintainability Implications

inferred — The retired operation is stopped before initiating order mutation, rather than relying on rollback after deletion. No new reservation, retry, cleanup, or recovery state is introduced by the compared route-removal change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	タイトルは、別の受注を削除する問題を理由に受注の一括削除機能を撤去するという、変更の主目的を正確に説明しています。
Linked Issues check	✅ Passed	Issue `#7208` は、選択した受注を正しく削除する案に加えて、安全に提供しない場合はルート、確認モーダル、JavaScript を削除する案を示しています。本PRは後者を実装しています。`OrderController::bulkDelete()` と `admin_order_bulk_delete` ルートを削除し、注文一覧の確認モーダルと送信処理を削除しています。翻訳キーも削除していま…
Out of Scope Changes check	✅ Passed	変更は `#7208` の一括削除機能の撤去に限定されています。コントローラ、ルート、注文一覧のUIとJavaScript、関連翻訳、機能テスト、該当E2Eテストを変更しています。これらの変更は撤去後の動作とテストの整合に必要です。無関係な変更は確認できません。

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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.}

codecov · 2026-10-02T07:46:56Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.51%. Comparing base (e8a9bb0) to head (6b922f8).

Additional details and impacted files

@@            Coverage Diff             @@
##              4.4    #7209      +/-   ##
==========================================
+ Coverage   78.47%   78.51%   +0.04%     
==========================================
  Files         651      651              
  Lines       31384    31373      -11     
==========================================
+ Hits        24628    24634       +6     
+ Misses       6756     6739      -17

Flag	Coverage Δ
Unit	`78.51% <ø> (+0.04%)`	⬆️

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.

nanasess

LGTM

nanasess approved these changes Oct 2, 2026

View reviewed changes

ttokoro20240902 requested a review from dotani1111 October 5, 2026 00:03

Navigation Menu

受注の一括削除が別の受注を削除するため、一括削除を撤去する - #7209

受注の一括削除が別の受注を削除するため、一括削除を撤去する#7209
ttokoro20240902 wants to merge 1 commit into
4.4from
fix/issue-7208-remove-order-bulk-delete

ttokoro20240902 commented Oct 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Oct 2, 2026

Uh oh!

nanasess left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ttokoro20240902 commented Oct 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

概要(Overview・Refs Issue)

方針(Policy)

実装に関する補足(Appendix)

テスト（Test)

相談（Discussion）

マイナーバージョン互換性保持のための制限事項チェックリスト

レビュワー確認項目

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Oct 2, 2026

Codecov Report

Uh oh!

nanasess left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ttokoro20240902 commented Oct 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 2, 2026 •

edited

Loading