Refactoring cart summary controller#18783
Conversation
|
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 You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request refactors the shop's cart summary and order thank-you routes, migrating from legacy controller defaults mappings to modern Symfony service-based controllers. Two new controller action classes are introduced with corresponding service declarations, and the old controller methods are marked as deprecated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
❌ Preview Environment deleted from BunnyshellAvailable commands:
|
15c5251 to
2a369a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/ShopBundle/Controller/CartSummaryController.php`:
- Around line 46-47: The form created in CartSummaryController uses
$this->formFactory->createNamed('', $formType ?? CartType::class) but isn’t
bound to the current cart, so bind the cart as the third argument: pass $cart to
createNamed() (i.e., $this->formFactory->createNamed('', $formType ??
CartType::class, $cart)) so the form can access and submit cart data correctly.
In `@UPGRADE-2.3.md`:
- Around line 64-66: The example override sets the sylius_shop_cart_summary
route path incorrectly to "/thank-you"; update the route declaration for
sylius_shop_cart_summary so its path remains "/" (i.e., change path: /thank-you
to path: /) to reflect the correct cart summary route in the documentation.
🧹 Nitpick comments (1)
src/Sylius/Bundle/ShopBundle/Controller/CartSummaryController.php (1)
38-38: Remove or use the unusedRequestparameter.
Static analysis already flags this; consider dropping it if not needed.🧹 Possible cleanup
- public function __invoke(Request $request, ?string $formType = null, ?string $template = null): Response + public function __invoke(?string $formType = null, ?string $template = null): Response
src/Sylius/Bundle/ShopBundle/Controller/CartSummaryController.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Sylius/Bundle/ShopBundle/Controller/CartSummaryAction.php`:
- Line 46: The form is created with $this->formFactory->createNamed('',
$formType ?? CartType::class) but no data, so the form isn't bound to the
current cart; modify the call to pass the cart as the data argument so the form
is built with the actual cart contents (use $cart as the second/third parameter
depending on overload), i.e. update the createNamed invocation in
CartSummaryAction to include $cart when creating the form (reference:
createNamed, $formFactory, CartType, $cart).
In `@src/Sylius/Bundle/ShopBundle/Resources/config/routing/order.yml`:
- Around line 4-9: The route definition sylius_shop_order_thank_you contains an
empty defaults: key which YAML parses as null and causes a Symfony route-loading
TypeError; fix it by either removing the empty defaults: line entirely or
initializing it as an empty mapping (defaults: {}) in the
sylius_shop_order_thank_you route definition so Symfony receives an array of
defaults.
2a369a0 to
2e0748d
Compare
Based on #18782
Summary by CodeRabbit
Release Notes
Documentation
Deprecation
Chores