Fix autocomplete in admin to be case-insensitive#18393
Fix autocomplete in admin to be case-insensitive#18393rust-le wants to merge 6 commits intoSylius:2.1from
Conversation
WalkthroughAdds case-insensitive matching to the translatable autocomplete query and introduces Behat UI support and a new BDD feature to test case-insensitive parent taxon autocomplete in the admin taxonomy flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin UI
participant BE as Behat Taxon FormElement
participant AH as AutocompleteHelper
participant API as Backend / Autocomplete
note right of BE #f0f8ff: Added searchParentTaxonAutocomplete()
Admin->>BE: trigger parent taxon autocomplete search(term)
BE->>AH: autocompleteHelper.search(driver, parentElementXPath, term)
AH->>API: request /autocomplete?query=term
note right of API #f6fff0: Server applies LOWER(field) LIKE LOWER(:query)
API-->>AH: results
AH-->>BE: array of results
BE-->>Admin: return results for assertion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{php,yaml,yml,xml,twig}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation in PHP, YAML, XML, and Twig files
Files:
src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php
**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
**/*.php: Use modern PHP 8.2+ syntax and features
Declare strict_types=1 in all PHP files
Follow the Sylius Coding Standard
Do not use deprecated features from PHP, Symfony, or Sylius
Use final for all classes, except entities and repositories
Use readonly for immutable services and value objects
Add type declarations for all properties, arguments, and return values
Use camelCase for variables and method names
Use SCREAMING_SNAKE_CASE for constants
Use fast returns instead of unnecessary nesting
Use trailing commas in multi-line arrays and argument lists
Order array keys alphabetically where applicable
Use PHPDoc only when necessary (e.g., @var Collection)
Group class elements in order: constants, properties, constructor, public, protected, private methods
Group getter and setter methods for the same properties together
Suffix interfaces with Interface and traits with Trait
Use use statements for all non-global classes
Sort use imports alphabetically and group by type (classes, functions, constants)
Files:
src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: End-to-end tests (PostgreSQL) / Non-JS, PHP 8.4, Symfony ^7.3, PostgreSQL 17.5
- GitHub Check: End-to-end tests (MySQL) / JS with Panther, PHP 8.3, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / JS with Chromedriver, PHP 8.4, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / Non-JS, PHP 8.3, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / JS with Panther, PHP 8.2, Symfony ^6.4 (test_cached), MySQL 8.0, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / JS with Chromedriver, PHP 8.2, Symfony ^6.4 (test_cached), MySQL 8.0, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / JS with Chromedriver, PHP 8.3, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / Non-JS, PHP 8.4, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / Non-JS, PHP 8.2, Symfony ^6.4 (test_cached), MySQL 8.0, Twig ^3.3
- GitHub Check: End-to-end tests (MariaDB) / Non-JS, PHP 8.4, Symfony ^7.3, MariaDB 11.4.7, State Machine Adapter symfony_workflow, ApiPlatform ~4.1.7
- GitHub Check: End-to-end tests (MariaDB) / Non-JS, PHP 8.4, Symfony ^7.3, MariaDB 11.4.7, State Machine Adapter symfony_workflow
- GitHub Check: End-to-end tests (MariaDB) / Non-JS, PHP 8.2, Symfony ^6.4, MariaDB 10.11.13, State Machine Adapter winzou_state_machine
- GitHub Check: Packages / PHP 8.2, Symfony ^6.4
- GitHub Check: Packages / PHP 8.4, Symfony ^7.3
- GitHub Check: Packages / PHP 8.3, Symfony ^7.3, ORM ^3.3
- GitHub Check: WIP
- GitHub Check: WIP
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Sylius/Behat/Context/Ui/Admin/ManagingTaxonsContext.php (1)
33-33: Class signature change affects immutability.The class was changed from
final readonly classtofinal class, removing the readonly constraint. This is necessary to support the mutable$autocompleteSearchResultsproperty, but it reduces immutability guarantees.As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
features/admin/taxonomy/managing_taxons/case_insensitive_parent_autocomplete.feature(1 hunks)src/Sylius/Behat/Context/Ui/Admin/ManagingTaxonsContext.php(2 hunks)src/Sylius/Behat/Element/Admin/Taxon/FormElement.php(1 hunks)src/Sylius/Behat/Element/Admin/Taxon/FormElementInterface.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{php,yaml,yml,xml,twig}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation in PHP, YAML, XML, and Twig files
Files:
src/Sylius/Behat/Element/Admin/Taxon/FormElement.phpsrc/Sylius/Behat/Context/Ui/Admin/ManagingTaxonsContext.phpsrc/Sylius/Behat/Element/Admin/Taxon/FormElementInterface.php
**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
**/*.php: Use modern PHP 8.2+ syntax and features
Declare strict_types=1 in all PHP files
Follow the Sylius Coding Standard
Do not use deprecated features from PHP, Symfony, or Sylius
Use final for all classes, except entities and repositories
Use readonly for immutable services and value objects
Add type declarations for all properties, arguments, and return values
Use camelCase for variables and method names
Use SCREAMING_SNAKE_CASE for constants
Use fast returns instead of unnecessary nesting
Use trailing commas in multi-line arrays and argument lists
Order array keys alphabetically where applicable
Use PHPDoc only when necessary (e.g., @var Collection)
Group class elements in order: constants, properties, constructor, public, protected, private methods
Group getter and setter methods for the same properties together
Suffix interfaces with Interface and traits with Trait
Use use statements for all non-global classes
Sort use imports alphabetically and group by type (classes, functions, constants)
Files:
src/Sylius/Behat/Element/Admin/Taxon/FormElement.phpsrc/Sylius/Behat/Context/Ui/Admin/ManagingTaxonsContext.phpsrc/Sylius/Behat/Element/Admin/Taxon/FormElementInterface.php
{features/**/*.feature,behat.yml*}
📄 CodeRabbit inference engine (AGENTS.md)
Use Behat for behavior-driven scenarios
Files:
features/admin/taxonomy/managing_taxons/case_insensitive_parent_autocomplete.feature
🧬 Code graph analysis (3)
src/Sylius/Behat/Element/Admin/Taxon/FormElement.php (1)
src/Sylius/Behat/Element/Admin/Taxon/FormElementInterface.php (1)
searchParentTaxonAutocomplete(49-49)
src/Sylius/Behat/Context/Ui/Admin/ManagingTaxonsContext.php (2)
src/Sylius/Behat/Element/Admin/Taxon/FormElement.php (2)
__construct(31-37)searchParentTaxonAutocomplete(110-119)src/Sylius/Behat/Element/Admin/Taxon/FormElementInterface.php (1)
searchParentTaxonAutocomplete(49-49)
src/Sylius/Behat/Element/Admin/Taxon/FormElementInterface.php (1)
src/Sylius/Behat/Element/Admin/Taxon/FormElement.php (1)
searchParentTaxonAutocomplete(110-119)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: End-to-end tests (MariaDB) / Get matrix
- GitHub Check: End-to-end tests (MySQL) / Get matrix
- GitHub Check: Frontend / Get matrix
- GitHub Check: Packages / Get matrix
- GitHub Check: End-to-end tests (PostgreSQL) / Get matrix
- GitHub Check: WIP
🔇 Additional comments (7)
src/Sylius/Behat/Element/Admin/Taxon/FormElementInterface.php (1)
48-49: LGTM!The new method signature is properly declared with strict typing and follows the interface conventions.
src/Sylius/Behat/Element/Admin/Taxon/FormElement.php (1)
110-119: LGTM!The method implementation correctly:
- Uses the injected
autocompleteHelperto perform the search- Delegates to the parent element's XPath as expected
- Returns a consistent array format using
array_values()or empty array fallback- Follows PHP 8.2+ syntax with proper type declarations
The implementation aligns with the interface contract and integrates well with existing autocomplete functionality in the class.
features/admin/taxonomy/managing_taxons/case_insensitive_parent_autocomplete.feature (2)
1-37: Well-structured BDD scenarios for case-insensitive testing.The feature file properly covers the key case-insensitive scenarios:
- Lowercase to uppercase matching
- Uppercase to mixed-case matching
- Mixed-case variations
- Partial matching with case insensitivity
The scenarios use appropriate Gherkin syntax and follow BDD best practices with clear Given-When-Then structure. The
@ui @mink:chromedrivertags correctly indicate browser-based testing requirements.As per coding guidelines
15-15: Verify ChromeDriver configuration and version compatibility
- Choose direct ChromeDriver (dmore/chrome-mink-driver with
--remote-debugging-*) or Selenium2Driver + chromedriver- Install a chromedriver binary on PATH (or pass via
-Dwebdriver.chrome.driver) that matches your Chrome/Chromium version (Chrome 59+ for headless)- Configure headless/display flags and update your behat.yml accordingly
src/Sylius/Behat/Context/Ui/Admin/ManagingTaxonsContext.php (3)
35-35: LGTM!The property declaration follows PHP 8.2+ syntax with proper type declaration and appropriate initialization.
37-48: LGTM!The constructor parameters have been properly migrated to
private readonly, maintaining immutability for injected dependencies while allowing the class to have mutable state for test results.As per coding guidelines
493-511: LGTM!The new methods are well-implemented:
- Proper Behat step annotations with descriptive patterns
- Correct usage of
testHelper->waitUntilPageLoads()for synchronization- Appropriate delegation to
formElement->searchParentTaxonAutocomplete()- Clear assertion with helpful error message using
sprintf()The methods follow the existing patterns in the context class and provide the necessary functionality for the BDD scenarios.
Based on learnings
features/admin/taxonomy/managing_taxons/case_insensitive_parent_autocomplete.feature
Outdated
Show resolved
Hide resolved
src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
features/admin/taxonomy/managing_taxons/case_insensitive_parent_autocomplete.feature(1 hunks)src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- features/admin/taxonomy/managing_taxons/case_insensitive_parent_autocomplete.feature
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{php,yaml,yml,xml,twig}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation in PHP, YAML, XML, and Twig files
Files:
src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php
**/*.php
📄 CodeRabbit inference engine (AGENTS.md)
**/*.php: Use modern PHP 8.2+ syntax and features
Declare strict_types=1 in all PHP files
Follow the Sylius Coding Standard
Do not use deprecated features from PHP, Symfony, or Sylius
Use final for all classes, except entities and repositories
Use readonly for immutable services and value objects
Add type declarations for all properties, arguments, and return values
Use camelCase for variables and method names
Use SCREAMING_SNAKE_CASE for constants
Use fast returns instead of unnecessary nesting
Use trailing commas in multi-line arrays and argument lists
Order array keys alphabetically where applicable
Use PHPDoc only when necessary (e.g., @var Collection)
Group class elements in order: constants, properties, constructor, public, protected, private methods
Group getter and setter methods for the same properties together
Suffix interfaces with Interface and traits with Trait
Use use statements for all non-global classes
Sort use imports alphabetically and group by type (classes, functions, constants)
Files:
src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: End-to-end tests (PostgreSQL) / Non-JS, PHP 8.4, Symfony ^7.3, PostgreSQL 17.5
- GitHub Check: End-to-end tests (PostgreSQL) / Non-JS, PHP 8.3, Symfony ^6.4, PostgreSQL 15.13
- GitHub Check: Frontend / NodeJS 24.x
- GitHub Check: Packages / PHP 8.3, Symfony ^6.4
- GitHub Check: Packages / PHP 8.4, Symfony ^7.3
- GitHub Check: End-to-end tests (MariaDB) / Non-JS, PHP 8.3, Symfony ^6.4, MariaDB 10.11.13, State Machine Adapter winzou_state_machine
- GitHub Check: End-to-end tests (MariaDB) / Non-JS, PHP 8.4, Symfony ^7.3, MariaDB 11.4.7, State Machine Adapter symfony_workflow
- GitHub Check: End-to-end tests (MariaDB) / Non-JS, PHP 8.4, Symfony ^7.3, MariaDB 11.4.7, State Machine Adapter symfony_workflow, ApiPlatform ~4.1.7
- GitHub Check: End-to-end tests (MySQL) / JS with Panther, PHP 8.4, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / JS with Chromedriver, PHP 8.4, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / Non-JS, PHP 8.3, Symfony ^6.4 (test_cached), MySQL 8.0, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / JS with Panther, PHP 8.3, Symfony ^6.4 (test_cached), MySQL 8.0, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / JS with Chromedriver, PHP 8.3, Symfony ^6.4 (test_cached), MySQL 8.0, Twig ^3.3
- GitHub Check: End-to-end tests (MySQL) / Non-JS, PHP 8.4, Symfony ^7.3 (test_cached), MySQL 8.4, Twig ^3.3
- GitHub Check: WIP
src/Sylius/Bundle/AdminBundle/Form/Type/TranslatableAutocompleteType.php
Show resolved
Hide resolved
This fix is for PostgreSQL, which is case-sensitive
c603c3d to
ff6d661
Compare
| /** | ||
| * @Given the store classifies its products as :taxonName with :taxonCode code | ||
| */ |
There was a problem hiding this comment.
We can use PHP Attributes already.
| And the store classifies its products as "Office Supplies" with "office_supplies" code | ||
| And I am logged in as an administrator | ||
|
|
||
| @ui @mink:chromedriver |
There was a problem hiding this comment.
| @ui @mink:chromedriver | |
| @no-api @ui @mink:chromedriver |
and the rest of scenarios.
| /** | ||
| * @When I search for :searchTerm in the parent taxon autocomplete | ||
| */ | ||
| public function iSearchForInTheParentTaxonAutocomplete(string $searchTerm): void | ||
| { | ||
| $this->autocompleteSearchResults = $this->formElement->searchParentTaxonAutocomplete($searchTerm); | ||
| } |
There was a problem hiding this comment.
The @When functions supposed to be placed before @Then (see above)
| final readonly class ManagingTaxonsContext implements Context | ||
| final class ManagingTaxonsContext implements Context | ||
| { | ||
| private array $autocompleteSearchResults = []; |
There was a problem hiding this comment.
Use sharedStorage instead of introducing state here so the class can remain read-only.
|
|
||
| public function isEnabled(): bool; | ||
|
|
||
| public function searchParentTaxonAutocomplete(string $searchTerm): array; |
There was a problem hiding this comment.
No need to complex the naming
| public function searchParentTaxonAutocomplete(string $searchTerm): array; | |
| public function searchParentTaxon(string $searchTerm): array; |
| @ui @mink:chromedriver | ||
| Scenario: Searching for parent taxon by lowercase code when taxon code has uppercase letters | ||
| When I want to create a new taxon | ||
| And I search for "tech_gadgets" in the parent taxon autocomplete |
There was a problem hiding this comment.
| And I search for "tech_gadgets" in the parent taxon autocomplete | |
| And I search for parent taxon "tech_gadgets" |
I’d avoid implementation details.
| Scenario: Searching for parent taxon by lowercase code when taxon code has uppercase letters | ||
| When I want to create a new taxon | ||
| And I search for "tech_gadgets" in the parent taxon autocomplete | ||
| Then I should see "Tech Gadgets" in the autocomplete results |
There was a problem hiding this comment.
| Then I should see "Tech Gadgets" in the autocomplete results | |
| Then I should see "Tech Gadgets" in the found results |
There was a problem hiding this comment.
Move this scenario file under the ui subdirectory as it contains only UI-specific scenarios.
Additionally: case_insensitive_parent_autocomplete -> case_insensitive_parent_search
|
This PR can be closed because I created a new one with all the fixes. New PR: #18477 |
|
Closed in favor of: #18477 |
This fix is for PostgreSQL, which is case-sensitive
Summary by CodeRabbit
Bug Fixes
Improvements
Tests