-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add missing changed InventoryBundle service to UPGRADE-2.0 #17595
Conversation
WalkthroughThe pull request details a comprehensive upgrade of the Sylius framework from version 2.0.2 to 2.0.3. It includes the addition of Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
UPGRADE-2.0.md (4)
1931-1932
: Fix the table formatting issue.The table in the InventoryBundle section is missing proper column alignment and has inconsistent cell counts.
-| **InventoryBundle** | -| `sylius.availability_checker` | `sylius.checker.inventory.availability` | +| **InventoryBundle** | | +| `sylius.availability_checker` | `sylius.checker.inventory.availability` |🧰 Tools
🪛 Markdownlint (0.37.0)
1931-1931: Expected: 2; Actual: 1; Too few cells, row will be missing data
Table column count(MD056, table-column-count)
Line range hint
1-24
: Critical upgrade information: Initial setup requirements.This section outlines essential initial changes needed before running the application. Pay special attention to:
- Package configuration changes in
_sylius.yaml
- Security firewall renames and user checker configurations
- Routing changes for shop and API
Ensure all these changes are applied before attempting to run the application, as they are foundational for Sylius 2.0 functionality.
🧰 Tools
🪛 Markdownlint (0.37.0)
1931-1931: Expected: 2; Actual: 1; Too few cells, row will be missing data
Table column count(MD056, table-column-count)
Line range hint
328-395
: Critical architectural change: Frontend modernization.Major frontend changes that require attention:
- Removal of jQuery in favor of SymfonyUX with Stimulus
- Transition from SemanticUI to Bootstrap
- Replacement of partial routes with Twig components
- Introduction of Sylius Twig Hooks system
Consider these steps for a smooth transition:
- Delete the
node_modules
directory and reinstall packages- Update Node.js to version 20 or 22
- Review and update any custom JavaScript code to work with Stimulus
- Update CSS classes from SemanticUI to Bootstrap equivalents
🧰 Tools
🪛 Markdownlint (0.37.0)
1931-1931: Expected: 2; Actual: 1; Too few cells, row will be missing data
Table column count(MD056, table-column-count)
Line range hint
396-677
: Critical service changes: Service renaming and reorganization.Extensive service renaming has occurred. Key points:
- Many services have been moved to more appropriate bundles
- Service visibility is now private by default
- New naming conventions have been implemented for better clarity
Action items:
- Update all service references in your custom code
- Review service visibility requirements and make necessary adjustments
- Update dependency injection configurations to use new service IDs
🧰 Tools
🪛 Markdownlint (0.37.0)
1931-1931: Expected: 2; Actual: 1; Too few cells, row will be missing data
Table column count(MD056, table-column-count)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UPGRADE-2.0.md
(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
UPGRADE-2.0.md
1931-1931: Expected: 2; Actual: 1; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
UPGRADE-2.0.md (1)
Line range hint
1-24
: Major architectural improvements that require careful migration planningThe upgrade introduces several significant architectural changes that improve maintainability and modernize the framework:
API Bundle reorganization:
- Normalizers and Denormalizers have been moved to dedicated subdirectories
- Better organization of Filters and Query extensions by context (admin/shop/common)
Frontend modernization:
- Removal of jQuery dependency
- Migration from SemanticUI to Bootstrap
- Introduction of SymfonyUX with Stimulus
Template system evolution:
- Replacement of Sonata Blocks with Twig Hooks
- More organized and maintainable template structure
- Better separation of admin and shop contexts
These changes will require careful planning for migration, particularly:
- Review all custom API extensions for compatibility with the new structure
- Update frontend code to work without jQuery
- Migrate any custom template events to the new Twig Hooks system
@@ -1929,6 +1929,7 @@ and maintainability of the application and also follows Symfony's best practices | |||
| `sylius.currency_converter` | `sylius.converter.currency` | | |||
| `sylius.currency_name_converter` | `sylius.converter.currency_name` | | |||
| **InventoryBundle** | | | |||
| `sylius.availability_checker` | `sylius.checker.inventory.availability` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical service rename that addresses the PR objective
This line documents the rename of sylius.availability_checker
to sylius.checker.inventory.availability
, which directly addresses the PR's main objective. This change explains why the service "Sylius\Bundle\ApiBundle\Serializer\ProductVariantNormalizer" was failing to find the non-existent sylius.availability_checker
service.
The service has been reorganized to follow the new naming convention:
- Old:
sylius.availability_checker
- New:
sylius.checker.inventory.availability
Make sure to update all references to this service in your codebase, particularly in the ProductVariantNormalizer and any other services that might depend on it.
Service
sylius.availability_checker
is no longer exists in sylius 2.0, but service renaming is not mentioned at UPGRADE-2.0 file.1.14
https://github.com/Sylius/SyliusInventoryBundle/blob/ee67e934c5dcb739329220f6385ee19462e67c36/DependencyInjection/SyliusInventoryExtension.php#L33
Error message is specific to my project, but nevertheless.
Summary by CodeRabbit
New Features
left
andright
sections in Twig hooks.Improvements
Deprecations
Bug Fixes