Conversation
…ndle\Converter\IriToIdentifierConverter::isIdentifier($fieldValue)` (fix after -> symfony/http-foundation@168b77c71)
…ted to url validation
This PR has been generated automatically. For more details see [upmerge_pr.yaml](/Sylius/Sylius/blob/2.1/.github/workflows/upmerge_pr.yaml). **Remember!** The upmerge should always be merged with using `Merge pull request` button. In case of conflicts, please resolve them manually with usign the following commands: ``` git fetch upstream gh pr checkout <this-pr-number> git merge upstream/2.0-docs -m "Resolve conflicts between 2.0 and 2.0-docs" ``` If you use other name for the upstream remote, please replace `upstream` with the name of your remote pointing to the `Sylius/Sylius` repository. Once the conflicts are resolved, please run `git merge --continue` and push the changes to this PR.
| Q | A |-----------------|----- | Branch? | 1.14 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | fixes failing tests | License | MIT <!-- - Bug fixes must be submitted against the 1.14 or 2.0 branch - Features and deprecations must be submitted against the 2.1 branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html -->
This PR has been generated automatically. For more details see [upmerge_pr.yaml](/Sylius/Sylius/blob/2.1/.github/workflows/upmerge_pr.yaml). **Remember!** The upmerge should always be merged with using `Merge pull request` button. In case of conflicts, please resolve them manually with usign the following commands: ``` git fetch upstream gh pr checkout <this-pr-number> git merge upstream/2.0-docs -m "Resolve conflicts between 2.0 and 2.0-docs" ``` If you use other name for the upstream remote, please replace `upstream` with the name of your remote pointing to the `Sylius/Sylius` repository. Once the conflicts are resolved, please run `git merge --continue` and push the changes to this PR.
This PR has been generated automatically. For more details see [upmerge_pr.yaml](/Sylius/Sylius/blob/2.1/.github/workflows/upmerge_pr.yaml). **Remember!** The upmerge should always be merged with using `Merge pull request` button. In case of conflicts, please resolve them manually with usign the following commands: ``` git fetch upstream gh pr checkout <this-pr-number> git merge upstream/2.0 -m "Resolve conflicts between 1.14 and 2.0" ``` If you use other name for the upstream remote, please replace `upstream` with the name of your remote pointing to the `Sylius/Sylius` repository. Once the conflicts are resolved, please run `git merge --continue` and push the changes to this PR.
This PR has been generated automatically. For more details see [upmerge_pr.yaml](/Sylius/Sylius/blob/2.1/.github/workflows/upmerge_pr.yaml). **Remember!** The upmerge should always be merged with using `Merge pull request` button. In case of conflicts, please resolve them manually with usign the following commands: ``` git fetch upstream gh pr checkout <this-pr-number> git merge upstream/2.0-docs -m "Resolve conflicts between 2.0 and 2.0-docs" ``` If you use other name for the upstream remote, please replace `upstream` with the name of your remote pointing to the `Sylius/Sylius` repository. Once the conflicts are resolved, please run `git merge --continue` and push the changes to this PR.
| Q | A |-----------------|----- | Branch? | 2.0 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | mentioned in Sylius/Stack#259 | License | MIT More details on related ticket. <!-- - Bug fixes must be submitted against the 1.14 or 2.0 branch - Features and deprecations must be submitted against the 2.1 branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html -->
| Q | A |-----------------|----- | Branch? | 1.14 <!-- see the comment below --> | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no <!-- don't forget to update the UPGRADE-*.md file --> | Related tickets | - | License | MIT <!-- - Bug fixes must be submitted against the 1.14 or 2.0 branch - Features and deprecations must be submitted against the 2.1 branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html --> All requests from `/_profiler` and `/_wdt` are currently part of `shop` firewall instead of `dev`. It's because the `dev` firewall is defined in last position and `/_profiler` and `_wdt` are not excluded from `%sylius.security.shop_regex%`. Defining it first fixes this issue. However, existing projects should also change their config, should I target 2.1 branch instead and add this change in the `UPGRADE-2.1.md` ?
Added support for displaying Sylius Plus logo in the admin navbar via menu. Included an example usage in the Administration section of the main menu. 
Added support for displaying Sylius Plus logo in the admin navbar via menu. Included an example usage in the Administration section of the main menu.     <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced the main navigation by adding an Administration section that offers new access options for administration-related content, including links to roles information. - **Style** - Introduced a new badge element in the navigation and refined dropdown layouts, improving visual consistency and usability through enhanced logo and icon displays. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Updated plus utm link <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced an "Administration" submenu in the admin sidebar, featuring a "roles" link with a Sylius Plus badge that opens in a new tab. - Added a new UI segment on the admin user form showcasing role selection and a permission checker toggle, highlighting Sylius Plus features. - Enhanced sidebar visuals with a Sylius Plus badge and new hover effects. - **Style** - Updated admin panel styles to support new sidebar badge and interactive blur effects. - **Bug Fixes** - Improved navigation reliability after viewing product images in admin and shop interfaces. - **Documentation** - Added new translation entries for roles and permission checker UI elements. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Updated plus utm link Fixes #17827
**Fix SVG margin issue causing frame overlap by adjusting line-height for btn-sm elements** The problem was that the margin on the SVG compressed each side by 4cm, making the frame too small and causing it to overlap the image. I chose to fix it by increasing the `line-height`, which provides enough space for elements using `btn-sm`. 
This PR has been generated automatically. For more details see [upmerge_pr.yaml](/Sylius/Sylius/blob/2.1/.github/workflows/upmerge_pr.yaml). **Remember!** The upmerge should always be merged with using `Merge pull request` button. In case of conflicts, please resolve them manually with usign the following commands: ``` git fetch upstream gh pr checkout <this-pr-number> git merge upstream/2.0 -m "Resolve conflicts between 1.14 and 2.0" ``` If you use other name for the upstream remote, please replace `upstream` with the name of your remote pointing to the `Sylius/Sylius` repository. Once the conflicts are resolved, please run `git merge --continue` and push the changes to this PR.
resolves #17727 The problem occurs in all API commands that implement `Sylius\Bundle\ApiBundle\Command\IriToIdentifierConversionAwareInterface`. | Q | A |-----------------|----- | Branch? | 1.13, 1.14 and 2.0 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | fixes #17727 | License | MIT
| Q | A |-----------------|----- | Branch? | 2.0 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Related tickets | n/a | License | MIT <!-- - Bug fixes must be submitted against the 1.14 or 2.0 branch - Features and deprecations must be submitted against the 2.1 branch - Make sure that the correct base branch is set To be sure you are not breaking any Backward Compatibilities, check the documentation: https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html --> Changing Add to card component event scope. Previously we had `emitUp` - only to parent. Now it is `emit` globally
This PR has been generated automatically. For more details see [refactor.yaml](/Sylius/Sylius/blob/2.1/.github/workflows/refactor.yaml).
Automated changes by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
WalkthroughThis update introduces an "Administration" submenu with a "roles" item in the admin main menu, adds a new roles section to the admin user form, and enhances UI elements with new styles and images. Documentation is expanded with a comprehensive guide on live forms for custom models, and translation files receive new role-related entries. Several templates and asset files are updated to support these features. Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUser as Admin User
participant AdminMenu as MainMenuBuilder
participant UI as Admin UI
participant SyliusPlus as Sylius Plus Site
AdminUser->>AdminMenu: Opens admin sidebar
AdminMenu->>UI: Renders "Administration" submenu with "roles" item
AdminUser->>UI: Clicks "roles" menu item
UI->>SyliusPlus: Opens Sylius Plus roles URL in new tab
sequenceDiagram
participant AdminUser as Admin User
participant UI as Admin User Form
participant RolesSection as Roles Section (Twig)
participant Translations as Translation File
AdminUser->>UI: Opens admin user create/update form
UI->>RolesSection: Renders roles card with role options and permission checker
RolesSection->>Translations: Retrieves translated role names and labels
AdminUser->>RolesSection: Interacts with role selection and permission checker
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/Sylius/Bundle/AdminBundle/Resources/assets/entrypoint.jsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@eslint/js' imported from /eslint.config.mjs ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Actionable comments posted: 5
🧹 Nitpick comments (6)
docs/the-customization-guide/customizing-templates.md (1)
236-237: Refine grammar for clarity
In the sentence "There're two way's of finding out the hook name…", consider replacing with "There are two ways of finding out the hook name…" to avoid uncommon contraction and misspelling.🧰 Tools
🪛 LanguageTool
[style] ~236-~236: The contraction ‘There’re’ is uncommon in written English.
There're two way's of finding out the hook name ...
Context: ...="">(THERE_RE_CONTRACTION_UNCOMMON)
[misspelling] ~237-~237: This word is normally spelled with a hyphen.
Context: ...aption> There're two way's of finding out the hook name you'd li...(EN_COMPOUNDS_TWO_WAY)
src/Sylius/Bundle/AdminBundle/templates/admin_user/form/sections/roles.html.twig (1)
1-4: Consider adding a title attribute for better accessibilityThe clickable div that opens a new window should have a title attribute to describe where the link leads, improving accessibility and user experience.
-<div class="card mb-3 plus-blur" onclick="window.open('https://sylius.com/plus/?utm_source=product&utm_medium=placeholder&utm_campaign=rbac-placeholder', '_blank')" style="cursor: pointer;"> +<div class="card mb-3 plus-blur" onclick="window.open('https://sylius.com/plus/?utm_source=product&utm_medium=placeholder&utm_campaign=rbac-placeholder', '_blank')" style="cursor: pointer;" title="Learn more about Sylius Plus roles and permissions">src/Sylius/Bundle/AdminBundle/Resources/assets/styles/_plus-features.scss (1)
1-41: LGTM with optimization opportunitiesThe blur effect implementation is functional but has duplicate code. The same blur filter and pointer-events rules are defined twice - once in
.blur-content:hoverand again in.plus-blur:hover .blur-content. Consider consolidating for DRY code..plus-blur { position: relative; overflow: hidden; .blur-content { transition: filter 0.3s ease; - - &:hover { - filter: blur(2px); - - * { - pointer-events: none; - } - } } .overlay-image { position: absolute; top: 50%; left: 50%; transform: translate(-50%, -50%); opacity: 0; z-index: 10; transition: opacity 0.3s ease; pointer-events: none; } &:hover { .overlay-image { opacity: 1; } .blur-content { filter: blur(2px); * { pointer-events: none; } } } }docs/the-customization-guide/customizing-forms/how-to-add-a-live-form-for-a-custom-model.md (3)
46-60: Clarify component identifier patternThe comment suggests following a convention for the service ID (
app_admin.twig.component.supplier.form), but then the tag's key uses a different format with colons (app_admin:twig:component:supplier:form). This might confuse developers about the relationship between these identifiers.Add a short explanation about how the service ID relates to the component key pattern, such as:
tags: - { name: 'sylius.live_component.admin', key: 'app_admin:twig:component:supplier:form' } # key == identifier used in Twig hooks configuration. + # Note: The key uses colons instead of dots compared to service ID, but should represent the same hierarchy.
113-113: Fix grammatical errorThere's a grammatical error in "Let's to create a new one" - should be either "Let's create a new one" or "We need to create a new one".
-Since we want to add a `LiveCollectionType` to our entity, we can no longer rely on the default form type from the resource bundle. Let's to create a new one: +Since we want to add a `LiveCollectionType` to our entity, we can no longer rely on the default form type from the resource bundle. Let's create a new one:🧰 Tools
🪛 LanguageTool
[grammar] ~113-~113: Using the to-infinitive may not be correct in this context. Consider using the bare infinitive (without “to”) instead.
Context: ...rm type from the resource bundle. Let's to create a new one: ```php <?php // src/Form/T...(LET_IT_INFINITIVE)
308-318: Missing use statements for Count constraintThe example uses the
Countconstraint without showing the necessaryusestatement. This could lead to implementation errors.Add a note about the required use statement:
// src/Form/Type/SupplierType.php ... +// Note: Add this use statement at the top of your file: +// use Symfony\Component\Validator\Constraints\Count; + ->add('channels', ChannelAutocompleteType::class, [ 'multiple' => true, 'required' => true, 'constraints' => [ new Count([ 'min' => 1, 'minMessage' => 'app.supplier.channels.min_count', ]), ], ])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
docs/.gitbook/assets/image (1) (1) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (1) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (15).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (2) (1) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (2) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (2).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (3) (1) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (3) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (3).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (4) (1) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (4) (1).pngis excluded by!**/*.pngdocs/.gitbook/assets/image (4).pngis excluded by!**/*.pngdocs/.gitbook/assets/image.pngis excluded by!**/*.pngsrc/Sylius/Bundle/AdminBundle/Resources/assets/images/sylius-plus-sidebar.svgis excluded by!**/*.svg
📒 Files selected for processing (24)
config/packages/security.yaml(1 hunks)docs/SUMMARY.md(1 hunks)docs/getting-started-with-sylius/customizing-the-shop.md(2 hunks)docs/the-customization-guide/customizing-forms/README.md(2 hunks)docs/the-customization-guide/customizing-forms/how-to-add-a-live-form-for-a-custom-model.md(1 hunks)docs/the-customization-guide/customizing-models/how-to-add-a-custom-model.md(2 hunks)docs/the-customization-guide/customizing-templates.md(1 hunks)src/Sylius/Bundle/AdminBundle/Menu/MainMenuBuilder.php(2 hunks)src/Sylius/Bundle/AdminBundle/Resources/assets/entrypoint.js(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/assets/styles/_navbar.scss(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/assets/styles/_plus-features.scss(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/assets/styles/main.scss(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/assets/styles/ui/_icons.scss(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/admin_user/create.yaml(1 hunks)src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/admin_user/update.yaml(1 hunks)src/Sylius/Bundle/AdminBundle/templates/admin_user/form/sections/roles.html.twig(1 hunks)src/Sylius/Bundle/AdminBundle/templates/shared/crud/common/sidebar/menu/menu.html.twig(1 hunks)src/Sylius/Bundle/AdminBundle/templates/shared/grid/action/delete.html.twig(1 hunks)src/Sylius/Bundle/AdminBundle/templates/shared/grid/action/show.html.twig(1 hunks)src/Sylius/Bundle/AdminBundle/templates/shared/grid/action/update.html.twig(1 hunks)src/Sylius/Bundle/ApiBundle/Converter/IriToIdentifierConverter.php(1 hunks)src/Sylius/Bundle/ApiBundle/Tests/Converter/IriToIdentifierConverterTest.php(2 hunks)src/Sylius/Bundle/ShopBundle/Twig/Component/Product/AddToCartFormComponent.php(1 hunks)src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Sylius/Bundle/ApiBundle/Tests/Converter/IriToIdentifierConverterTest.php (1)
src/Sylius/Bundle/ApiBundle/Converter/IriToIdentifierConverter.php (1)
isIdentifier(90-105)
🪛 LanguageTool
docs/the-customization-guide/customizing-templates.md
[style] ~236-~236: The contraction ‘There’re’ is uncommon in written English.
Context: ...="">
(THERE_RE_CONTRACTION_UNCOMMON)
[misspelling] ~237-~237: This word is normally spelled with a hyphen.
Context: ...aption> There're two way's of finding out the hook name you'd li...
(EN_COMPOUNDS_TWO_WAY)
docs/the-customization-guide/customizing-forms/how-to-add-a-live-form-for-a-custom-model.md
[typographical] ~62-~62: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...n. ``` The service has been registered, however, it's not yet used anywhere. The next s...
(HOWEVER_SENTENCE)
[style] ~87-~87: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...esource: '@=_context.resource' We need to provide: yaml # config/packages/syl...
(REP_NEED_TO_VB)
[locale-violation] ~109-~109: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...get to create and execute the migration afterwards. ### Supplier FormType Since we want ...
(AFTERWARDS_US)
[grammar] ~113-~113: Using the to-infinitive may not be correct in this context. Consider using the bare infinitive (without “to”) instead.
Context: ...rm type from the resource bundle. Let's to create a new one: ```php <?php // src/Form/T...
(LET_IT_INFINITIVE)
[style] ~240-~240: Consider replacing ‘gives’ with a different word to let your writing stand out.
Context: ...eld separately is a bit tedious, but it gives us the flexibility to customize each field...
(GIVE_TIME_STYLE)
[locale-violation] ~261-~261: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...y Maker wizard and handle the migration afterwards. ### Channel Autocomplete Type ```php...
(AFTERWARDS_US)
[style] ~322-~322: Consider an alternative to strengthen your wording.
Context: ...{% tab title="Basic" %} Ready to go. No further changes needed. {% endtab %} {% tab title="Adv...
(CHANGES_ADJUSTMENTS)
🔇 Additional comments (37)
src/Sylius/Bundle/ApiBundle/Converter/IriToIdentifierConverter.php (1)
96-97: Approve this security improvementAdding URL sanitization before route matching is a good security practice. This helps prevent potential issues with malformed or malicious URLs that could otherwise cause unexpected behavior in the routing system.
src/Sylius/Bundle/ApiBundle/Tests/Converter/IriToIdentifierConverterTest.php (6)
95-108: Comprehensive test for URL sanitization with Unicode charactersThis test correctly verifies that the converter sanitizes URLs with various Unicode characters before attempting to match them against routes. The implementation ensures that the converter passes the exact sanitized value to the router.
110-124: Good test coverage for invalid URL handlingThis test ensures that sanitized but still invalid URLs are properly rejected by verifying the router is called with the sanitized value and then returns appropriate results.
126-137: Approve test for sanitized URLs that resolve to valid routesThis test specifically handles the case where a URL containing line breaks would be invalid without sanitization but becomes valid after sanitization. It verifies the correct behavior of accepting such URLs after they've been sanitized.
139-150: Good test coverage for URLs that remain invalid after sanitizationThis test ensures that URLs that still don't match routes after sanitization are properly rejected, providing complete coverage of the sanitization behavior.
250-263: Thorough data provider for Unicode URL testingThis data provider includes a diverse set of Unicode characters from multiple writing systems (Cyrillic, Greek, Korean, Vietnamese, Turkish, Arabic, Chinese, Japanese), ensuring robust testing of the sanitization functionality across different language scenarios.
265-276: Comprehensive data provider for invalid URL scenariosThis data provider covers a wide range of potentially problematic inputs including line breaks, control characters, script tags, malformed URLs, and JavaScript protocol handlers. This is important for ensuring the sanitization process properly handles edge cases and potential attack vectors.
src/Sylius/Bundle/AdminBundle/templates/shared/grid/action/show.html.twig (3)
1-6: Improved path resolution logic looks good!The new conditional approach for determining the path is more robust and explicit than the previous implementation. It properly handles the case when a direct URL is provided and falls back to generating a path from routes when needed.
9-9: Proper handling of unavailable actions.Adding the
disabledvariable is a good practice to track when no valid path is available, which will be used to visually indicate disabled state to users.
11-11: Enhanced UI feedback for unavailable actions.The conditional addition of the "disabled" class provides appropriate visual feedback to users when the action is not available, improving the overall user experience.
src/Sylius/Bundle/AdminBundle/templates/shared/grid/action/delete.html.twig (3)
3-8: Improved path resolution logic looks good!The new conditional approach for determining the path is more robust and handles edge cases better. This is consistent with the pattern applied to the other grid action templates.
10-10: Proper tracking of disabled state.The addition of the
disabledvariable is a good practice to track when the action should be disabled due to an invalid path.
12-12: Enhanced component integration.Passing the
disabledflag to the delete_modal component enables proper handling of the disabled state within the component, improving the user interface consistency.src/Sylius/Bundle/AdminBundle/templates/shared/grid/action/update.html.twig (3)
1-6: Improved path resolution logic looks good!The new conditional approach for determining the path is more robust and explicit. It properly handles cases where a direct URL is provided and falls back to generating a path from routes.
9-9: Proper handling of unavailable actions.Adding the
disabledvariable to track when no valid path is available is a good practice that improves code readability and maintainability.
11-11: Enhanced UI feedback for unavailable actions.Conditionally adding the "disabled" class provides appropriate visual feedback to users when the action is unavailable, improving the user experience and maintaining consistency with other grid actions.
src/Sylius/Bundle/AdminBundle/Resources/assets/styles/main.scss (1)
37-37: Import plus-features styles
Adding@import "plus-features";at the end properly includes the new blur-overlay stylesheet for Sylius Plus features.src/Sylius/Bundle/AdminBundle/Resources/assets/entrypoint.js (1)
29-29: Add Sylius Plus sidebar badge asset
The import of./images/sylius-plus-sidebar.svgcorrectly brings in the new badge icon for menu items flagged withplus_logo. Ensure the bundler’s asset loader handles SVG imports as expected.src/Sylius/Bundle/AdminBundle/Resources/assets/styles/_navbar.scss (1)
29-31: Define.navbar-plus-badgewidth
The new.navbar-plus-badge { width: 50px; }rule provides consistent sizing for the Sylius Plus badge in the navbar. This aligns with the expected design.src/Sylius/Bundle/AdminBundle/Resources/assets/styles/ui/_icons.scss (1)
5-7: Set custom line-height for small buttons
Introducing--tblr-btn-line-height: 1.4;for.btn-smand.btn-group-sm > .btnensures icon/button alignment remains consistent in the compact UI.src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/admin_user/create.yaml (1)
27-29: Register newrolessection in admin user form
Theroleshook undersections#leftcorrectly points to theroles.html.twigtemplate with priority -100, ensuring it renders in the intended order.src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/admin_user/update.yaml (1)
27-29: Ensure roles section is added with correct priority
The newroleshookable is inserted into the left form section with priority-100. Please verify that the same priority is used in the corresponding create hook (create.yaml) to maintain consistent ordering across forms.docs/SUMMARY.md (1)
113-114: Update Table of Contents to include live form guide
The "Customizing Forms" section and its sub-entry for adding a live form are correctly added. Verify that these links match the directory structure and that the target files exist.docs/getting-started-with-sylius/customizing-the-shop.md (1)
25-25: Verify updated image paths
The image references have been updated toimage (3) (1) (1).png(line 25) andimage (1) (1) (1).png(line 113). Please confirm these files are present in the.gitbook/assetsdirectory (or the correct assets folder) and render as expected.Also applies to: 113-113
docs/the-customization-guide/customizing-templates.md (2)
235-235: Verify updated image reference
The example image for "Insert new element" was changed toimage (1) (1).png. Ensure this asset is committed and that the relative path is correct.
242-242: Verify profiler screenshot path
The profiler screenshot reference was updated toimage (4) (1).png. Please confirm the file exists and renders properly in the documentation.config/packages/security.yaml (1)
15-18: Dev firewall repositioned before admin
Moving thedevfirewall aboveadminensures profiler, WDT, and asset routes bypass security correctly, preventing them from matching the admin firewall unintentionally.docs/the-customization-guide/customizing-forms/README.md (2)
22-22: Path correction for customizing-models documentation link.The path to the customizing-models documentation has been properly updated to use a relative path that reflects the correct directory structure (
../customizing-models/instead ofcustomizing-models/).
171-171: Path correction for screenshot image.The path to the screenshot image has been correctly updated to use a relative path that reflects the proper directory structure (
../../.gitbook/assets/instead of../.gitbook/assets/).docs/the-customization-guide/customizing-models/how-to-add-a-custom-model.md (3)
164-164: Resource route configuration excludes 'show' action.The routing configuration for the
app_admin_supplierresource now excludes the 'show' route, which is appropriate as the 'show' action isn't necessary for this admin resource since users will likely navigate to the 'update' page to view details.
180-192: Added translation step for the Supplier resource.Adding translations for the new Supplier resource is an important step that was previously missing from the documentation. This ensures proper internationalization support for the custom model.
193-193: Updated step number for admin panel check.The step number for the admin panel check has been correctly updated to accommodate the new translations step.
src/Sylius/Bundle/AdminBundle/Menu/MainMenuBuilder.php (2)
44-44: Added Administration submenu to the main menu.The method call
$this->addAdministrationSubMenu($menu)has been added to create a new Administration section in the main menu.
397-412: Implementation of Administration submenu with roles management.A new private method
addAdministrationSubMenuhas been added to create an Administration submenu with a roles item. The implementation correctly:
- Creates a submenu with an appropriate icon
- Adds a "roles" menu item that links to a Sylius Plus promotional URL
- Sets appropriate attributes for the external link (opening in a new tab)
- Uses the
plus_logoextra attribute to visually indicate this is a premium featureThis change is part of a broader integration of role management UI elements in the admin panel, which includes new templates, Twig hooks, and translation strings.
src/Sylius/Bundle/UiBundle/Resources/translations/messages.en.yml (2)
348-348: Added translation for permission checker toggle.A new translation key
enabled_permission_checkerwith the value "Enabled permission checker" has been added, which will be used in the role management interface.
776-779: Added translations for role types.Four new role-related translation keys have been added:
role_super_admin: "Super Admin"role_product_manager: "Product Manager"role_fulfillment_worker: "Fulfilment Worker"role_channel_admin: "Channel Admin"These translations support the role management feature being introduced in the admin interface.
src/Sylius/Bundle/AdminBundle/templates/shared/crud/common/sidebar/menu/menu.html.twig (1)
92-98: LGTM: Menu item display enhancements with Plus featuresThe changes add proper alignment and spacing with Bootstrap 5 classes and implement a consistent hierarchy for displaying item labels, plus badges, and external link icons. The conditional logic to distinguish Sylius Plus features is well-structured.
| $this->variant = $newVariant; | ||
|
|
||
| $this->emitUp(self::SYLIUS_SHOP_VARIANT_CHANGED, ['variantId' => $this->variant?->getId()]); | ||
| $this->emit(self::SYLIUS_SHOP_VARIANT_CHANGED, ['variantId' => $this->variant?->getId()]); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Event emission scope changed from bubbling up to local.
The change from $this->emitUp() to $this->emit() modifies how the SYLIUS_SHOP_VARIANT_CHANGED event propagates. This event will now only be handled locally within the component rather than bubbling up to parent components.
Make sure this intentional change doesn't break any parent components that might have been relying on receiving this event. Since this is part of an upmerge from 2.0 to 2.1, verify that any components depending on this event in the 2.1 branch have been updated accordingly.
🏁 Script executed:
#!/bin/bash
# Search for any listeners for the SYLIUS_SHOP_VARIANT_CHANGED event
echo "Searching for components listening for SYLIUS_SHOP_VARIANT_CHANGED event..."
rg -l "SYLIUS_SHOP_VARIANT_CHANGED" --type php
# Search for any usage of emitUp in the codebase to understand the pattern
echo -e "\nSearching for usage of emitUp method to understand the pattern..."
rg -l "emitUp\(" --type phpLength of output: 536
#!/bin/bash
# Display how SummaryComponent listens for SYLIUS_SHOP_VARIANT_CHANGED
rg -n "SYLIUS_SHOP_VARIANT_CHANGED" -C5 src/Sylius/Bundle/ShopBundle/Twig/Component/Product/SummaryComponent.php || true#!/bin/bash
# Show how SummaryComponent listens for SYLIUS_SHOP_VARIANT_CHANGED
rg -n "SYLIUS_SHOP_VARIANT_CHANGED" -C5 src/Sylius/Bundle/ShopBundle/Twig/Component/Product/SummaryComponent.php || true#!/bin/bash
# Inspect how SummaryComponent subscribes to the SYLIUS_SHOP_VARIANT_CHANGED event
rg -n "SYLIUS_SHOP_VARIANT_CHANGED" -C5 src/Sylius/Bundle/ShopBundle/Twig/Component/Product/SummaryComponent.php || true
# Inspect the $listeners property in SummaryComponent
rg -n "protected \$listeners" -C5 src/Sylius/Bundle/ShopBundle/Twig/Component/Product/SummaryComponent.php || trueEnsure variant‐changed event still reaches the product summary component
The switch from
$this->emitUp(self::SYLIUS_SHOP_VARIANT_CHANGED, ['variantId' => $this->variant?->getId()]);to
$this->emit(self::SYLIUS_SHOP_VARIANT_CHANGED, ['variantId' => $this->variant?->getId()]);in
src/Sylius/Bundle/ShopBundle/Twig/Component/Product/AddToCartFormComponent.php:107
now confines the event to the form component. However, SummaryComponent (src/Sylius/Bundle/ShopBundle/Twig/Component/Product/SummaryComponent.php) still listens for SYLIUS_SHOP_VARIANT_CHANGED and will no longer receive it.
Please choose one of the following fixes:
- Revert to
emitUp()so the event continues bubbling. - Target the summary explicitly:
- $this->emit(self::SYLIUS_SHOP_VARIANT_CHANGED, ['variantId' => $this->variant?->getId()]); + $this->emitTo(SummaryComponent::getName(), self::SYLIUS_SHOP_VARIANT_CHANGED, ['variantId' => $this->variant?->getId()]);
- Update
SummaryComponent::$listeners(or its equivalent) to handle a locally emitted event bus.
| <div class="mb-3 field"> | ||
| <label for="sylius_admin_admin_user_rolesResources" class="form-label">Roles</label> | ||
| <select multiple="multiple" class="form-select" id="sylius_admin_admin_user_rolesResources"> | ||
| <option>{{ 'sylius.ui.role_super_admin'|trans }}</option> | ||
| <option>{{ 'sylius.ui.role_product_manager'|trans }}</option> | ||
| <option>{{ 'sylius.ui.role_fulfillment_worker'|trans }}</option> | ||
| <option>{{ 'sylius.ui.role_channel_admin'|trans }}</option> | ||
| </select> | ||
| </div> |
There was a problem hiding this comment.
Missing form name integration with Symfony form handling
The multi-select element isn't properly integrated with Symfony's form handler. It has a hard-coded ID that doesn't match Symfony's form naming convention and doesn't use the form_widget function, making it non-functional for data binding.
Consider changing to:
- <div class="mb-3 field">
- <label for="sylius_admin_admin_user_rolesResources" class="form-label">Roles</label>
- <select multiple="multiple" class="form-select" id="sylius_admin_admin_user_rolesResources">
- <option>{{ 'sylius.ui.role_super_admin'|trans }}</option>
- <option>{{ 'sylius.ui.role_product_manager'|trans }}</option>
- <option>{{ 'sylius.ui.role_fulfillment_worker'|trans }}</option>
- <option>{{ 'sylius.ui.role_channel_admin'|trans }}</option>
- </select>
- </div>
+ <div class="mb-3 field">
+ <label for="{{ form.roles.vars.id }}" class="form-label">{{ 'sylius.ui.roles'|trans }}</label>
+ {{ form_widget(form.roles, {'attr': {'class': 'form-select'}}) }}
+ {{ form_errors(form.roles) }}
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="mb-3 field"> | |
| <label for="sylius_admin_admin_user_rolesResources" class="form-label">Roles</label> | |
| <select multiple="multiple" class="form-select" id="sylius_admin_admin_user_rolesResources"> | |
| <option>{{ 'sylius.ui.role_super_admin'|trans }}</option> | |
| <option>{{ 'sylius.ui.role_product_manager'|trans }}</option> | |
| <option>{{ 'sylius.ui.role_fulfillment_worker'|trans }}</option> | |
| <option>{{ 'sylius.ui.role_channel_admin'|trans }}</option> | |
| </select> | |
| </div> | |
| <div class="mb-3 field"> | |
| <label for="{{ form.roles.vars.id }}" class="form-label">{{ 'sylius.ui.roles'|trans }}</label> | |
| {{ form_widget(form.roles, {'attr': {'class': 'form-select'}}) }} | |
| {{ form_errors(form.roles) }} | |
| </div> |
| <div class="form-check"> | ||
| <input type="checkbox" class="form-check-input" id="sylius_admin_admin_user_enablePermissionChecker" value="1" checked> | ||
| <label class="form-check-label" for="sylius_admin_admin_user_enablePermissionChecker"> | ||
| {{ 'sylius.ui.enabled_permission_checker'|trans }} | ||
| </label> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Permission checker not integrated with Symfony form
Similar to the roles dropdown, the permission checker checkbox isn't properly integrated with Symfony's form handling, making it non-functional for data binding.
Consider changing to:
- <div class="mb-3 field">
- <div class="form-check">
- <input type="checkbox" class="form-check-input" id="sylius_admin_admin_user_enablePermissionChecker" value="1" checked>
- <label class="form-check-label" for="sylius_admin_admin_user_enablePermissionChecker">
- {{ 'sylius.ui.enabled_permission_checker'|trans }}
- </label>
- </div>
- </div>
+ <div class="mb-3 field">
+ <div class="form-check">
+ {{ form_widget(form.enablePermissionChecker, {'attr': {'class': 'form-check-input'}}) }}
+ {{ form_label(form.enablePermissionChecker, 'sylius.ui.enabled_permission_checker'|trans, {'label_attr': {'class': 'form-check-label'}}) }}
+ {{ form_errors(form.enablePermissionChecker) }}
+ </div>
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div class="form-check"> | |
| <input type="checkbox" class="form-check-input" id="sylius_admin_admin_user_enablePermissionChecker" value="1" checked> | |
| <label class="form-check-label" for="sylius_admin_admin_user_enablePermissionChecker"> | |
| {{ 'sylius.ui.enabled_permission_checker'|trans }} | |
| </label> | |
| </div> | |
| </div> | |
| <div class="mb-3 field"> | |
| <div class="form-check"> | |
| {{ form_widget(form.enablePermissionChecker, {'attr': {'class': 'form-check-input'}}) }} | |
| {{ form_label(form.enablePermissionChecker, 'sylius.ui.enabled_permission_checker'|trans, {'label_attr': {'class': 'form-check-label'}}) }} | |
| {{ form_errors(form.enablePermissionChecker) }} | |
| </div> | |
| </div> |
| #[AsEntityAutocompleteField(route: 'sylius_admin_entity_autocomplete')] | ||
| class ChannelAutocompleteType extends AbstractType | ||
| { | ||
| public function configureOptions(OptionsResolver $resolver): void | ||
| { | ||
| $resolver->setDefaults([ | ||
| 'class' => Channel::class, | ||
| 'placeholder' => 'Choose a Channel', | ||
| 'choice_label' => 'name', | ||
| 'searchable_fields' => ['name'], | ||
| ]); | ||
| } | ||
|
|
||
| public function getParent(): string | ||
| { | ||
| return BaseEntityAutocompleteType::class; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing mandatory symfony/ux-autocomplete package requirement
The guide uses Symfony\UX\Autocomplete classes but doesn't mention installing the required package. Without explicit installation instructions, developers might encounter errors when implementing this feature.
Add installation instructions at the beginning of the autocomplete section:
## Live autocomplete
+### Installing required packages
+
+Before implementing the autocomplete feature, install the Symfony UX Autocomplete package:
+
+```bash
+composer require symfony/ux-autocomplete
+```
+
+Don't forget to run the Webpack Encore build to include the necessary JavaScript assets:
+
+```bash
+npm install --force
+npm run build
+```
+
### Expanding the Supplier Entity again| 'entry_type' => ZoneChoiceType::class, | ||
| 'allow_add' => true, | ||
| 'allow_delete' => true, | ||
| 'by_reference' => false, | ||
| 'button_add_options' => [ | ||
| 'label' => 'sylius.form.zone.add_member' | ||
| ], | ||
| 'button_delete_options' => [ | ||
| 'attr' => ['class' => 'btn btn-outline-danger'] | ||
| ], | ||
| 'delete_empty' => false, | ||
| ]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing entry_options for ZoneChoiceType configuration
When using collections with entity types like ZoneChoiceType, it's often necessary to provide entry_options to properly configure the entry form. This might lead to errors or unexpected behavior if specific options are required by ZoneChoiceType.
Consider adding entry_options:
->add('zones', LiveCollectionType::class, [
'entry_type' => ZoneChoiceType::class,
+ 'entry_options' => [
+ 'label' => false,
+ 'required' => true,
+ ],
'allow_add' => true,
'allow_delete' => true,
'by_reference' => false,
'button_add_options' => [
'label' => 'sylius.form.zone.add_member'
],
'button_delete_options' => [
'attr' => ['class' => 'btn btn-outline-danger']
],
'delete_empty' => false,
])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'entry_type' => ZoneChoiceType::class, | |
| 'allow_add' => true, | |
| 'allow_delete' => true, | |
| 'by_reference' => false, | |
| 'button_add_options' => [ | |
| 'label' => 'sylius.form.zone.add_member' | |
| ], | |
| 'button_delete_options' => [ | |
| 'attr' => ['class' => 'btn btn-outline-danger'] | |
| ], | |
| 'delete_empty' => false, | |
| ]) | |
| ->add('zones', LiveCollectionType::class, [ | |
| 'entry_type' => ZoneChoiceType::class, | |
| 'entry_options' => [ | |
| 'label' => false, | |
| 'required' => true, | |
| ], | |
| 'allow_add' => true, | |
| 'allow_delete' => true, | |
| 'by_reference' => false, | |
| 'button_add_options' => [ | |
| 'label' => 'sylius.form.zone.add_member' | |
| ], | |
| 'button_delete_options' => [ | |
| 'attr' => ['class' => 'btn btn-outline-danger'] | |
| ], | |
| 'delete_empty' => false, | |
| ]) |
This PR has been generated automatically.
For more details see upmerge_pr.yaml.
Remember! The upmerge should always be merged with using
Merge pull requestbutton.In case of conflicts, please resolve them manually with usign the following commands:
If you use other name for the upstream remote, please replace
upstreamwith the name of your remote pointing to theSylius/Syliusrepository.Once the conflicts are resolved, please run
git merge --continueand push the changes to this PR.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores