feat: EC-CUBE 4.4への対応 - #197
Conversation
Coupon42 を EC-CUBE 4.4(Symfony 7.4 / Doctrine ORM 3.0 / PHP 8.2+)へ対応し コードを Coupon44 へ改名。 - Annotation → PHP 属性(Entity の #[ORM\*] / #[UniqueEntity]、Controller の #[Route]/#[Template]、Processor の #[ShoppingFlow]/#[OrderFlow]) - @ParamConverter → #[MapEntity](Symfony\Bridge\Doctrine\Attribute\MapEntity) - Doctrine ORM 3.0: 型付きプロパティ・flush() 引数廃止対応・AbstractRepository ::save(AbstractEntity): void 互換化・@extends 付与 - Symfony 7 / PurchaseFlow インターフェースの : void 戻り値・Assert 名前付き引数 - Form の getName() → getBlockPrefix() - composer(code/name/version=4.4.0) / phpunit.xml.dist を PHPUnit 11 形式へ - CI(ci.yml)を 4.4 × PHP8.2-8.5 × MySQL8/PostgreSQL に更新し static-analysis 追加 - docker-compose 開発環境 / phpstan(level6) / rector / php-cs-fixer / CLAUDE.md 追加 - Tests を Coupon44 / PHPUnit 11 / ORM3 に追従、重複テスト整理 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughクーポンプラグインをCoupon42からCoupon44へ全面移行した。Entity/Repository/Form/Controller/Service/PurchaseFlowプロセッサ/Event等の名前空間をCoupon44に変更し、Doctrineアノテーションを属性へ、メソッドシグネチャに型宣言を追加。CI/Docker開発環境・静的解析ツール(php-cs-fixer/rector/phpstan)を新設・更新した。 Changesプラグイン本体のCoupon44移行
CI/Docker開発環境と静的解析ツール整備
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Customer
participant CouponShoppingController
participant CouponRepository
participant CouponService
participant CouponOrderRepository
Customer->>CouponShoppingController: shoppingCoupon(couponCd)
CouponShoppingController->>CouponRepository: findActiveCoupon(couponCd)
CouponRepository-->>CouponShoppingController: Coupon or null
CouponShoppingController->>CouponService: existsCouponProduct(Coupon, Order)
CouponService-->>CouponShoppingController: couponProducts
CouponShoppingController->>CouponService: recalcOrder(Coupon, couponProducts)
CouponService-->>CouponShoppingController: discount
CouponShoppingController->>CouponService: saveCouponOrder(Order, Coupon, couponCd, Customer, discount)
CouponService->>CouponOrderRepository: save(CouponOrder)
CouponShoppingController-->>Customer: redirect to shopping
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 |
PR #197 のレビュー指摘を反映。 - クーポン利用画面の「利用する/しない」トグルJSが旧ブロックプレフィックス (front_plugin_coupon_shopping) を参照しており、実レンダリング名 (coupon_use) と不一致で動作していなかったため修正(既存バグ)。 - CouponService::existsDepthCategory の親カテゴリ探索が反転・find(entity) 誤用 だったため getParent() での再帰に修正。未使用になった CategoryRepository の 注入を除去(本体が商品保存時に祖先カテゴリを保存するため実害は無いデッドコード整理)。 - ORM3 で nullable になった getter の算術/比較を ?? 0・null ガードで防御 (CouponRepository::checkCouponUseTime, CouponStateProcessor, CouponProcessor)。 - deleteCoupon / removeCouponOrder / CouponController::edit のループ内 flush を ループ外 1 回に集約。 - テスト統合時に欠落していた値引き明細の税区分(NON_TAXABLE)アサーションを CouponProcessorTest に復活。 検証: PHPUnit 118 tests OK / php-cs-fixer 差分ゼロ / rector 変更ゼロ / phpstan level 6 エラーゼロ。フロントのトグル挙動は Playwright で修正前後を確認。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
Tests/Web/Admin/OrderControllerTest.php (1)
188-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueキャンセル時クーポン表示テストは恒久的にスキップ状態。
markTestSkipped()によりキャンセル時のクーポン表示シナリオのカバレッジが失われるが、根本原因(DAMA DoctrineTestBundle と本体 StockReduceProcessor の悲観ロックの非互換性)はプラグイン非依存であり、コメントで十分に説明・検証済み。本体側の制約が解消され次第、@phpstan-ignore以降のコードで再有効化できるよう、本体側 Issue へのリンクなど追跡手段を残しておくと良い。Also applies to: 222-222
🤖 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/Web/Admin/OrderControllerTest.php` around lines 188 - 205, `testOrderEditWithCouponCancel()` is being permanently skipped, so the coupon-cancel coverage is lost. Keep the skip only as a temporary workaround, but add a clear tracking reference in the test comment or nearby documentation to the upstream issue/blocker tied to `StockReduceProcessor` and the `DAMA DoctrineTestBundle` pessimistic lock conflict, so the `markTestSkipped()` block can be safely re-enabled later..github/workflows/ci.yml (4)
1-20: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winワークフローに明示的な
permissionsが定義されていません。静的解析ツール(zizmor)が指摘する通り、
permissions:ブロックが存在しないためGITHUB_TOKENが既定の(過剰な)権限で実行されます。ワークフロー全体、または各ジョブに最小権限を明示することを推奨します。♻️ 提案例
+permissions: + contents: read + jobs: run-on-linux:Also applies to: 161-163
🤖 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 @.github/workflows/ci.yml around lines 1 - 20, The CI workflow currently runs with the default broad GITHUB_TOKEN permissions because there is no explicit permissions block. Add a minimal permissions declaration for the workflow in ci.yml, or set job-level permissions on run-on-linux if different scopes are needed, so the GitHub Actions job uses only the required access.Source: Linters/SAST tools
62-64: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
actions/checkoutにpersist-credentials: falseの設定がありません。静的解析ツール(zizmor)が指摘する通り、
actions/checkout@v4はデフォルトで認証情報(GITHUB_TOKEN)を.git/configに永続化します。後続ステップでの意図しない資格情報の漏洩・悪用を防ぐため、リポジトリの書き込みが不要なチェックアウトにはpersist-credentials: falseを追加することを推奨します(本ジョブ内の他の checkout ステップにも同様に適用できます)。♻️ 提案例(static-analysis ジョブ)
- name: Checkout uses: actions/checkout@v4 + with: + persist-credentials: falseAlso applies to: 174-191
🤖 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 @.github/workflows/ci.yml around lines 62 - 64, The Checkout step using actions/checkout@v4 is missing the persist-credentials setting, so the default GITHUB_TOKEN may be written into .git/config. Update the Checkout invocation in this workflow to set persist-credentials to false, and apply the same change to the other checkout steps referenced by the review. Use the Checkout step under the static-analysis job as the primary place to fix, and keep the change localized to the actions/checkout usage.Source: Linters/SAST tools
81-90: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
actions/cache@v4のキャッシュポイズニング対策を検討してください。静的解析ツール(zizmor)が
actions/cache@v4の既定挙動(全ブランチでの書き込みキャッシュ有効化)によるキャッシュポイズニングのリスクを指摘しています。フォークからのPRでも本ワークフローが実行される場合、restore/saveを分離しデフォルトブランチのみ書き込み許可にする、またはキャッシュキーにブランチ/PR情報を含める等の緩和策を検討してください。Also applies to: 193-202
Source: Linters/SAST tools
65-68: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
nanasess/setup-php@masterはブランチ参照であり、サプライチェーンリスクがあります。
@masterはタグや特定コミットではなく可変参照のため、上流での予期しない変更や侵害が即座にCIへ反映される可能性があります。特定のタグ/リリースまたはコミットSHAへの固定を推奨します。Also applies to: 178-181
🤖 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 @.github/workflows/ci.yml around lines 65 - 68, The Setup PHP step is using the mutable nanasess/setup-php@master reference, which should be pinned for supply-chain safety. Update the workflow to use a fixed tag/release or commit SHA for the Setup PHP action wherever it appears, including the repeated Setup PHP entries, so the CI runs against an immutable version of nanasess/setup-php.
🤖 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 `@Controller/CouponShoppingController.php`:
- Around line 60-62: The shoppingCoupon action is using a different template
resolution style than the other coupon screens. Update the #[Template]
annotation on shoppingCoupon to use the same
`@Coupon44/default/shopping_coupon.twig` reference as the rest of the coupon UI,
so CouponShoppingController stays consistent with the shared template lookup
approach.
In `@Repository/CouponOrderRepository.php`:
- Around line 50-55: The CouponOrderRepository::save method signature is
incompatible with AbstractRepository::save, so restore it to match the parent
exactly. Update save(AbstractEntity $entity): mixed in CouponOrderRepository and
remove the custom void return type and any renamed parameter that breaks
inheritance, while keeping the persist/flush behavior inside the method.
In `@Service/CouponService.php`:
- Line 108: `recalcOrder()` が返す `discount_price` は文字列の小数値なので、`saveCouponOrder()`
の `int $discount` と型契約がずれています。`CouponService::recalcOrder()` から
`saveCouponOrder()` を呼ぶ箇所と `saveCouponOrder()` / `CouponOrder::setDiscount()`
の定義を見直し、円単位にするなら呼び出し前に明示的に整数化して渡すか、`discount` を小数対応の型に揃えて
`Controller/CouponShoppingController` 側も含めて一貫させてください。
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1-20: The CI workflow currently runs with the default broad
GITHUB_TOKEN permissions because there is no explicit permissions block. Add a
minimal permissions declaration for the workflow in ci.yml, or set job-level
permissions on run-on-linux if different scopes are needed, so the GitHub
Actions job uses only the required access.
- Around line 62-64: The Checkout step using actions/checkout@v4 is missing the
persist-credentials setting, so the default GITHUB_TOKEN may be written into
.git/config. Update the Checkout invocation in this workflow to set
persist-credentials to false, and apply the same change to the other checkout
steps referenced by the review. Use the Checkout step under the static-analysis
job as the primary place to fix, and keep the change localized to the
actions/checkout usage.
- Around line 65-68: The Setup PHP step is using the mutable
nanasess/setup-php@master reference, which should be pinned for supply-chain
safety. Update the workflow to use a fixed tag/release or commit SHA for the
Setup PHP action wherever it appears, including the repeated Setup PHP entries,
so the CI runs against an immutable version of nanasess/setup-php.
In `@Tests/Web/Admin/OrderControllerTest.php`:
- Around line 188-205: `testOrderEditWithCouponCancel()` is being permanently
skipped, so the coupon-cancel coverage is lost. Keep the skip only as a
temporary workaround, but add a clear tracking reference in the test comment or
nearby documentation to the upstream issue/blocker tied to
`StockReduceProcessor` and the `DAMA DoctrineTestBundle` pessimistic lock
conflict, so the `markTestSkipped()` block can be safely re-enabled later.
🪄 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: fa274bc8-689c-4d59-8a39-448bd816d98a
📒 Files selected for processing (51)
.github/workflows/ci.yml.github/workflows/main.yml.gitignoreCLAUDE.mdController/Admin/CouponController.phpController/Admin/CouponSearchModelController.phpController/CouponShoppingController.phpEntity/Coupon.phpEntity/CouponDetail.phpEntity/CouponOrder.phpEvent.phpForm/Type/CouponDetailType.phpForm/Type/CouponSearchCategoryType.phpForm/Type/CouponType.phpForm/Type/CouponUseType.phpNav.phpPluginManager.phpREADME.mdRepository/CouponDetailRepository.phpRepository/CouponOrderRepository.phpRepository/CouponRepository.phpResource/.php-cs-fixer.dist.phpResource/rector.phpResource/template/admin/regist.twigResource/template/admin/search_category.twigResource/template/admin/search_product.twigResource/template/default/shopping_coupon.twigService/CouponService.phpService/PurchaseFlow/Processor/CouponProcessor.phpService/PurchaseFlow/Processor/CouponStateProcessor.phpTests/Fixtures/CreateCouponTrait.phpTests/Form/Type/CouponTypeTest.phpTests/Repository/CouponCouponOrderRepositoryTest.phpTests/Repository/CouponCouponRepositoryTest.phpTests/Service/CouponServiceTest.phpTests/Service/PurchaseFlow/CouponProcessorTest.phpTests/Service/PurchaseFlow/Processor/CouponProcessorTest.phpTests/Web/Admin/CouponControllerTest.phpTests/Web/Admin/CouponSearchModelControllerTest.phpTests/Web/Admin/OrderControllerTest.phpTests/Web/CouponControllerTest.phpTests/bootstrap.phpcomposer.jsondocker-compose.dev.ymldocker-compose.mysql.ymldocker-compose.pgsql.ymldocker-compose.ymldockerbuild/dev-framework.yamldockerbuild/grant_to_dbuser.sqlphpstan.neon.distphpunit.xml.dist
💤 Files with no reviewable changes (1)
- Tests/Service/PurchaseFlow/CouponProcessorTest.php
PR #197 の CodeRabbit 指摘を裏どりのうえ対応。 - CouponShoppingController の #[Template] を他画面と同じ名前空間表記 (@Coupon44/default/shopping_coupon.twig) に統一。旧表記でも描画は可能 (Playwright / web テストで確認済み) だが解決方式を揃える。 - CouponService::saveCouponOrder の $discount を int から string|int|float へ。 recalcOrder() の戻り値(小数通貨では小数を含む DECIMAL 文字列)を int で受けると 小数が切り捨てられるため、CouponOrder::setDiscount と型を揃える(JPY では影響なし)。 - CI (ci.yml): GITHUB_TOKEN を最小権限 (contents: read) に。全 actions/checkout に persist-credentials: false を付与 (zizmor 指摘のクレデンシャル永続化対策)。 - OrderControllerTest のスキップに本体側追跡用の TODO/参照を追記。 備考: CouponOrderRepository::save(): void の非互換指摘は誤り。本体 AbstractRepository::save(AbstractEntity $entity): void と完全一致(裏どり済み)。 検証: PHPUnit 118 OK / php-cs-fixer 0 / rector 変更なし / phpstan level 6 エラー0。 クーポン利用画面の描画とトグルは Playwright で再確認。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai レビューありがとうございます。裏どりのうえ対応しました(commit bf5a9bf)。 Actionable / インライン(各スレッドに返信済み)
Nitpick
検証: PHPUnit 118 OK / php-cs-fixer 0 / rector 変更なし / phpstan level 6 エラー0。クーポン利用画面の描画とトグルは Playwright で再確認済みです。 |
|
Tip For best results, initiate chat on the files or code changes.
インラインの各スレッドについても個別に確認します。何か追加の懸念が出てきた場合は改めてお知らせします。 (=^・ω・^=) |
|
@ttokoro20240902 1点確認です。 「カテゴリクーポンが対象カテゴリの子孫商品にも効く」というテストを入れて頂きたいです! |
dotani1111
left a comment
There was a problem hiding this comment.
4.4/Symfony7 対応をレビュー・実機検証しました。Coupon42→44 の移行、および購入フロー関与部分(#[ShoppingFlow]/#[OrderFlow] 登録・値引き明細 OrderItemType::DISCOUNT 税率0%保持・小数通貨対応の型調整)を確認。CI 全 green。実機で「全商品500円割引クーポン作成→フロント会員購入で適用」を通し、値引き -500円・お支払い合計が正しく減額されることを実数で確認しました。Blocker なし。LGTM 👍
nanasess
left a comment
There was a problem hiding this comment.
4.4 環境(PHP 8.5.4 / Symfony 7.4.13 / Doctrine ORM 3.6.2 / DBAL 4.4.3 / SQLite)へ本 PR のブランチをインストールし、実機で動作確認したうえでレビューしました。
クーポンの登録・一覧・有効/無効切り替え、購入フローでのクーポン適用(値引き・税再計算・注文確定・利用履歴)を確認し、単一配送の購入導線は完全に動作しました。移行の網羅性も高く、テストのデータプロバイダ static 化など PHPUnit 10+ 対応も適切です。
ただし クーポンを適用した状態で「お届け先の複数指定」を確定すると購入フローが 500 になります(下記コメント参照)。本体 4.4 で TaxRuleService::calcTax() に型が付いたことに追随できていないのが原因で、4.4 対応 PR としては対応が必要と考え request-changes とさせてください。
実機動作検証の結果
| 検証項目 | 結果 |
|---|---|
| クーポン新規登録 | ✅ 「クーポンを登録しました。」 |
| クーポン一覧 | ✅ 残発行枚数が 99/100 に減算 |
| 有効/無効の切り替え(PUT + CSRF) | ✅ 「クーポンの状態を変更しました。」 |
| 購入フローへのクーポン欄差し込み | ✅ ご注文手続き画面に表示 |
| クーポンコード適用 | ✅ 合計 ¥4,080 → 値引 -¥500 → 支払 ¥3,580、消費税も ¥325 に再計算 |
| 注文確定 | ✅ 受注 payment_total=3580 / discount=500 |
| クーポン利用履歴 | ✅ plg_coupon_order に記録 |
| 複数配送 + クーポン | ❌ 500 |
| ページ管理からのテンプレート編集 | ❌ 反映されない |
良い点
- 単一配送のクーポン適用が値引き・税再計算・注文確定・利用履歴まで完全に動作する
- クーポンの有効/無効切り替えが PUT + CSRF トークンで実装されている
- テストのデータプロバイダが
public function→public static functionに正しく変更されている(PHPUnit 10+ で必須) permissions: contents: read/persist-credentials: falseなど CI のハードニングが入っている
検証の過程で取り下げた指摘
existsDepthCategory()の親カテゴリ遡上を有効化すると割引適用範囲が変わるのでは: 本体は商品保存時に$Category->getPath()で祖先カテゴリまでdtb_product_categoryに登録します(ProductController.php:544/CsvImportController.php:1199)。実 DB でも商品がカテゴリ 4(3→1 の子孫)に対し 1,2,3,4 全行を保持していることを確認したため、親遡上は標準データでは冗長で、金額は変わりません。修正自体は docblock の意図どおりで正しいと判断しました- PHPStan level 6 で
expects string, string|null givenが出るのでは: PHPStan 2.1.44 で最小再現を作り level 5/6/7/8 を実測したところ、このargument.typeは level 8 でのみ報告されました。PR の「level 6 エラーゼロ」と整合します
| public function isLowerLimitCoupon($productCoupon, $lowerLimitMoney) | ||
| public function isLowerLimitCoupon(array $productCoupon, int $lowerLimitMoney): bool | ||
| { | ||
| $subTotal = 0; |
There was a problem hiding this comment.
[major] 複数配送 + クーポン利用で購入フローが 500 になります(isLowerLimitCoupon() / 214 行の calcTax() 呼び出し)
getCouponProducts()(402 行)は rounding_type_id に明示的に null を入れ得ます。
'rounding_type_id' => $orderItem->getRoundingType() ? $orderItem->getRoundingType()->getId() : null,本体 4.3 → 4.4 で calcTax() に型が付いたことが引き金です。
// upstream/4.3
public function calcTax($price, $taxRate, $RoundingType, $taxAdjust = '0')
// 4.4
public function calcTax(string $price, string $taxRate, int $RoundingType, string $taxAdjust = '0'): string再現手順(4.4 実機で確認済み)
- 商品を数量 2 でカートに入れ、ゲスト購入で「ご注文手続き」まで進む
- クーポンコードを適用する(
-¥500が反映される) - 「お届け先を追加する」→ お届け先の複数指定画面
- 「選択したお届け先に送る」をクリック
→ 「システムエラーが発生しました。」画面になり、購入を続行できません。
front.ERROR - ["Eccube\\Service\\TaxRuleService::calcTax(): Argument #3 ($RoundingType)
must be of type int, null given, called in .../Service/CouponService.php on line 214"]
#0 Service/CouponService.php(214)
#1 Service/PurchaseFlow/Processor/CouponProcessor.php(160)
#2 src/Eccube/Service/PurchaseFlow/ItemHolderValidator.php(28)
#3 src/Eccube/Service/PurchaseFlow/PurchaseFlow.php(150)
#4 src/Eccube/Controller/AbstractShoppingController.php(39)
#5 src/Eccube/Controller/ShippingMultipleController.php(266)
到達経路(本体コードで確認)
ShippingMultipleControllerが既存の Shipping / OrderItem を全削除(162-172 行)- RoundingType を設定せずに OrderItem を作り直す(237-245 行)
- その直後に
executePurchaseFlow($Order, false)(265 行) PurchaseFlow::validate()はitemHolderValidators(149 行)をitemHolderPreprocessors(164 行)より先に実行する- → RoundingType を設定する
TaxProcessor(ItemHolderPreprocessor)より前にCouponProcessor::validate()が走る
recalcOrder()(178 行)には rounding_type_id === null のフォールバックがありますが、foreach が値渡しのため $couponProducts に書き戻されず、同じ配列を受け取る isLowerLimitCoupon() にはフォールバックがありません。
修正案: フォールバックを private メソッドに切り出し、recalcOrder() と isLowerLimitCoupon() の両方で使う。
private function resolveTaxValues(int $productClassId, array $value): array
{
if ($value['tax_rate'] < 1 || $value['rounding_type_id'] === null) {
$ProductClass = $this->productClassRepository->find($productClassId);
$TaxRule = $this->taxRuleRepository->getByRule($ProductClass->getProduct(), $ProductClass);
$value['tax_rate'] = $TaxRule->getTaxRate();
$value['rounding_type_id'] = $TaxRule->getRoundingType()->getId();
}
return $value;
}あわせて testValidateWithLowerLimit に rounding_type_id = null(RoundingType 未設定の OrderItem)ケースを追加すると退行を防げます。
There was a problem hiding this comment.
ご指摘のとおりでした。裏どりして修正しました(ae08221)。
CouponService に resolveTaxValues() を切り出し、recalcOrder() と isLowerLimitCoupon() の両方から呼ぶようにしました。ご提案の形に加えて、ProductClass が解決できない場合はデフォルト課税規則にフォールバックする null 安全化を入れています。
実機で before / after を確認しました(4.4 / PHP 8.2.31 / SQLite、ゲスト購入 + 数量2 + クーポン適用 → お届け先の複数指定 → 選択したお届け先に送る)。
- 修正前(
isLowerLimitCoupon()のフォールバックのみ外した状態):
Eccube\Service\TaxRuleService::calcTax(): Argument #3 ($RoundingType) must be of type int, null given, called in .../Service/CouponService.php on line 209で 500 - 修正後: 注文手続き画面へ正常に戻り、クーポン(-¥500)も維持。そのまま注文確定まで到達(
payment_total=6660/discount=500、plg_coupon_order記録、利用枚数 100→99)
テストも追加しました。
CouponServiceTest::testIsLowerLimitCouponWithoutRoundingType:RoundingType未設定のOrderItemからexistsCouponProduct()経由で得た配列(rounding_type_idが null)で計算できることCouponProcessorTest::testValidateWithoutRoundingType: 同状態でvalidate()が失敗しないこと(複数配送導線の回帰ガード)
| * @see https://github.com/EC-CUBE/coupon-plugin/issues/128 | ||
| */ | ||
| #[Route(path: '/plugin/coupon/shopping/shopping_coupon', name: 'plugin_coupon_shopping')] | ||
| #[Template('@Coupon44/default/shopping_coupon.twig')] |
There was a problem hiding this comment.
[major] @Coupon44/... へのテンプレート参照変更で、ページ管理からの編集が反映されなくなっています
// 移行前
@Template("Coupon42/Resource/template/default/shopping_coupon.twig")
// 移行後
#[Template('@Coupon44/default/shopping_coupon.twig')]この 2 つは 解決先が異なります。
| 参照形式 | 解決先 |
|---|---|
@Coupon44/default/shopping_coupon.twig |
app/Plugin/Coupon44/Resource/template/default/shopping_coupon.twig(プラグイン本体) |
Coupon44/Resource/template/default/shopping_coupon.twig(移行前の形式) |
app/template/default/Coupon44/...(テーマ側) |
dtb_page には 名前空間なしのパスで登録されています。
$ sqlite3 var/eccube.db "select id, file_name, url from dtb_page where file_name like '%Coupon44%';"
98|Coupon44/Resource/template/default/shopping_coupon|plugin_coupon_shopping本体の PageController は $templatePath.'/'.$Page->getFileName().'.twig'(eccube_theme_front_dir = app/template/default)に dumpFile() するため、ページ管理で編集した内容はテーマ側に保存されるのに、コントローラはプラグイン本体を読みます。
実機で確認: 管理画面のページ管理(/admin/content/page/98/edit)でテンプレートに目印を追記して保存したところ、
$ ls -la app/template/default/Coupon44/Resource/template/default/
-rw-r--r-- 1 nanasess nanasess 2987 shopping_coupon.twig # ← 新規作成された
$ grep -rl "COUPON-PAGE-EDIT-MARKER-98765" app/ app/Plugin/Coupon44/
app/template/default/Coupon44/Resource/template/default/shopping_coupon.twig保存先はテーマ側のみで、コントローラが読むプラグイン本体側は変わりません。つまり店舗側は「編集したのに反映されない」状態になります。
(同じ問題が EC-CUBE/ProductReview-plugin#95 にもあります。)
修正案: いずれかに統一してください。
- (a)
#[Template]の参照を移行前と同じ名前空間なし形式に戻す - (b)
@Coupon44/...を維持するなら、dtb_pageへの登録を廃止し、ページ管理の対象外であることを明示する
There was a problem hiding this comment.
ご指摘のとおりでした。名前空間なし表記に戻しました(ae08221)。
もともと f613669 では移行前と同じ名前空間なし表記でしたが、bf5a9bf で CodeRabbit の「表記統一」指摘に対応した際に @Coupon44/... へ変えてしまったのが原因です。当時「旧表記でも描画される」ことは確認していましたが、ページ管理経由の上書きを見落としていました。失礼しました。
修正案 (a) を採用し、再発防止のため理由をコメントで明記しています。
// dtb_page.file_name (PluginManager::createPage で登録) と一致させるため, あえて名前空間
// (@Coupon44) を使わない。名前空間なしの場合はテーマ側 (app/template/default/...) が
// プラグイン本体 (app/Plugin) より優先して解決されるため, 管理画面のページ管理で編集した
// 内容が反映される。@Coupon44 にするとプラグイン本体に固定解決され, ページ管理での編集が
// 無視されてしまうので変更しないこと。
#[Template('Coupon44/Resource/template/default/shopping_coupon.twig')]実機確認: twig のパス解決順が app/template/default(テーマ)→ src/Eccube/Resource/template/default → app/Plugin であることを debug:config twig paths で確認し、テーマ側に app/template/default/Coupon44/Resource/template/default/shopping_coupon.twig(ページ管理の保存先)を置いた状態でクーポン入力画面を開くと、テーマ側の内容が描画されることを確認しました(修正前は @Coupon44 によりプラグイン本体側が描画されていました)。
なお Event.php の addSnippet('@Coupon44/...') は移行前も @Coupon42/... で dtb_page 管理外のため、変更していません。
| * @param CouponOrder $CouponOrder | ||
| */ | ||
| public function save($CouponOrder) | ||
| public function save(AbstractEntity $CouponOrder): void |
There was a problem hiding this comment.
[minor] ORM 3 の scoped flush 廃止で、フロー途中の flush が UnitOfWork 全体に波及しています
4.3 では $em->flush($CouponOrder) と対象エンティティだけをフラッシュしていましたが、ORM 3 でスコープ付き flush が廃止されたため $em->flush() に置き換わっています。これ自体は避けられない変更です。
ただし影響は save() だけではなく、移行前は以下がすべて scoped flush でした。
| ファイル | 移行前 |
|---|---|
Repository/CouponOrderRepository.php:47 |
$em->flush($CouponOrder) |
Service/PurchaseFlow/Processor/CouponStateProcessor.php:112 |
flush($Coupon) |
Service/PurchaseFlow/Processor/CouponStateProcessor.php:128 |
flush($Coupon) |
Service/PurchaseFlow/Processor/CouponProcessor.php:196 |
flush($CouponOrder) |
Service/PurchaseFlow/Processor/CouponProcessor.php:203 |
flush($Coupon) |
とくに CouponStateProcessor::process() は管理画面の受注編集フローの途中(ItemHolderPreprocessor)で呼ばれるため、後続の itemHolderPostValidator が失敗しても中間状態が確定してしまう経路が生まれます。
修正案: save() は親クラス(AbstractRepository::save = persist のみ)どおりに戻し、呼び出し側(CouponService::saveCouponOrder / CouponStateProcessor::process)で明示的に flush する方が副作用の所在が読みやすくなります。戻せない事情がある場合は saveAndFlush() のように親の契約と区別できる名前をご検討ください。
There was a problem hiding this comment.
ご指摘のとおりでした。save() は親 AbstractRepository::save() と同じ persist のみに戻し、flush は呼び出し側で明示するようにしました(ae08221)。
調べたところ、CouponStateProcessor の 2 箇所(103 / 124 行)は save 直後の 112 / 127 行で自前 flush しており、save() 内の flush は冗長でした。そのため実際に save() の flush に依存していたのは CouponService::saveCouponOrder の 1 箇所だけで、同所に $this->entityManager->flush(); を追加しています(CouponStateProcessor は変更なし)。
テスト側は save() 後に検索する 3 ケース(CouponCouponOrderRepositoryTest の testSave / testFindUseCouponNonMember / testGetCouponOrder)に明示的な flush を追加しました。
購入フロー(クーポン適用 → 注文確定)も実機で確認し、plg_coupon_order の記録・利用枚数の減算が正常であることを確認済みです。
| // TODO 本体側 (EC-CUBE/ec-cube) で DAMA DoctrineTestBundle と StockReduceProcessor の | ||
| // 悲観ロックの非互換が解消され次第、markTestSkipped を外して再有効化する。 | ||
| // 追跡: https://github.com/EC-CUBE/ec-cube (StockReduceProcessor / EntityManager::lock) | ||
| $this->markTestSkipped('本体 StockReduceProcessor の悲観ロックが DAMA テストトランザクションと非互換のためスキップ(プラグイン非依存)'); |
There was a problem hiding this comment.
[minor] CANCEL 時のクーポン返却テストが全面 skip され、追跡 Issue 番号がありません
testOrderEditWithCouponCancel が markTestSkipped で丸ごと無効化されています。移行前(origin/4.4)には存在し実行されていたテストなので、カバレッジとしては退行です。
skip 理由(StockReduceProcessor の悲観ロックがテストハーネスと非互換)は妥当と考えます(本体の app/config/eccube/services_test.yaml:12-13 で TransactionListener がテスト時に無効化されており、DBAL 4 のラッパ層でトランザクションが開始されないため UnitOfWork::lock() が失敗します)。
ただし CouponStateProcessor::process() の枚数戻し分岐は $orderStatusId == CANCEL || PROCESSING || RETURNED の or 条件なので、生きている testOrderEditWithCouponReturn(RETURNED)が同じ分岐を通ります。失われるのは CANCEL 固有の遷移経路の検証です。
お願い
- (a) TODO の追跡先がリポジトリ URL のみで Issue 番号が無く、事実上追跡されない状態です。EC-CUBE/ec-cube 側に Issue を作成して番号を書いていただけますでしょうか
- (b) Web リクエストを介さず
CouponStateProcessor::process()を直接呼ぶユニットテスト(CANCEL の Order を渡しCoupon::getCouponUseTime()が +1 されることを検証)で代替できないでしょうか
There was a problem hiding this comment.
両方対応しました(ae08221)。
(a) 本体側 Issue を作成しました: EC-CUBE/ec-cube#7016
起票にあたり、コンテナ内で当該テストのスキップを一時解除して実際の例外とスタックトレースを採取しました。
Doctrine\ORM\TransactionRequiredException: An open transaction is required for this operation.
vendor/doctrine/orm/src/UnitOfWork.php:2261
vendor/doctrine/orm/src/EntityManager.php:485
src/Eccube/Service/PurchaseFlow/Processor/StockReduceProcessor.php:82
src/Eccube/Service/OrderStateMachine.php:155 → 48
src/Eccube/Controller/Admin/Order/EditController.php:182
ご指摘の分析どおり、ORM 3 の UnitOfWork::lock() は DBAL Connection::isTransactionActive() を要求しますが、DAMA は StaticDriver::connect() でドライバ層の接続に beginTransaction() するため false になり、さらに services_test.yaml で TransactionListener が無効化されているため DBAL 層でもトランザクションが開かれない、という流れでした。テスト側の TODO コメントに Issue 番号を記載しています。
(b) 代替ユニットテストを追加しました: Tests/Service/PurchaseFlow/Processor/CouponStateProcessorTest
Web リクエストを介さず CouponStateProcessor::process() を直接呼び、以下を検証しています。
- キャンセル / 返品で
Coupon::getCouponUseTime()が +1、CouponOrderのorder_change_statusが true・order_dateが null になること - 戻しが発行枚数で頭打ちになること
- 戻し済みの受注を対象外ステータスへ戻すと再度使用状態になること(-1)
なお実装を読み進める過程で、process() の分岐条件にある PROCESSING は supports() が対象ステータスを NEW / PAID / IN_PROGRESS / DELIVERED / CANCEL / RETURNED に限定しているため到達しないことが分かりました(実質デッドコード)。この挙動も testProcessWithUnsupportedOrderStatus(購入処理中 / 決済処理中)で明文化しています。
| $this->assertEquals(-1000, $OrderItem->getPrice()); | ||
| $this->assertEquals($Coupon->getCouponName(), $OrderItem->getProductName()); | ||
| // クーポン値引き明細は不課税で追加される | ||
| $this->assertSame(TaxType::NON_TAXABLE, $OrderItem->getTaxType()->getId()); |
There was a problem hiding this comment.
[minor] 旧 CouponProcessorTest にあった isDiscount() のアサーションが移行先にありません
削除された Tests/Service/PurchaseFlow/CouponProcessorTest.php には self::assertTrue($DiscountItem->isDiscount()) があり、追加される明細の OrderItemType が「値引き」であることを検証していました。
TaxType::NON_TAXABLE の検証は移行先の 3 ケース(153 / 208 / 234 行)へ移植されていますが、isDiscount()(OrderItemType::DISCOUNT)の検証は移植されていません。
addCouponDiscountItem() は setOrderItemType($DiscountType) を行っているので、ここが壊れると受注金額の集計(値引き明細の合算)が狂いますが、現状のテストは processorName / price / productName / taxType しか見ていないため検知できません。
修正案: self::assertTrue($OrderItem->isDiscount(), '値引き明細として追加されている'); を追加。
There was a problem hiding this comment.
ご指摘のとおり移植漏れでした。3 ケース(153 / 208 / 234 行付近)すべてに追加しました(ae08221)。
// OrderItemType が「値引き」で追加される (受注金額の集計に影響する)
$this->assertTrue($OrderItem->isDiscount(), '値引き明細として追加されている');| - 3306:3306 | ||
| options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 | ||
| mysql8: | ||
| image: mysql:8 |
There was a problem hiding this comment.
[minor] CI の DB イメージが 4.4 のサポート範囲とずれています
| 場所 | MySQL | PostgreSQL |
|---|---|---|
.github/workflows/ci.yml |
mysql:8 | postgres:14 |
docker-compose.mysql.yml(本 PR で追加) |
mysql:8.4 | — |
docker-compose.pgsql.yml(本 PR で追加) |
— | postgres:18 |
本体 unit-test.yml |
8.4 | 18 / 13 |
同一 PR 内で CI とローカル検証環境の DB バージョンが食い違っています。移行前の ci.yml から引き継がれた設定で本 PR が壊したものではありませんが、4.4 対応 PR なので合わせておくのが自然かと思います。
なお database_server_version: 8(MySQL)は本体 4.4 と同値なので変更不要です。ずれているのは image と postgres 側です。
There was a problem hiding this comment.
ご指摘のとおり、同一 PR 内で食い違っていました。本体 4.4 に合わせました(ae08221)。
- services
mysql8.image:mysql:8→mysql:8.4 - services
postgres.image:postgres:14→postgres:18 - matrix pgsql
database_server_version:14→18 - matrix mysql8
database_server_version: 8は本体 4.4 と同値のため変更していません
|
|
||
| - name: Setup PHP | ||
| uses: shivammathur/setup-php@v2 | ||
| uses: nanasess/setup-php@master |
There was a problem hiding this comment.
[question] extensions: ':xdebug'(xdebug 無効化)の指定が落ちています
Setup PHP ステップから、元々あった extensions: ':xdebug' の指定が落ちています(static-analysis ジョブ側も同様)。xdebug が有効なままだとテスト実行が大幅に遅くなるため、意図的な変更かご確認ください。
There was a problem hiding this comment.
意図的な変更ではなく、移行時の取りこぼしでした。両ジョブ(run-on-linux / static-analysis)の Setup PHP に復活させました(ae08221)。
- name: Setup PHP
uses: nanasess/setup-php@master
with:
php-version: ${{ matrix.php }}
# xdebug が有効だとテスト実行が大幅に遅くなるため無効化する
extensions: ':xdebug'| use Plugin\Coupon42\Entity\Coupon; | ||
| use Plugin\Coupon42\Repository\CouponOrderRepository; | ||
| use Plugin\Coupon42\Repository\CouponRepository; | ||
| use Plugin\Coupon44\Entity\Coupon; |
There was a problem hiding this comment.
[nit] 未使用の use が残っています
use Plugin\Coupon44\Entity\Coupon;Event.php 内で Coupon クラスは一切参照されていません(コード・docblock とも)。移行前はコンストラクタで CouponRepository などを受け取っていたので、その整理の際の取り残しかと思います。
php-cs-fixer(no_unused_imports を含む @Symfony)でも検出されなかったため CI は通りますが、不要です。
There was a problem hiding this comment.
ご指摘のとおり未使用でした(Coupon の出現は log_info の文字列のみ)。削除しました(ae08221)。
| @@ -451,7 +451,7 @@ | |||
| </div> | |||
There was a problem hiding this comment.
[nit] Bootstrap 5 に存在しない mr-2 / mr-3 が残っています(450 行ほか)
Bootstrap 5.3 では右マージンのユーティリティは me-* にリネームされているため、これらは何のスタイルも当たらず、ボタン/アイコンの余白が詰まって表示されます(html/template/admin/assets/css/bootstrap.css に .me-2 は存在し .mr-2 のルールは存在しないことを確認済み。app.css の 1 ヒットは pug サンプルのコメント内でスタイル定義ではありません)。
該当は regist.twig の 411 / 450 / 473 行と、regist_product_list_prototype.twig:17 / regist_category_list_prototype.twig:16 です。同じ regist.twig の 391 行だけが me-2 になっており不統一です。
移行前(origin/4.4)から存在する既存の不具合で本 PR が壊したものではなく、本体側テンプレートにも同種の残骸があるため優先度は低いですが、Bootstrap 5.3 世代への移行 PR なので、ついでに直しておくと良さそうです。(別 PR での対応でも構いません。)
There was a problem hiding this comment.
ご指摘のとおりでした。本 PR に同梱して 5 箇所すべて修正しました(ae08221)。
regist.twig411 / 473 行:mr-3→me-3regist.twig450 行:mr-2→me-2regist_product_list_prototype.twig:17/regist_category_list_prototype.twig:16:mr-3→me-3
html/template/admin/assets/css/bootstrap.css に .mr-2 のルールが 0 件、.me-2 が存在することも確認しました。また ml-* / pr-* / float-left / badge-* / sr-only など他の Bootstrap 4 残骸は grep で 0 件でした。
default ブランチ (4.2) にも同じ 5 箇所があるため、別 PR を出しました: #200
nanasess のレビュー指摘 9 件を実機・コード両面で裏どりのうえ対応。 ■ 複数配送 + クーポンで購入フローが 500 になる問題を修正 CouponService に resolveTaxValues() を切り出し、recalcOrder() と isLowerLimitCoupon() の両方で税率・丸め規則のフォールバックを行う。 ShippingMultipleController は RoundingType を設定せずに OrderItem を作り直し、 RoundingType を設定する TaxProcessor (ItemHolderPreprocessor) より先に itemHolderValidator が走るため rounding_type_id が null で渡る。本体 4.4 の TaxRuleService::calcTax() は第3引数が非 nullable な int のため TypeError になっていた。 実機で「修正前は 500 / 修正後は正常」を確認済み。 ■ ページ管理からのテンプレート編集が反映されない問題を修正 CouponShoppingController の #[Template] を名前空間なし表記に戻す。dtb_page には 名前空間なしのパスが登録されており、ページ管理の保存先はテーマ側 (app/template/default/...)。@Coupon44 はプラグイン本体に固定解決されるため 編集が無視されていた。再発防止のため理由をコメントで明記。 ■ CouponOrderRepository::save() の flush を呼び出し側へ ORM 3 でスコープ付き flush が廃止され flush() が UnitOfWork 全体に波及するため、 save() は親 AbstractRepository と同じ persist のみに戻し、 CouponService::saveCouponOrder で明示的に flush する。CouponStateProcessor は save 直後に自前 flush があるため変更不要(内部 flush は冗長だった)。 ■ テスト - CouponStateProcessorTest を新規追加。キャンセル/返品時のクーポン枚数戻し、 発行枚数での頭打ち、ステータス復帰時の再使用を Web リクエストを介さず検証。 supports() の対象外ステータス (購入処理中/決済処理中) は process() に到達しない ことも明文化(process() 内の PROCESSING 条件は実質デッドコード) - CouponProcessorTest に isDiscount() の検証を追加(旧テストからの移植漏れ) - CouponProcessorTest / CouponServiceTest に RoundingType 未設定ケースを追加 - OrderControllerTest のスキップ理由に本体側 Issue 番号 (EC-CUBE/ec-cube#7016) を記載 ■ CI - DB イメージを 4.4 のサポート範囲に合わせる (mysql:8.4 / postgres:18) - Setup PHP に extensions: ':xdebug' を復活(テスト実行の高速化) ■ その他 - Event.php の未使用 import (Entity\Coupon) を削除 - Bootstrap 5 に存在しない mr-2 / mr-3 を me-2 / me-3 に修正(既存不具合) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@nanasess レビューありがとうございました。実機での再現手順まで添えていただき助かりました。指摘 9 件すべてを裏どりのうえ対応しました(ae08221)。各スレッドに個別に返信していますが、要点は以下です。
補足:
|
本体 4.4 の依存が doctrine/dbal 4.4.4 に上がり、SQLite では SQLitePlatform::getCreateDatabaseSQL が未サポートで例外になるため、 static-analysis ジョブ (SQLite) が Setup EC-CUBE で失敗していた。 Could not create database var/eccube.db for connection named default Operation "Doctrine\DBAL\Platforms\SQLitePlatform::getCreateDatabaseSQL" is not supported by platform. DB ファイルは doctrine:schema:create の接続時に生成されるため、本体 4.4 の unit-test.yml と同様に SQLite では database:create をスキップする。 MySQL / PostgreSQL の PHPUnit ジョブは従来どおり database:create を実行する。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nanasess
left a comment
There was a problem hiding this comment.
再確認しました — LGTM
major 2 件を含む 9 件すべてにご対応いただき、ありがとうございます。major と、挙動が変わる flush の除去は実機で確認しました。
検証環境: EC-CUBE 4.4.0-dev (
89dec55c) / PHP 8.5.4 / Symfony 7.4.13 / Doctrine ORM 3.6.2 / DBAL 4.4.3 / SQLite /APP_ENV=prod/ symfony server (HTTPS)
実機検証: [major] 複数配送 + クーポンで購入フローが 500
再現手順をそのまま再実行しました。
- 商品を数量 2 でカートに入れ、ゲスト購入で「ご注文手続き」へ
- クーポン
TEST44COUPONを適用(-¥500が反映) - 「お届け先を追加する」→ お届け先の複数指定画面
- 「選択したお届け先に送る」
→ 500 にならず購入手続き画面に戻り、そのまま注文完了まで通りました(dtb_order: total=6660 / discount=500、plg_coupon_order にレコード生成)。
resolveTaxValues() への切り出しに加えて、ProductClass が解決できない場合に getByRule() の既定課税規則へフォールバックする null 安全化まで入れていただいた点は、当初の提案より堅い実装だと思います(getByRule() は 4.4 本体で全引数 nullable なので引数なし呼び出しも正当です)。回帰テスト testValidateWithoutRoundingType の追加もありがとうございます。
実機検証: [major] テンプレート参照の名前空間化
テーマ側 app/template/default/Coupon44/Resource/template/default/shopping_coupon.twig に目印を入れて /plugin/coupon/shopping/shopping_coupon を開いたところ、目印が表示されました。dtb_page.file_name(Coupon44/Resource/template/default/shopping_coupon)と一致しています。
管理画面側は @Coupon44/... のまま残し、ページ管理の対象であるフロントだけ名前空間なしにする使い分けも適切です。「変更しないこと」の理由をコメントで明記いただいたので、再発も防げると思います。
実機検証: [minor] save() からの flush 除去
AbstractRepository::save() と同じ persist のみに戻した影響を確認しました。
| 操作 | 結果 |
|---|---|
| クーポン利用で注文完了 | plg_coupon_order にレコード生成(discount=500 / order_date セット)・coupon_use_time 99 → 98 |
| 管理画面で受注を「注文取消し」に変更 | coupon_use_time 98 → 99 に返却・order_change_status=1 / order_date=null |
CouponStateProcessor の 2 箇所は直後に自前 flush() があるため影響がなく、saveCouponOrder() にだけ flush() を追加する対応で正しく動作しています。
そのほかの指摘
| 指摘 | 対応 |
|---|---|
| [minor] CANCEL 時のクーポン返却テストの skip | 本体 issue #7016 を起票・ユニットテストで代替する旨を明記 |
[minor] isDiscount() のアサーション欠落 |
3 ケースすべてに追加 |
| [minor] CI の DB イメージ | mysql:8.4 / postgres:18 |
[question] extensions: ':xdebug' |
両ジョブに復活 |
[nit] 未使用の use |
削除 |
[nit] Bootstrap 4 の mr-2 / mr-3 |
5 箇所すべて me-* に修正 |
#7016 の起票にあたって実際にスキップを解除し、TransactionRequiredException のスタックトレースを採取いただいた点は、本体側での調査の助けになると思います。
CI は 10 ジョブ green を確認しました。approve します。
概要
クーポンプラグインを EC-CUBE 4.4(Symfony 7.4 / Doctrine ORM 3.0 / PHP 8.2+) に対応させ、コードを
Coupon42→Coupon44に改名します。4.3 とは非互換(属性必須・ORM 3・PHP 8.2+)のため、新規4.4ブランチへの取り込みです。参考: 4.3→4.4 マイグレーション手順 (doc #346) / 先行対応 related-product-plugin #91
変更内容
1. EC-CUBE 4.4 対応(コア移行)
@ORM\*/@UniqueEntity→#[ORM\*]/#[UniqueEntity]、Controller の@Route/@Template→#[Route]/#[Template](Sensio依存除去)、PurchaseFlow の@ShoppingFlow/@OrderFlow→Eccube\Attribute\*属性へ@ParamConverterの除去(本プラグイン固有):CouponController::enable/deleteを#[MapEntity(id: 'id')](Symfony\Bridge\Doctrine\Attribute\MapEntity)へ移行typeはDoctrine\DBAL\Types\Types::*)、flush($entity)→flush()、AbstractRepository::save(AbstractEntity $entity): voidとのシグネチャ互換化、Repository に@extends AbstractRepository<Entity>を付与process/validate/prepare/commit/rollback)の: void戻り値、Assert\*の名前付き引数化、コンストラクタプロモーション・戻り値/引数の型明示getName()→getBlockPrefix()、buildForm(): void/configureOptions(): voidphpunit.xml.distを<source>/<extensions>形式へPlugin\Coupon44\...に統一、データプロバイダの static 化、flush()・戻り値型・decimal の文字列比較(assertEquals)へ追従、重複していたTests/Service/PurchaseFlow/CouponProcessorTest.php(旧版)を整理code/version: 4.4.0/ Twig 名前空間@Coupon44を統一2. Docker Compose によるテスト環境
docker-compose.yml+dev/mysql/pgsqlオーバーレイ、dockerbuild/ghcr.io/ec-cube/ec-cube-php:8.2-apache-4.4)でプラグインを自動導入・有効化。APP_ENV=dev起動+dev 用 Cookie 設定(cookie_secure:false/cookie_samesite:lax)で HTTP ログインに対応3. 静的解析・整形ツール
phpstan.neon.dist(level 6・baseline なし)/Resource/rector.php/Resource/.php-cs-fixer.dist.phpを追加(.php設定は本体のPlugin\:サービス検出で 500 を避けるためResource/配下に配置)?string)と not-null カラムのcolumnType差分は本体コアと同様ignoreErrorsで許容(理由をコメント明記)CLAUDE.mdに開発・テスト手順、アーキテクチャ、移行・配置の注意を記載4. CI(
.github/workflows/)ci.yml: マトリクスを EC-CUBE 4.4 / PHP 8.2-8.5 / MySQL8・PostgreSQL に更新、checkout@v4・$GITHUB_OUTPUT化、--ignore-platform-req=ext-redis、phpunit 前にcache:warmup、static-analysisジョブ(php-cs-fixer / rector / phpstan)追加main.yml(release): 配布から開発・テスト用ファイル(docker-compose / dockerbuild / CLAUDE.md / phpstan / rector / php-cs-fixer)を除外5. レビュー指摘対応(commit ae08221)
CouponServiceにresolveTaxValues()を切り出し、recalcOrder()/isLowerLimitCoupon()の双方で税率・丸め規則を解決。ShippingMultipleControllerはRoundingType未設定でOrderItemを作り直し、TaxProcessor(ItemHolderPreprocessor)より先にitemHolderValidatorが走るためrounding_type_idが null で渡り、非 nullable になったTaxRuleService::calcTax()でTypeErrorになっていたCouponShoppingControllerの#[Template]をdtb_page.file_nameと一致する名前空間なし表記に戻す(@Coupon44はプラグイン本体に固定解決され、テーマ側に保存されるページ管理の編集が無視される)。再発防止の理由をコメントで明記CouponOrderRepository::save()を親と同じ persist のみに戻す: ORM 3 で flush が UnitOfWork 全体に波及するため、flush はCouponService::saveCouponOrderで明示(CouponStateProcessorは save 直後に自前 flush があり変更不要)CouponStateProcessorTest(キャンセル/返品の枚数戻し・発行枚数での頭打ち・ステータス復帰時の再使用。supports()対象外ステータスがprocess()に到達しないことも明文化)、RoundingType未設定ケース(CouponServiceTest/CouponProcessorTest)、isDiscount()の検証(移植漏れ)mysql:8.4/postgres:18)、extensions: ':xdebug'を復活Event.phpの未使用 import 削除、Bootstrap 5 に存在しないmr-*→me-*(既存不具合。4.2向けは fix: Bootstrap 5 に存在しない mr-2 / mr-3 を me-2 / me-3 に修正 #200)OrderControllerTest::testOrderEditWithCouponCancelの追跡先として本体側に Issue を作成: テスト環境で EntityManager::lock() が TransactionRequiredException になり、キャンセル遷移を含む受注編集の Web テストが書けない ec-cube#7016テスト
ローカルの docker-compose 4.4 環境(
8.2-apache-4.4/ MySQL 8)で以下を確認済み:複数配送・ページ管理の実機確認(レビュー指摘分)
calcTax(): Argument #3 ($RoundingType) must be of type int, null givenで 500 になることを再現したうえで、修正後は正常に注文手続きへ戻り、そのまま注文確定まで到達することを確認(payment_total=6660/discount=500、plg_coupon_order記録、利用枚数 100→99)app/template/default/Coupon44/...)に置いた内容がクーポン入力画面に反映されることを確認Skipped 1 件について(
OrderControllerTest::testOrderEditWithCouponCancel)受注ステータスをキャンセルへ遷移させると本体の
StockReduceProcessorが在庫戻しのためEntityManager::lock()(悲観ロック)を実行しますが、これは開いたトランザクションを要求し、DAMA DoctrineTestBundle のテスト用トランザクションではTransactionRequiredExceptionになります(本体側 4.4 のテストハーネス制約で本番の実リクエストでは発生しない)。CouponStateProcessorを無効化しても同じ例外が出ることを確認しており、本プラグイン起因ではないため当該ケースのみスキップしています。CI(PR上)でも EC-CUBE 4.4 × PHP 8.2/8.3/8.4/8.5 × MySQL8/PostgreSQL の PHPUnit と static-analysis を確認します。
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores