Conversation
WalkthroughForm-loading and asynchronous wait logic was moved from the removed Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Behat test / Page
participant Session as Mink Session (Driver)
participant DriverHelper as DriverHelper
Note over Test,DriverHelper #D6EAF8: Old flow replaced document-based waits
Test->>DriverHelper: call waitForFormToStopLoading($session)
DriverHelper->>Session: session.wait(timeout, "document.readyState === 'complete' && !document.querySelector('[data-live-is-loading]')")
alt conditions satisfied
Session-->>DriverHelper: returns
DriverHelper-->>Test: proceed
else timeout or no-js
Session-->>DriverHelper: returns (no-op)
DriverHelper-->>Test: proceed
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
be760ca to
ed874bc
Compare
❌ Preview Environment deleted from BunnyshellAvailable commands:
|
aa45053 to
1c0b658
Compare
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 (6)
src/Sylius/Behat/Page/Shop/Account/AddressBook/CreatePage.php(2 hunks)src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php(2 hunks)src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php(1 hunks)src/Sylius/Behat/Service/AutocompleteHelper.php(3 hunks)src/Sylius/Behat/Service/DriverHelper.php(1 hunks)src/Sylius/Behat/Service/JQueryHelper.php(0 hunks)
💤 Files with no reviewable changes (1)
- src/Sylius/Behat/Service/JQueryHelper.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php
- src/Sylius/Behat/Page/Shop/Account/AddressBook/CreatePage.php
🧰 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/Behat/Service/DriverHelper.phpsrc/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.phpsrc/Sylius/Behat/Service/AutocompleteHelper.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/Service/DriverHelper.phpsrc/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.phpsrc/Sylius/Behat/Service/AutocompleteHelper.php
🧬 Code graph analysis (3)
src/Sylius/Behat/Service/DriverHelper.php (2)
src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php (1)
waitForFormToStopLoading(74-77)src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php (1)
waitForFormToStopLoading(284-287)
src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php (4)
src/Sylius/Behat/Service/DriverHelper.php (2)
DriverHelper(22-52)waitForFormToStopLoading(46-51)src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php (1)
waitForFormToStopLoading(284-287)src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePageInterface.php (2)
waitForFormToStopLoading(32-32)saveChanges(34-34)src/Sylius/Behat/Page/Admin/Crud/UpdatePage.php (1)
saveChanges(37-44)
src/Sylius/Behat/Service/AutocompleteHelper.php (1)
src/Sylius/Behat/Service/DriverHelper.php (2)
DriverHelper(22-52)waitForAsynchronousActionsToFinish(41-44)
⏰ 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). (2)
- GitHub Check: Static checks / PHP 8.4, Symfony ^7.3
- GitHub Check: Static checks / PHP 8.3, Symfony ^6.4
🔇 Additional comments (3)
src/Sylius/Behat/Service/DriverHelper.php (1)
46-51: LGTM!The
waitForFormToStopLoadingmethod correctly guards the wait call with a JavaScript driver check and follows the same pattern aswaitForPageToLoad. The configurable timeout parameter is a nice addition for flexibility.src/Sylius/Behat/Service/AutocompleteHelper.php (1)
43-43: Migration to DriverHelper looks correct.The replacement of
JQueryHelper::waitForAsynchronousActionsToFinishwithDriverHelper::waitForAsynchronousActionsToFinishis consistent across all call sites. However, these calls depend on the fix for the critical issue inDriverHelper::waitForAsynchronousActionsToFinish(missing JavaScript driver check).Also applies to: 59-59, 71-71, 75-75
src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php (1)
19-19: LGTM!The migration from
JQueryHelpertoDriverHelperis complete and correct. All call sites properly useDriverHelper::waitForFormToStopLoading($this->getSession()), and the method includes the necessary JavaScript driver check.Also applies to: 66-66, 71-71, 76-76, 81-81
1c0b658 to
cf6938f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Sylius/Behat/Service/AutocompleteHelper.php (1)
84-84: Optional: Fix property name typo in existing code.The property name
singleNodeStepappears to be a typo and should likely besingleNodeValue(the standard XPath result property). This could prevent the visibility check from working correctly.Apply this diff to fix the typo:
- 'document.evaluate("%s", document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeStep.nodeType === 1 && document.evaluate("%s", document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeStep.offsetParent !== null', + 'document.evaluate("%s", document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue.nodeType === 1 && document.evaluate("%s", document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue.offsetParent !== null',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/Sylius/Behat/Page/Shop/Account/AddressBook/CreatePage.php(2 hunks)src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php(2 hunks)src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php(1 hunks)src/Sylius/Behat/Service/AutocompleteHelper.php(3 hunks)src/Sylius/Behat/Service/DriverHelper.php(1 hunks)src/Sylius/Behat/Service/JQueryHelper.php(0 hunks)
💤 Files with no reviewable changes (1)
- src/Sylius/Behat/Service/JQueryHelper.php
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php
- src/Sylius/Behat/Service/DriverHelper.php
- src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php
- src/Sylius/Behat/Page/Shop/Account/AddressBook/CreatePage.php
🧰 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/Behat/Service/AutocompleteHelper.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/Service/AutocompleteHelper.php
🧬 Code graph analysis (1)
src/Sylius/Behat/Service/AutocompleteHelper.php (1)
src/Sylius/Behat/Service/DriverHelper.php (2)
DriverHelper(22-62)waitForAsynchronousActionsToFinish(51-54)
⏰ 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). (1)
- GitHub Check: Static checks / PHP 8.3, Symfony ^6.4
Summary
JQueryHelpermethods toDriverHelperJQueryHelperclassDriverHelperinsteadBackground
The
JQueryHelpercontained two methods for waiting on asynchronous actions:waitForAsynchronousActionsToFinish()waitForFormToStopLoading()These methods are still needed for Behat tests, but
DriverHelperis a more appropriate location since it already handles JavaScript driver detection and page loading waits.Changes
Modified
DriverHelper: AddedwaitForAsynchronousActionsToFinish()andwaitForFormToStopLoading()methodsAutocompleteHelper: Updated to useDriverHelperinstead ofJQueryHelperCreatePage,UpdatePage,AddressPage: Updated to useDriverHelper::waitForFormToStopLoading()Removed
src/Sylius/Behat/Service/JQueryHelper.php(32 lines)Summary by CodeRabbit