ec-cube
ec-cube copied to clipboard
EC-CUBE
PurchaseFlowのProcessorやValidatorにpriorityを設定できるようにする
本体のPurchaseFlowの各プロセッサーの前後にカスタマイズしたプロセッサーを差し込ませられるよう、 PurchaseFlowのProcessorやValidatorにpriorityを設定できるようにしました。
概要(Overview・Refs Issue)
実装した各プロセッサーを以下のようにservices.yamlに定義するだけで設定できます。
services:
Eccube\Service\PurchaseFlow\Processor\ProductStatusValidator: # 商品の公開状態のチェック
tags:
- { name: eccube.item.validator, flow_type: cart, priority: 90 }
- { name: eccube.item.validator, flow_type: shopping, priority: 90 }
- { name: eccube.item.validator, flow_type: order, priority: 90 }
タグのnameはPurchaseFlowPassに設定されているものを指定してください。
flow_typeは必須でcart、shopping、orderいずれかを設定してください。 cartを指定するとカートページ、shoppingを指定するろ購入手続きページ、orderを指定すると受注管理ページで実行されます。
priorityは設定しなかったらおそらく0が付与されます。数値が大きいほど、タグ付けされたサービスがコレクション内で早く配置されます。
方針(Policy)
実装に関する補足(Appendix)
テスト（Test)
相談（Discussion）
マイナーバージョン互換性保持のための制限事項チェックリスト
- [ ] 既存機能の仕様変更
- [ ] フックポイントの呼び出しタイミングの変更
- [ ] フックポイントのパラメータの削除・データ型の変更
- [ ] twigファイルに渡しているパラメータの削除・データ型の変更
- [ ] Serviceクラスの公開関数の、引数の削除・データ型の変更
- [ ] 入出力ファイル(CSVなど)のフォーマット変更
レビュワー確認項目
- [ ] 動作確認
- [ ] コードレビュー
- [ ] E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
- [ ] 互換性が保持されているか
- [ ] セキュリティ上の問題がないか
Codecov Report
Attention: Patch coverage is 65.78947% with 13 lines in your changes are missing coverage. Please review.
Project coverage is 82.41%. Comparing base (
0536832) to head (8e91775). Report is 47 commits behind head on 4.3.
:exclamation: Current head 8e91775 differs from pull request most recent head d55f3b0. Consider uploading reports for the commit d55f3b0 to get more accurate results
| Files | Patch % | Lines |
|---|---|---|
| .../DependencyInjection/Compiler/PurchaseFlowPass.php | 65.51% | 10 Missing :warning: |
| src/Eccube/Service/PurchaseFlow/PurchaseFlow.php | 66.66% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## 4.3 #5147 +/- ##
============================================
+ Coverage 81.87% 82.41% +0.53%
============================================
Files 480 480
Lines 26334 26358 +24
============================================
+ Hits 21562 21722 +160
+ Misses 4772 4636 -136
| Flag | Coverage Δ | |
|---|---|---|
| E2E | 82.41% <65.78%> (+13.12%) |
:arrow_up: |
| Unit | 82.41% <65.78%> (+3.65%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
開発している一部のプラグインが動かなくなるので、 本体に設定されているProcessorやValidatorをカスタマイズやプラグインで取り除けるような仕組みを考えます。
@dotani1111
プラグインのservices.yamlで本体のPurchaseFlowの定義を上書きしているプラグインは影響があると思います。
@ji-eunsoo
「商品オプションプラグイン」とか「まとめ買い価格設定プラグイン」とか価格を操作しているプラグインは PurchaseFlowのプロセッサーの定義を上書きしたり、オリジナルのプロセッサーをカスタマイズして挿入していると思います。
@chihiro-adachi
アノテーションではプライオリティを指定できません。 アノテーションでプライオリティを設定できるよう実装できるかも今のところ調査できてません。
今回の実装の流れとしては、タグ付けされたプロセッサーをプライオリティ順に登録。 その後、アノテーションで設定されたプロセッサーをプロセッサー名のABC順で登録されるかと思います。
@kurozumi
アノテーション指定とyaml指定が重複した場合は二重に登録されます。
ご相談なのですが、重複指定した場合はyamlが優先されるようにできないでしょうか？ 4.2/4.3でコードが共存できればありがたいです。
@chihiro-adachi
ご相談なのですが、重複指定した場合はyamlが優先されるようにできないでしょうか？
yamlでプロセッサーが指定されていたらアノテーションのプロセッサーは登録されないようにしました。
@kurozumi ご対応ありがとうございます。 yamlとアノテーションを同時に設定したところ、どちらも実行されてしまいました。 お手数ですが、こちらご確認いただけますか？
@kurozumi ありがとうございます。 実施した手順は以下になります。
-
Customize以下にサンプルバリデータを作成 app/Customize/Service/PurchaseFlow/Processor/SampleValidator.php
-
サンプルバリデータに@CartFlowを付与
-
purchaseflow.yamlに追加
eccube.all.flow.purchase.processor.Customize.Service.PurchaseFlow.Processor:
class: Customize\Service\PurchaseFlow\Processor\SampleValidator
tags:
- { name: eccube.item.validator, flow_type: cart, priority: 10 }
こちらの状態で、カートに遷移して、PurchaseFlow::dump()を実行すると以下のようになりました。 一番下に追加されているのが、アノテーションの方になります。
├ cart flow
ItemValidator
│├ Eccube\Service\PurchaseFlow\Processor\DeliverySettingValidator
│├ Eccube\Service\PurchaseFlow\Processor\ProductStatusValidator
│├ Eccube\Service\PurchaseFlow\Processor\PriceChangeValidator
│├ Eccube\Service\PurchaseFlow\Processor\StockValidator
│├ Eccube\Service\PurchaseFlow\Processor\SaleLimitValidator
│├ Eccube\Service\PurchaseFlow\Processor\ClassCategoryValidator
│├ Customize\Service\PurchaseFlow\Processor\SampleValidator
│└ Customize\Service\PurchaseFlow\Processor\SampleValidator
@dotani1111
eccube.all.flow.purchase.processor.Customize.Service.PurchaseFlow.Processor:
class: Customize\Service\PurchaseFlow\Processor\SampleValidator
tags:
- { name: eccube.item.validator, flow_type: cart, priority: 10 }
プロセッサーの登録チェックは各プロセッサーの id でチェックしているのですが上記の書き方だと purchaseflow.yamlは eccube.all.flow.purchase.processor.Customize.Service.PurchaseFlow.Processor が id として登録され、 アノテーションは Customize\Service\PurchaseFlow\Processor\SampleValidator が id として登録されます。
異なる id なので purchase.yaml とアノテーションの両方が登録されていました。
なので、id でチェックせずクラス名でチェックするよう修正しました。
いつも以下の書き方をしていたので気が付きませんでした。。
Customize\Service\PurchaseFlow\Processor\SampleValidator:
tags:
- { name: eccube.item.validator, flow_type: cart, priority: 10 }
@kurozumi ご対応ありがとうございます！
purchaseflow.yamlは eccube.all.flow.purchase.processor.Customize.Service.PurchaseFlow.Processor が id として登録され、 アノテーションは Customize\Service\PurchaseFlow\Processor\SampleValidator が id として登録されます。
勉強になります！ありがとうございます🙏 動作確認して、修正を確認致しました。
最後に、最新コミットの取り込みだけ、よろしくお願いします。
@dotani1111
一点懸念があります。
今回、purchaseflow.yaml に設定している priority の最大値を100にして 各処理毎に priority を10減らしていますが、この幅が正解なのかは正直はわかっていません。
本体の priority の値を一度設定してしまうと、その後なにかの事情で priority の値を変更した場合に大きな影響が発生するかもしれません。
なので、priority の幅は慎重に決定したほうが良いと思うのですがいかがでしょうか？
@kurozumi 差し込みたい場所のパターンは大体決まってたりしますか？ これまで経験されたユースケースを教えていただけると判断しやすいかもしれません。
@chihiro-adachi
差し込みたいパターンもユースケースは今のところないのですが、他のプラグインのプロセッサーより早く実行したいといった場合にpriorityを設定できる幅が広いほうが良いのかなと思いました。
今のところ特に困り事はないです。
@kurozumi ありがとうございます。 今後の拡張性を考えて、1000始まりの100刻みに変更させて頂きます。 また最新コミットへの追従も行います。
@kurozumi テストが落ちてしまっているのですが、こちら解消できますでしょうか？ ローカルでも購入フローに遷移する際に、エラーが発生してしまいました。。
@dotani1111
テストが落ちていてローカルでもエラーが発生しているのは確認済みでただいま原因調査しております。 改善しますので少々お待ち下さい。