fix: マスタEntityをtraitで拡張するとマスタデータ管理から消える問題を修正 (#5400 #6273)#6892
Open
nanasess wants to merge 2 commits into
Open
fix: マスタEntityをtraitで拡張するとマスタデータ管理から消える問題を修正 (#5400 #6273)#6892nanasess wants to merge 2 commits into
nanasess wants to merge 2 commits into
Conversation
|
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 skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDeviceType拡張プラグインを追加し、マスタデータ管理画面での選択・保存をE2Eで確認するテストを導入しました。あわせて、Trait Proxyのファイルパス解決をsourceFile基準に変更しています。 Changesマスタエンティティ拡張プラグイン追加
Trait Proxyパス解決ロジックの修正
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant plugin_extend_spec as plugin-extend.spec.ts
participant MasterEntityExtensionLocal as MasterEntityExtensionLocal
participant MasterDataManagePage as MasterDataManagePage
participant PluginTestConfig as PluginTestConfig
plugin_extend_spec->>MasterEntityExtensionLocal: start(page, db, config)
MasterEntityExtensionLocal->>PluginTestConfig: traits設定を含むローカルプラグインを生成
plugin_extend_spec->>MasterDataManagePage: go(page)
plugin_extend_spec->>MasterDataManagePage: 選択肢が存在("mtb_device_type")
MasterDataManagePage-->>plugin_extend_spec: boolean
plugin_extend_spec->>MasterDataManagePage: 選択(label)
plugin_extend_spec->>MasterDataManagePage: 保存()
plugin_extend_spec->>MasterEntityExtensionLocal: plugin無効化/削除
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Entity/Master 配下のマスタEntityをプラグインのtraitで拡張すると、 マスタデータ管理ページのプルダウンから該当テーブルが消え、さらに プラグインインストール時のschema updateがカラムを自動追加しない問題を修正する。 原因は TraitProxyAttributeDriver / ReloadSafeAttributeDriver の getAllClassNames() で、Proxyパス生成時に basename() を用いて Entity/Master 等のサブディレクトリ情報を落としていたため。 $sourceFile のパス構造ごと proxies ディレクトリへ変換するよう修正した。 - 親ドライバ TraitProxyAttributeDriver: プルダウン表示 (EC-CUBE#5400) - 子ドライバ ReloadSafeAttributeDriver: install時の自動schema update (EC-CUBE#6273) の両方を修正 (annotation時代の EC-CUBE#5401 + EC-CUBE#6274 を4.4で統合的に解決) 回帰テストを Codeception から Playwright へ移植: - MasterEntityExtension プラグイン (テストデータ) - MasterDataManagePage / MasterEntityExtensionLocal - plugin-extend.spec.ts に test_master_entity_extension を追加 - plugin-test.yml の plugin-extend マトリクスに追加 Closes EC-CUBE#5400 Closes EC-CUBE#6273 Refs EC-CUBE#5401 EC-CUBE#6274 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Eccube/Doctrine/ORM/Mapping/Driver/ReloadSafeAttributeDriver.php (1)
94-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftプロキシパス解決ロジックの重複（
TraitProxyAttributeDriverと同一）
ReloadSafeAttributeDriverはTraitProxyAttributeDriverを継承しているにもかかわらず、$projectDir→$this->trait_proxies_directoryの置換ロジック（Lines 94-108）が親クラスのgetAllClassNames()（TraitProxyAttributeDriver.php Lines 74-91）と完全に重複しています。今回のIssue#5400/#6273修正が両ファイルに同時に手動反映されている点からも、将来同種の修正が片方だけに適用されて再度ズレるリスクがあります。共通処理を親クラスの
protectedメソッド（例:resolveProxyFile(string $sourceFile, string $path): string）に切り出し、両ドライバから呼び出す形にすると保守性が向上します。🤖 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/Doctrine/ORM/Mapping/Driver/ReloadSafeAttributeDriver.php` around lines 94 - 108, The proxy path resolution block in ReloadSafeAttributeDriver duplicates the same logic already present in TraitProxyAttributeDriver::getAllClassNames(), so extract the shared $projectDir to $this->trait_proxies_directory replacement into a protected helper on the parent class (for example, a resolveProxyFile-style method) and have both drivers call it instead of keeping separate copies. Keep the existing sourceFile/path handling and file_exists check behavior, but centralize the transformation so future fixes for Issue `#5400` / `#6273` only need to be made in one place.
🤖 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 `@src/Eccube/Doctrine/ORM/Mapping/Driver/ReloadSafeAttributeDriver.php`:
- Around line 94-108: The proxy path resolution block in
ReloadSafeAttributeDriver duplicates the same logic already present in
TraitProxyAttributeDriver::getAllClassNames(), so extract the shared $projectDir
to $this->trait_proxies_directory replacement into a protected helper on the
parent class (for example, a resolveProxyFile-style method) and have both
drivers call it instead of keeping separate copies. Keep the existing
sourceFile/path handling and file_exists check behavior, but centralize the
transformation so future fixes for Issue `#5400` / `#6273` only need to be made in
one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd05317e-d308-4f58-a7c8-6ccccf82a208
📒 Files selected for processing (8)
.github/workflows/plugin-test.ymlcodeception/_data/plugins/MasterEntityExtension-1.0.0/Entity/DeviceTypeTrait.phpcodeception/_data/plugins/MasterEntityExtension-1.0.0/composer.jsone2e/models/plugins/master-entity-extension-local.tse2e/pages/master-data-manage.page.tse2e/tests/plugin-extend.spec.tssrc/Eccube/Doctrine/ORM/Mapping/Driver/ReloadSafeAttributeDriver.phpsrc/Eccube/Doctrine/ORM/Mapping/Driver/TraitProxyAttributeDriver.php
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6892 +/- ##
=======================================
Coverage 75.32% 75.32%
=======================================
Files 519 519
Lines 25483 25483
=======================================
Hits 19196 19196
Misses 6287 6287
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:
|
test_master_entity_extension がマスタデータ管理画面へ遷移したまま 無効化() を呼び、プラグイン管理画面前提の i.fa-pause が見つからず タイムアウトしていた (MySQL/PostgreSQL 両方で失敗)。 マスタデータ検証後に /store/plugin へ戻してから無効化・削除するよう修正。 なお選択・保存 (プルダウン検証の本丸) は両DBで成功しており、 失敗は後片付けのナビゲーションのみが原因。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 6, 2026
Closed
ttokoro20240902
approved these changes
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要(Overview・Refs Issue)
Closes #5400
Closes #6273
(関連PR: #5401 / #6274)
Eccube\Entity\Master配下のマスタ Entity をプラグインの trait で拡張すると、以下の 2 つの不具合が発生していました。/admin/setting/system/masterdata) のプルダウンから、拡張したテーブルが消える。schema:updateが自動実行されず、trait で追加したカラムが作成されない (手動でbin/console doctrine:schema:update --forceを実行すれば作成される)。いずれも根本原因は同一で、Proxy パス解決を担う 2 つのドライバの
getAllClassNames()が、Proxy パス生成時にbasename($sourceFile)を用いてEntity/Master/などのサブディレクトリ情報を落としていたためです。この結果Entity/Master/DeviceTypeの Proxy が「存在しない」と誤判定され、メタデータ収集からエンティティが漏れていました。方針(Policy)
basename()でファイル名だけに丸めるのをやめ、$sourceFileのパス構造ごとtrait_proxies_directoryへ変換するように修正しました。これはEntityProxyServiceが Proxy を生成する際のパス規則 (projectDir 相対でサブディレクトリを保持) と一致します。TraitProxyAttributeDriver(通常のメタデータ収集 = プルダウン表示) → マスタデータのEntityを拡張すると、マスタデータ管理ページでテーブルを選択できなくなる #5400ReloadSafeAttributeDriver(SchemaService::executeCallback経由の install 時 schema update) → JobテーブルがTraitでカラム追加できない #6273src/Eccube/Entity/決め打ちではなく$sourceFileを汎用的に変換するため、app/Customize/Entityやapp/Plugin/*/Entityのサブディレクトリ配下も同時に救済します。実装に関する補足(Appendix)
src/Eccube/Doctrine/ORM/Mapping/Driver/TraitProxyAttributeDriver.phpsrc/Eccube/Doctrine/ORM/Mapping/Driver/ReloadSafeAttributeDriver.phpstr_replace()が置換しなかった場合 (Proxy ディレクトリ配下に写像されないパス) は Proxy 扱いしないよう$proxyFile !== $sourceFileのガードを追加しています。codeception/_data/plugins/MasterEntityExtension-1.0.0/(trait は annotation → PHP8 属性へ移植)e2e/pages/master-data-manage.page.ts/e2e/models/plugins/master-entity-extension-local.tse2e/tests/plugin-extend.spec.tsにtest_master_entity_extensionを追加.github/workflows/plugin-test.ymlのplugin-extendマトリクスに追加テスト（Test)
✅
vendor/bin/php-cs-fixer fix(PSR-12, 違反なし)✅
vendor/bin/phpstan analyse(level 6, エラーなし)✅ SQLite 環境で A/B 対照実測を実施し、因果を確認 (
mtb_device_typeをMasterEntityExtensionプラグインで拡張):eccube:plugin:install後のmtb_device_typeid, name, sort_no, discriminator_type, notesnotesが自動追加id, name, sort_no, discriminator_type両ケースとも install 時の一時 Proxy は
Master/サブパスを保持して生成される (gen -> /tmp/proxy_xxx/src/Eccube/Entity/Master/DeviceType.php) ため、差分の唯一の要因はドライバのbasenameバグであることを確認済みです。E2E (Playwright
plugin-extend/test_master_entity_extension) は本 PR の CI で検証されます。相談（Discussion）
マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目
🤖 Generated with Claude Code
Summary by CodeRabbit