Skip to content

Refactoring cart summary controller#18783

Merged
NoResponseMate merged 1 commit intoSylius:new-routingfrom
loic425:refactoring-cart-summary
Feb 24, 2026
Merged

Refactoring cart summary controller#18783
NoResponseMate merged 1 commit intoSylius:new-routingfrom
loic425:refactoring-cart-summary

Conversation

@loic425
Copy link
Copy Markdown
Member

@loic425 loic425 commented Feb 4, 2026

Q A
Branch? 2.3
Bug fix? no
New feature? yes
BC breaks? yes (but an easy one)
Deprecations? no
Related tickets partially #18212
License MIT

Based on #18782

Summary by CodeRabbit

Release Notes

  • Documentation

    • Added upgrade guide for v2.3 route configuration changes with migration instructions for customized routes.
  • Deprecation

    • Legacy controller methods marked for removal in v3.0.
  • Chores

    • Updated route handling structure and refactored controller implementations for improved architecture.

@loic425 loic425 requested review from a team as code owners February 4, 2026 07:49
@probot-autolabeler probot-autolabeler bot added Maintenance CI configurations, READMEs, releases, etc. Shop ShopBundle related issues and PRs. labels Feb 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 4, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This 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

Cohort / File(s) Summary
Documentation & Deprecation
UPGRADE-2.3.md, src/Sylius/Bundle/CoreBundle/Controller/OrderController.php
Adds upgrade guide documenting route refactoring from 2.2 to 2.3 with migration examples. Deprecates summaryAction and thankYouAction methods in OrderController, indicating removal in Sylius 3.0.
Route Configuration
src/Sylius/Bundle/ShopBundle/Resources/config/routing/cart.yml, src/Sylius/Bundle/ShopBundle/Resources/config/routing/order.yml
Replaces legacy controller defaults mappings with direct controller service references: sylius_shop_cart_summary and sylius_shop_order_thank_you routes now use controller: entries instead of _controller in defaults.
Controller Services & Implementation
src/Sylius/Bundle/ShopBundle/Resources/config/services/controller.xml, src/Sylius/Bundle/ShopBundle/Controller/CartSummaryAction.php, src/Sylius/Bundle/ShopBundle/Controller/OrderThankYouAction.php
Adds two new service-based controller classes: CartSummaryAction (handles cart rendering with form) and OrderThankYouAction (handles order thank-you page with session-based order retrieval). Both are registered as container services with injected dependencies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • Rafikooo
  • GSadee
  • NoResponseMate

Poem

🐰 Hop, hop! Controllers align,
From defaults tangled, now they shine,
Services spring to life anew,
Cart and thanks both fresh and true,
Symfony's way, the modern brew! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Refactoring cart summary controller' accurately describes the main change—the refactoring of the cart summary controller—but omits the equally significant refactoring of the order thank you controller and related routing/service changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 4, 2026

❌ Preview Environment deleted from Bunnyshell

Available commands:

  • 🚀 /bns:deploy to redeploy the environment

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 unused Request parameter.
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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@loic425 loic425 marked this pull request as draft February 24, 2026 08:13
@loic425 loic425 force-pushed the refactoring-cart-summary branch from 2a369a0 to 2e0748d Compare February 24, 2026 11:23
@probot-autolabeler probot-autolabeler bot added the Admin AdminBundle related issues and PRs. label Feb 24, 2026
@loic425 loic425 changed the base branch from 2.3 to new-routing February 24, 2026 11:24
@loic425 loic425 marked this pull request as ready for review February 24, 2026 11:24
@NoResponseMate NoResponseMate merged commit 17c6029 into Sylius:new-routing Feb 24, 2026
30 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Admin AdminBundle related issues and PRs. Maintenance CI configurations, READMEs, releases, etc. Shop ShopBundle related issues and PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants