Skip to content

Remove JQueryHelper#18503

Merged
GSadee merged 1 commit intoSylius:2.1from
Rafikooo:maintenance/remove-jquery-helper
Nov 4, 2025
Merged

Remove JQueryHelper#18503
GSadee merged 1 commit intoSylius:2.1from
Rafikooo:maintenance/remove-jquery-helper

Conversation

@Rafikooo
Copy link
Copy Markdown
Contributor

@Rafikooo Rafikooo commented Nov 2, 2025

Summary

  • Move JQueryHelper methods to DriverHelper
  • Delete obsolete JQueryHelper class
  • Update all usages to use DriverHelper instead

Background

The JQueryHelper contained two methods for waiting on asynchronous actions:

  • waitForAsynchronousActionsToFinish()
  • waitForFormToStopLoading()

These methods are still needed for Behat tests, but DriverHelper is a more appropriate location since it already handles JavaScript driver detection and page loading waits.

Changes

Modified

  • DriverHelper: Added waitForAsynchronousActionsToFinish() and waitForFormToStopLoading() methods
  • AutocompleteHelper: Updated to use DriverHelper instead of JQueryHelper
  • CreatePage, UpdatePage, AddressPage: Updated to use DriverHelper::waitForFormToStopLoading()

Removed

  • src/Sylius/Behat/Service/JQueryHelper.php (32 lines)

Summary by CodeRabbit

  • Chores
    • Consolidated test infrastructure by consolidating asynchronous action and form loading wait helpers into a unified driver helper system. Updated test pages for account address management, address book operations, and checkout flows to use the new consolidated helper, improving code consistency and test maintainability.

@Rafikooo Rafikooo requested review from a team as code owners November 2, 2025 23:59
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 2, 2025

Walkthrough

Form-loading and asynchronous wait logic was moved from the removed JQueryHelper into DriverHelper. Call sites in Behat page classes and helpers were updated to use DriverHelper session-based wait methods; the JQueryHelper file was deleted.

Changes

Cohort / File(s) Change Summary
Page class updates
src/Sylius/Behat/Page/Shop/Account/AddressBook/CreatePage.php, src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php, src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php
Replaced JQueryHelper import/calls with DriverHelper; updated calls to wait for form-loading to use DriverHelper::waitForFormToStopLoading($this->getSession()).
Autocomplete helper updates
src/Sylius/Behat/Service/AutocompleteHelper.php
Replaced JQueryHelper::waitForAsynchronousActionsToFinish($session) with DriverHelper::waitForAsynchronousActionsToFinish($session) at all wait points (chooseValues, removeValue, activateAutocompleteDropdown).
Driver helper additions
src/Sylius/Behat/Service/DriverHelper.php
Added public static function waitForAsynchronousActionsToFinish(Session $session): void and public static function waitForFormToStopLoading(Session $session, int $timeout = 1000): void to provide session-based waiting logic.
Removed helper
src/Sylius/Behat/Service/JQueryHelper.php
Deleted file and removed class Sylius\Behat\Service\JQueryHelper and its methods waitForAsynchronousActionsToFinish and waitForFormToStopLoading.

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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Repetitive refactor: consistent replacement of helper calls across a small set of files.
  • Pay attention to:
    • Any remaining references to JQueryHelper.
    • Correct session vs. document parameter usage in updated call sites.
    • Behavior in non-JS drivers (ensure DriverHelper mirrors previous no-JS behavior).

Poem

🐰 I hopped from jQuery to session light,
Swapped waits that lingered in the night,
One helper gone, two helpers born,
Tests now wake with a fresher morn,
Hop-hop — the checks are tidy, bright.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Remove JQueryHelper' is concise and directly summarizes the main change in the changeset. The entire purpose of the PR is to remove the JQueryHelper class and migrate its functionality to DriverHelper, which is accurately captured by the title. The title is clear, specific, and allows a teammate scanning the repository history to quickly understand the primary objective.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 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.

@Rafikooo Rafikooo force-pushed the maintenance/remove-jquery-helper branch from be760ca to ed874bc Compare November 3, 2025 00:03
@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 3, 2025

❌ Preview Environment deleted from Bunnyshell

Available commands:

  • 🚀 /bns:deploy to redeploy the environment

@Rafikooo Rafikooo force-pushed the maintenance/remove-jquery-helper branch from aa45053 to 1c0b658 Compare November 3, 2025 00:30
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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed874bc and 1c0b658.

📒 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.php
  • src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php
  • 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/DriverHelper.php
  • src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php
  • src/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 waitForFormToStopLoading method correctly guards the wait call with a JavaScript driver check and follows the same pattern as waitForPageToLoad. 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::waitForAsynchronousActionsToFinish with DriverHelper::waitForAsynchronousActionsToFinish is consistent across all call sites. However, these calls depend on the fix for the critical issue in DriverHelper::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 JQueryHelper to DriverHelper is complete and correct. All call sites properly use DriverHelper::waitForFormToStopLoading($this->getSession()), and the method includes the necessary JavaScript driver check.

Also applies to: 66-66, 71-71, 76-76, 81-81

@Rafikooo Rafikooo changed the title Remove unused JQueryHelper Remove JQueryHelper Nov 3, 2025
@Rafikooo Rafikooo force-pushed the maintenance/remove-jquery-helper branch from 1c0b658 to cf6938f Compare November 3, 2025 10:36
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: 1

🧹 Nitpick comments (1)
src/Sylius/Behat/Service/AutocompleteHelper.php (1)

84-84: Optional: Fix property name typo in existing code.

The property name singleNodeStep appears to be a typo and should likely be singleNodeValue (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

📥 Commits

Reviewing files that changed from the base of the PR and between 1c0b658 and cf6938f.

📒 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

@GSadee GSadee added Behat Issues and PRs aimed at improving Behat usage. Maintenance CI configurations, READMEs, releases, etc. labels Nov 4, 2025
@GSadee GSadee merged commit 88de7f4 into Sylius:2.1 Nov 4, 2025
48 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Behat Issues and PRs aimed at improving Behat usage. Maintenance CI configurations, READMEs, releases, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants