feat: 注文手続きで配送方法・支払い方法の保存と復元を追加 (#6819)#6833
Open
ttokoro20240902 wants to merge 10 commits into
Open
Conversation
dtb_customer に preferred_payment_id / preferred_delivery_id を追加し、 ON DELETE SET NULL で参照先削除時は自動的に未設定へ戻す。 スキーマは Entity 属性から schema:update で反映するためマイグレーションは作成しない。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- ShoppingController に検証共通メソッドと POST /shopping/restore_preferred を追加 - 注文確定時にチェックボックス ON で会員へ保存（失敗しても注文は継続） - OrderType に save_preferred_shipping_payment を会員かつ単一配送先のみ動的追加 - 注文手続き画面に情報ボックス・復元ボタン・警告、確認画面にチェックボックス、 完了画面に保存失敗メッセージを追加 - front.shopping.preferred_* の翻訳キーを追加 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- validatePreferredShippingPayment の単体テスト（検証順・販売種別・組み合わせ） - Customer の優先設定アクセサ・永続化・ON DELETE SET NULL のテスト - 表示/復元/保存チェックボックス/確定時保存/ゲスト 403 の機能テスト Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
単一配送先のときだけ動的追加する方式では, 複数配送先の注文確定時に save_preferred_shipping_payment を含む細工した POST が extra fields エラーとなり 注文自体が失敗する。フィールドは会員なら常に定義し, 表示はテンプレートで 単一配送先のみに制御, 複数配送先時の送信値は確定処理でスキップする。 複数配送先の機能テスト（情報ボックスのみ表示・復元スキップ・確定時保存スキップ）を追加。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
保存（チェックボックスONで注文）→ 情報ボックス表示 → 手動復元のジャーニーを front-order.spec.ts の serial describe として追加（既存ヘルパーを再利用）。 CI での実行は front-order suite の matrix 登録（#6829）に依存する。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
e2e-test.yml の matrix は許可リストであり, 列挙されていない spec は CI で実行されない。 front-order(16件) / front-mypage(10件+skip 1) / front-invoice(2件) は テストカバレッジ向上で追加された際から matrix 未登録のままで, 一度も CI で実行されていなかった。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCustomer に優先配送・優先支払いを持たせ、注文フローで保存・検証・手動復元できるようにし、テンプレート文言・完了画面の表示、成功アラートスタイル、単体／Web／E2E テスト、CI スイート登録を追加しています。 Changes配送方法・支払い方法の保存と復元機能
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Eccube/Controller/ShoppingController.php (1)
439-444:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
confirm()のエラー戻りで保存済み設定の文脈が落ちています。ここは
Shopping/index.twigを再表示しているのに、index()/redirectTo()で渡しているpreferredPaymentId/preferredDeliveryId/preferredUnavailableReason/isMultipleShippingが返却値に含まれていません。 この分岐だけ保存済み設定の表示・警告が消えるか、テンプレート側が直接参照していれば未定義変数になります。差分案
return [ 'form' => $form->createView(), 'Order' => $Order, 'activeTradeLaws' => $activeTradeLaws, 'Prefs' => $this->prefRepository->findAll(), + 'preferredPaymentId' => $preferredInfo['preferredPaymentId'], + 'preferredPaymentName' => $preferredInfo['preferredPaymentName'], + 'preferredDeliveryId' => $preferredInfo['preferredDeliveryId'], + 'preferredDeliveryName' => $preferredInfo['preferredDeliveryName'], + 'isMultipleShipping' => $preferredInfo['isMultipleShipping'], + 'preferredUnavailableReason' => $preferredInfo['preferredUnavailableReason'], ];Based on PR objectives,
index・redirectTo・confirmから同じ検証結果をテンプレートへ返す前提です.🤖 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/Controller/ShoppingController.php` around lines 439 - 444, The confirm() error-return branch is missing the saved-preference context keys that index() and redirectTo() supply, causing the template to lose preferredPaymentId, preferredDeliveryId, preferredUnavailableReason and isMultipleShipping; update the return array in ShoppingController::confirm() to include those same keys (pulling values from the same sources used by index()/redirectTo(), e.g., the request/session or the Order object) so the template receives identical validation/context data.
🤖 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 `@src/Eccube/Controller/ShoppingController.php`:
- Around line 1064-1069: The loop treating $allowedPayments as Payment objects
is wrong because PaymentRepository::findAllowedPayments() returns rows as arrays
(e.g. ['id' => ...]); update the foreach over $allowedPayments (in
ShoppingController where $allowedPayments is populated from
$this->paymentRepository->findAllowedPayments) to handle both shapes: if the
item is an object call ->isVisible() and ->getId(), otherwise treat it as an
array and read the id and visibility keys (e.g. $row['id'] and $row['visible']
or fallback $row['is_visible']) before pushing into $allowedPaymentIds. Ensure
the branch covers both cases so existing tests that return mocks still work and
real repository rows don’t cause fatal errors.
In `@src/Eccube/Resource/template/default/Shopping/alert.twig`:
- Around line 19-30: 現在のループで `eccube.front.success` のフラッシュメッセージが
`ec-cartRole__error` /
`ec-alert-warning`（エラー／警告用クラス）で包まれているため、成功メッセージ用に見た目を分離してください：`eccube.front.success`
を出力している該当テンプレート内のループ（現在の for-block）を見つけ、ラッパーのクラス名をエラー/警告用から成功専用（例:
`ec-cartRole__success` と `ec-alert-success`
などプロジェクトの命名規則に沿った名前）に差し替え、既存の翻訳/改行処理（`{{ success|trans|nl2br }}`）はそのまま残すことと、対応する
CSS スタイルを追加/調整して成功メッセージが警告と異なる外観になるようにしてください。
In `@src/Eccube/Resource/template/default/Shopping/index.twig`:
- Around line 334-335: The template adds a duplicate CSRF hidden input (name
constant('Eccube\\Common\\Constant::TOKEN_NAME')) inside the same form that
already contains form._token, which can overwrite the token when submitting to
shopping_confirm; fix this by moving the restore button (the <button ...
formaction="{{ url('shopping_restore_preferred') }}" ...> with its hidden token
input) into its own separate <form> so it has its own CSRF token and does not
interfere with the main shopping_confirm form, ensuring the original form keeps
only its existing form._token and the new restore form renders its own
csrf_token(constant('Eccube\\Common\\Constant::TOKEN_NAME')).
---
Outside diff comments:
In `@src/Eccube/Controller/ShoppingController.php`:
- Around line 439-444: The confirm() error-return branch is missing the
saved-preference context keys that index() and redirectTo() supply, causing the
template to lose preferredPaymentId, preferredDeliveryId,
preferredUnavailableReason and isMultipleShipping; update the return array in
ShoppingController::confirm() to include those same keys (pulling values from
the same sources used by index()/redirectTo(), e.g., the request/session or the
Order object) so the template receives identical validation/context data.
🪄 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: 4dbb763b-7fad-4d54-a153-c7837e4daaef
📒 Files selected for processing (14)
.github/workflows/e2e-test.ymle2e/tests/front-order.spec.tssrc/Eccube/Controller/ShoppingController.phpsrc/Eccube/Entity/Customer.phpsrc/Eccube/Form/Type/Shopping/OrderType.phpsrc/Eccube/Resource/locale/messages.en.yamlsrc/Eccube/Resource/locale/messages.ja.yamlsrc/Eccube/Resource/template/default/Shopping/alert.twigsrc/Eccube/Resource/template/default/Shopping/complete.twigsrc/Eccube/Resource/template/default/Shopping/confirm.twigsrc/Eccube/Resource/template/default/Shopping/index.twigtests/Eccube/Tests/Controller/ShoppingControllerValidatePreferredShippingPaymentTest.phptests/Eccube/Tests/Entity/CustomerTest.phptests/Eccube/Tests/Web/ShoppingControllerPreferredShippingPaymentTest.php
受入基準のうち機能テストで埋めにくかった 2 点を補完する。 - savePreferredShippingPayment の単体テスト: flush 例外を飲み込み注文を継続し、 注文完了画面用フラッシュ(preferred_save_error)を積むこと。複数配送先・OFF の 保存スキップも単体で固定。EntityManager をモックして例外を注入する。 - 注文手続き画面の初期表示で、保存情報が利用不可のとき警告を表示し復元ボタンを 出さないこと(index テンプレートの preferredUnavailableReason 描画経路)。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/Eccube/Tests/Controller/ShoppingControllerSavePreferredShippingPaymentTest.php (1)
73-75:ReflectionClassの対象をモックではなくShoppingController::classに固定してください（テストの安定性向上）
- 本テストでは
ReflectionClass($this->controller)でentityManager/sessionを差し替えていますが、これらはAbstractController側のprotectedプロパティです。- 一方で
savePreferredShippingPayment()はShoppingController側でprivateなので、反射対象はShoppingController::classに固定して PHPUnit モック生成物への依存を減らすのが安全です（73-75、183-185）。修正例
- $reflection = new \ReflectionClass($this->controller); + $reflection = new \ReflectionClass(ShoppingController::class); $reflection->getProperty('entityManager')->setValue($this->controller, $this->entityManager); $reflection->getProperty('session')->setValue($this->controller, $session); @@ - $reflection = new \ReflectionClass($this->controller); + $reflection = new \ReflectionClass(ShoppingController::class); $method = $reflection->getMethod('savePreferredShippingPayment'); $method->invoke($this->controller, $Order, $form);🤖 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/Eccube/Tests/Controller/ShoppingControllerSavePreferredShippingPaymentTest.php` around lines 73 - 75, Replace ReflectionClass($this->controller) with ReflectionClass(ShoppingController::class) when setting protected properties so the reflection targets the concrete ShoppingController class instead of the PHPUnit mock; update both occurrences that set entityManager and session (the code that prepares for calling the private savePreferredShippingPayment method) to use ShoppingController::class to avoid coupling tests to the mock instance and ensure stable access to the protected/private members.
🤖 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
`@tests/Eccube/Tests/Controller/ShoppingControllerSavePreferredShippingPaymentTest.php`:
- Around line 1-14: The EC-CUBE license header is placed after the
declare(strict_types=1); directive; move the entire license comment block so it
appears immediately after the opening <?php tag and before
declare(strict_types=1); to comply with the PHP file header guideline; locate
the current license comment and the declare(strict_types=1); statement in
Tests/Controller/ShoppingControllerSavePreferredShippingPaymentTest.php and
reorder them so the license block precedes the declare line.
---
Nitpick comments:
In
`@tests/Eccube/Tests/Controller/ShoppingControllerSavePreferredShippingPaymentTest.php`:
- Around line 73-75: Replace ReflectionClass($this->controller) with
ReflectionClass(ShoppingController::class) when setting protected properties so
the reflection targets the concrete ShoppingController class instead of the
PHPUnit mock; update both occurrences that set entityManager and session (the
code that prepares for calling the private savePreferredShippingPayment method)
to use ShoppingController::class to avoid coupling tests to the mock instance
and ensure stable access to the protected/private members.
🪄 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: 19c367ee-3a25-41b4-a483-a2ad6771cc17
📒 Files selected for processing (2)
tests/Eccube/Tests/Controller/ShoppingControllerSavePreferredShippingPaymentTest.phptests/Eccube/Tests/Web/ShoppingControllerPreferredShippingPaymentTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Eccube/Tests/Web/ShoppingControllerPreferredShippingPaymentTest.php
CI の rector が検出したテストコードの code-quality 指摘を適用。 createStub への置換・assertInstanceOf 化・(string) キャスト等。挙動変更なし。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CodeRabbit 指摘対応: - 成功フラッシュ(eccube.front.success)を ec-alert-warning(警告) から ec-alert-success へ分離。外側も ec-cartRole__success に改名し意味を一致 - _15.1.cart.scss に .ec-alert-success(緑背景) を追加 CI(sqlite) 失敗対応: - CustomerTest::testPreferredResetToNullWhenReferenceDeleted は 外部キー ON DELETE SET NULL を検証するが, sqlite は FK を既定で 強制しないため markTestSkipped(pgsql/mysql で担保) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@html/template/default/assets/scss/project/_15.1.cart.scss`:
- Line 452: The success alert component has insufficient text contrast between
the background color `#5cb85c` (line 452) and the text color `#fff` (line 469),
which fails accessibility requirements for regular text. To fix this, either
darken the background color value at line 452 to increase contrast with the
white text, or adjust the text color at line 469 to a darker shade that provides
better contrast against the green background. Ensure the final combination meets
WCAG AA contrast ratio requirements for normal text.
🪄 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: 8278db48-2ab3-40d2-b91a-165ea453871e
📒 Files selected for processing (3)
html/template/default/assets/scss/project/_15.1.cart.scsssrc/Eccube/Resource/template/default/Shopping/alert.twigtests/Eccube/Tests/Entity/CustomerTest.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Eccube/Resource/template/default/Shopping/alert.twig
- tests/Eccube/Tests/Entity/CustomerTest.php
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6833 +/- ##
============================================
- Coverage 75.33% 74.89% -0.45%
============================================
Files 483 463 -20
Lines 26277 24185 -2092
============================================
- Hits 19797 18114 -1683
+ Misses 6480 6071 -409
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:
|
保存設定の復元(restorePreferred)で配送方法を差し替える際, 旧配送業者に属するお届け時間(time_id / shipping_delivery_time)が Shipping に残り, 新しい配送業者と不整合になっていた。通常フロー(ShippingType)と同様にクリアする。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Resolves #6819
概要
会員が注文手続きで使用した配送方法・支払い方法を保存し、次回以降の注文時に**「この設定を使用する」ボタンで手動復元**できる機能を本体に追加します。復元は自動適用せず、ユーザーが明示的にボタンを押したときのみ Order/Shipping に保存値を適用して合計金額を再計算します。保存は会員 1 人につき 1 組（上書き）です。
/shopping）/ 注文確認（/shopping/confirm）/ 注文完了（/shopping/complete）スクリーンショット
① 注文手続き画面: 保存された設定の情報ボックス（
/shopping）保存済みの会員に表示される「保存された設定があります」ボックスと「この設定を使用する」ボタン。「お客様情報」と「配送情報」の間に配置されることが分かります。
② 注文手続き画面: 手動復元の成功メッセージ（
/shopping）「この設定を使用する」を押した後の success フラッシュ「保存された設定を適用しました」。ステップナビ直下に成功用スタイル（緑）で表示されます。
③ 注文手続き画面: 保存設定が利用不可のときの警告（
/shopping）保存された支払い方法が非公開等で利用できない場合、情報ボックスの代わりに警告「保存された支払い方法が現在利用できません」を同じ位置に表示します。
④ 注文確認画面: 保存チェックボックス（
/shopping/confirm）合計ボックス内（合計金額と「注文する」ボタンの間）の「この配送方法と支払い方法を保存する」チェックボックス。
実装内容
CustomerにPreferredPayment/PreferredDelivery（ManyToOne,onDelete: SET NULL）を追加validatePreferredShippingPayment()（表示・復元で共用の検証）、POST /shopping/restore_preferred（会員のみ・CSRF 必須）、checkoutの保存処理（失敗しても注文は継続）、confirmのisMultipleShipping受け渡しOrderTypeにsave_preferred_shipping_payment（会員時に常時定義・表示はテンプレートで単一配送先のみ）index（情報ボックス・復元ボタン・警告）/confirm（保存チェックボックス）/complete（保存失敗メッセージ）/alert（success フラッシュ表示）front.shopping.preferred_*を ja / en に追加設計上の判断
doctrine:schema:create/schema:update --forceが反映します（本体のスキーマ管理規約）。formactionでPOST /shopping/restore_preferred）。注文手続き画面は画面全体が 1 つの<form>のためフォームのネストは不可。クライアント JS で配送セレクト・支払ラジオを書き換える方式は、選択中の配送方法によっては保存支払方法のラジオが未描画で復元しきれない／非公開・組み合わせ・販売種別の検証をサーバーで担保できない／機能テストで担保できない、ため採用しません。DeliveryRepository::getDeliveries()/PaymentRepository::findAllowedPayments()を利用し、独自の重複ロジックを作りません。テスト
validatePreferredShippingPayment14 件（検証順・販売種別・組み合わせ）ON DELETE SET NULL3 件e2e/tests/front-order.spec.tsに追加（受注一覧 表示項目設定ではなくFront Order 配送方法・支払い方法の保存と復元describe）E2E の matrix 登録について
本 PR の E2E は
front-order.spec.tsに統合しています。front-ordersuite は従来e2e-test.ymlの matrix に未登録で CI 実行されていなかったため、#6829 と同一の matrix 登録コミットを cherry-pick で含めています（front-order / front-mypage / front-invoice）。#6829 が先にマージされた場合も同一内容のため Git が吸収します。🤖 Generated with Claude Code
Summary by CodeRabbit
Summary