improvement: 未実装のイベント定数を削除 (#6529)#6894
Conversation
ProductClassController 向けに定義されている以下のイベント定数を削除する。 これらは dispatch されておらず、定数名・イベント文字列値ともにコア/テスト/ プラグインのいずれからも参照されていない完全な未使用コード。 - ADMIN_PRODUCT_PRODUCT_CLASS_INDEX_INITIALIZE - ADMIN_PRODUCT_PRODUCT_CLASS_INDEX_CLASSES - ADMIN_PRODUCT_PRODUCT_CLASS_EDIT_INITIALIZE - ADMIN_PRODUCT_PRODUCT_CLASS_EDIT_COMPLETE - ADMIN_PRODUCT_PRODUCT_CLASS_EDIT_UPDATE - ADMIN_PRODUCT_PRODUCT_CLASS_EDIT_DELETE Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthrough`EccubeEvents.php``から未実装または整理対象の管理画面・フロント画面イベント定数が削除され、CSVエクスポートのイベント説明が共通イベント利用に更新されました。 Changesイベント定数とCSV説明の整理
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6894 +/- ##
==========================================
- Coverage 75.79% 75.76% -0.04%
==========================================
Files 546 546
Lines 27136 27136
==========================================
- Hits 20569 20559 -10
- Misses 6567 6577 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@ttokoro20240902 @dotani1111 |
#6529 の追加調査で洗い出した、dispatch されておらず購読側からも 参照されない完全な dead 定数 46 件を削除する。 - ADMIN 系 16 件(ADMIM タイポ定数・旧管理画面の残骸を含む) - FRONT_CART 系 13 件(3.x→4.x のカート処理書き換えで dispatch が消えた残骸) - FRONT_SHOPPING 系 11 件(旧購入フローの confirm/delivery/payment 系) - FRONT_PRODUCT 系 5 件 / FRONT_MYPAGE 系 1 件 リポジトリ全体(src/app(Plugin・Customize)/tests/html)で定数名参照・ イベント文字列値参照の両方を検索し、いずれも 0 件であることを確認済み。 dispatch されていないため実挙動の互換性影響はない。 セクション見出しコメントは既存記法(CsvImportController 等)に合わせ残置。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ADMIN_ORDER_CSV_EXPORT_SHIPPING の削除に追従。 OrderController::exportCsv() は受注 CSV・出荷 CSV のどちらのエクスポート でも ADMIN_ORDER_CSV_EXPORT_ORDER の 1 イベントのみを dispatch しており、 _SHIPPING は配線されていない dead 定数だった。列追加を購読で行う際の 案内が誤ったイベント名を指していたため修正する。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
nanasess
left a comment
There was a problem hiding this comment.
LGTM 👍 追加コミット分もあわせて検証しました。
検証内容
1. 削除された46定数の残存参照 → 0件
PR head(42806b1052)の git 管理下全ファイルに対し、削除された46定数の「定数名参照」「イベント文字列値参照」の両方を検索し、いずれも 0 件であることを確認しました。4.4 からのマージで取り込まれた新規コードが削除済み定数を参照してしまう事故も発生していません。
2. 網羅性・過剰削除 → 問題なし
PR head に残る 165 定数はすべて EccubeEvents:: で参照されており、未使用定数の残りはゼロです。使用中の定数が誤って削除された形跡もありません。
3. 構文チェック → php -l パス
CSV Skill の記述修正について
ADMIN_ORDER_CSV_EXPORT_SHIPPING の削除に追従した Skill 修正は的確です。実コードでも裏取りしました。
OrderController::exportCsv()は受注CSV(L278CsvType::CSV_TYPE_ORDER)・出荷CSV(L291CsvType::CSV_TYPE_SHIPPING)の両方から呼ばれる- しかし内部で dispatch しているのは
ADMIN_ORDER_CSV_EXPORT_ORDERのみ(L347)
つまり _SHIPPING は配線されていない dead 定数であり、Skill が存在しないイベント名を案内していた状態でした。ドキュメントを実態に合わせる修正として妥当です。
補足
4.4 マージにより FRONT_PRODUCT_DETAIL_JSON_LD(商品詳細の JSON-LD 構造化データ用フック)が 1 件増えています。本 PR の削除対象とは無関係で dispatch も配線済みのため、影響はありません。定数数は 210 → 165(46 削除 + 1 追加)となります。
BC 影響については、削除対象がいずれも dispatch されていない dead 定数のため、購読していたプラグインが存在したとしても元々コールされておらず、実挙動の互換性影響はないという整理に同意します。
ご対応ありがとうございました。
概要(Overview・Refs Issue)
Closes #6529
EccubeEventsに定義されている未使用イベント定数を削除します。dispatch されておらず、購読側からも参照されない完全な dead 定数が対象です。本 PR は 2 段階で構成されます。
1. Issue 本文記載の
ProductClassController向け 6 定数2. 追加調査で洗い出した 46 定数(#6529 コメント参照)
EccubeEvents.phpの全定数について、コードベース全体(src//app/(Plugin・Customize 含む)/tests//html/)で「定数名参照」「イベント文字列値参照」の両方を検索し、参照 0 件・dispatch 実績 0 件を確認したもの。ADMIMタイポ定数、Ajax 化・画面統合で使われなくなった残骸FRONT_MYPAGE_MYPAGE_DELETE_INITIALIZE合計 6 + 46 = 52 定数を削除します。
方針(Policy)
dispatch()実績がなく、イベントとして発火されていません。定数名 / イベント文字列値の両面でリポジトリ全体を検索しても定義箇所以外の参照は 0 件でした。/** Admin/Xxx/YyyController */)および各アクションのインラインコメント(// index等)は、同ファイルの既存記法(FileControllerセクションが定数ゼロでもコメントを残している)に合わせて残置しました。ADMIN_ORDER_CSV_EXPORT_SHIPPINGについて補足:OrderController::exportCsv()は受注 CSV・出荷 CSV のどちらのエクスポートでもADMIN_ORDER_CSV_EXPORT_ORDERの 1 イベントのみを dispatch しており、_SHIPPINGは配線されていない dead 定数でした。削除に合わせて、列追加を購読で行う際の案内(CSV 関連の開発者向けドキュメント)が誤ったイベント名を指していた箇所も修正しています。実装に関する補足(Appendix)
CartControllerTest… OK (5 tests)Admin/Order/OrderControllerTest… OK (18 tests)Admin/Product/ProductClassControllerTest… OK (15 tests, 38 assertions)マイナーバージョン互換性保持のための制限事項チェックリスト
undefined constantになります(未実装のイベントハンドラの削除 #6529 の受入方針に基づく削除)。Summary by CodeRabbit