Skip to content

[Behat] Remove final keyword from element classes#17700

Merged
GSadee merged 8 commits intoSylius:2.0from
Rafikooo:unfinal-elements
Mar 3, 2025
Merged

[Behat] Remove final keyword from element classes#17700
GSadee merged 8 commits intoSylius:2.0from
Rafikooo:unfinal-elements

Conversation

@Rafikooo
Copy link
Copy Markdown
Contributor

@Rafikooo Rafikooo commented Feb 27, 2025

Q A
Branch? 2.0
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
License MIT

Summary by CodeRabbit

  • Refactor
    • Improved component flexibility by relaxing restrictions on class extension across multiple classes. These under-the-hood changes maintain existing functionality while paving the way for enhanced customizability and future enhancements.

@Rafikooo Rafikooo requested review from a team as code owners February 27, 2025 11:35
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 27, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request updates a series of class declarations by removing the final keyword across multiple files. With this change, all affected classes can now be extended, enhancing the flexibility of the codebase. Additionally, several methods and properties have had their visibility changed from private to protected, allowing for greater accessibility in subclasses. No other modifications to methods, properties, or logic have been made.

Changes

File(s) Change Summary
src/.../Admin/Account/ResetElement.php, src/.../Admin/CatalogPromotion/{FilterElement.php, FormElement.php}, src/.../Admin/Channel/{DiscountedProductsCheckingPeriodInputElement.php, ExcludeTaxonsFromShowingLowestPriceInputElement.php, LowestPriceFlagElement.php, ShippingAddressInCheckoutRequiredElement.php, ShopBillingDataElement.php}, src/.../Admin/{Currency, CustomerGroup, ExchangeRate, Locale}/FormElement.php, src/.../Admin/NotificationsElement.php, src/.../Admin/Product/{AssociationsFormElement.php, AttributesFormElement.php, ChannelPricingsFormElement.php, MediaFormElement.php, TaxonomyFormElement.php, TranslationsFormElement.php}, src/.../Admin/Promotion/FormElement.php, src/.../Admin/PromotionCoupon/FormElement.php, src/.../Admin/ShippingMethod/FormElement.php, src/.../Admin/TaxCategory/FormElement.php, src/.../Admin/Taxon/{FormElement.php, ImageFormElement.php, TreeElement.php}, src/.../Admin/TopBarElement.php Removed final keyword to allow subclassing.
src/.../Element/BrowserElement.php, src/.../Product/IndexPage/VerticalMenuElement.php, src/.../Product/ShowPage/{AssociationsElement.php, AttributesElement.php, DetailsElement.php, LowestPriceInformationElement.php, MediaElement.php, OptionsElement.php, PricingElement.php, ShippingElement.php, TaxonomyElement.php, TranslationsElement.php, VariantsElement.php} Removed final keyword to enable extension.
src/.../Shop/Account/RegisterElement.php, src/.../Shop/CartWidgetElement.php, src/.../Shop/CheckoutSubtotalElement.php, src/.../Shop/MenuElement.php Removed final keyword to permit subclassing.
src/.../Page/Shop/Account/RegisterThankYouPage.php, src/.../Page/TestPlugin/MainPage.php Removed final keyword to allow class extension.
src/.../Page/Admin/{Administrator/UpdatePage.php, Channel/CreatePage.php, Channel/FormTrait.php, Channel/UpdatePage.php, Country/CreatePage.php, Country/IndexPage.php, Country/UpdatePage.php, Crud/CreatePage.php, Crud/IndexPage.php, Crud/UpdatePage.php, Currency/IndexPage.php, Customer/IndexPage.php, DashboardPage.php, Inventory/IndexPage.php, Order/IndexPage.php, Order/ShowPage.php, Order/UpdatePage.php, Payment/IndexPage.php, PaymentRequest/IndexPage.php, ProductReview/IndexPage.php, Promotion/CreatePage.php, Promotion/IndexPage.php, Shipment/IndexPage.php, ShippingMethod/IndexPage.php} Changed method/property visibility from private to protected to enhance accessibility for subclasses.
src/.../Page/Shop/{Account/AddressBook/UpdatePage.php, Account/DashboardPage.php, Account/Order/IndexPage.php, Account/Order/ShowPage.php, Account/ProfileUpdatePage.php, Cart/SummaryPage.php, Checkout/AddressPage.php, Checkout/CompletePage.php, HomePage.php, Page.php, Product/ShowPage.php, ProductReview/CreatePage.php} Changed method/property visibility from private to protected to enhance accessibility for subclasses.

Poem

Hop, hop, hop, I'm a coding hare,
Final's no more, now classes share,
With each removed key, new paths unfold,
My nibbling code leaves old bounds cold,
In fields of class, I bounce with glee,
A joyful rabbit set at free!


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 27, 2025

❌ Preview Environment deleted from Bunnyshell

Available commands:

  • 🚀 /bns:deploy to redeploy the environment

@Prometee
Copy link
Copy Markdown
Contributor

@Rafikooo thank you very much for this usefull Pr, can you also check if there is some private properties and methods?

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: 0

🔭 Outside diff range comments (1)
src/Sylius/Behat/Page/Admin/Order/UpdatePage.php (1)

138-146: ⚠️ Potential issue

Fix return type compatibility issue.

The pipeline is failing because this method's return type (?NodeElement) is incompatible with the parent class's method signature. The parent UpdatePage::getFieldElement returns NodeElement without allowing null.

Apply this diff to fix the compatibility issue:

-protected function getFieldElement(string $element): ?NodeElement
+protected function getFieldElement(string $element): NodeElement

Then update the method implementation to ensure it never returns null (add appropriate error handling if needed).

🧰 Tools
🪛 GitHub Actions: Continuous Integration (Minimal)

[error] 138-138: PHP Fatal error: Declaration of Sylius\Behat\Page\Admin\Order\UpdatePage::getFieldElement(string $element): ?Behat\Mink\Element\NodeElement must be compatible with Sylius\Behat\Page\Admin\Crud\UpdatePage::getFieldElement(string $element): Behat\Mink\Element\NodeElement

🧹 Nitpick comments (2)
src/Sylius/Behat/Page/Admin/Product/UpdateConfigurableProductPage.php (1)

38-38: Property visibility change improves inheritance capabilities.

Changing the $autocompleteHelper property from private to protected allows subclasses to access this dependency, improving the extensibility of this class.

Consider adding the readonly modifier for consistency with other similar property changes in the codebase (like in UpdateSimpleProductPage).

src/Sylius/Behat/Page/Admin/Inventory/IndexPage.php (1)

30-30: Property visibility modification enhances extensibility.

Changing the $autocompleteHelper property from private to protected enables subclasses to access this dependency, which aligns with making the class structure more extensible.

For consistency, consider adopting the readonly modifier for this property similar to other classes in the codebase.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 835b137 and 1eb9b55.

📒 Files selected for processing (91)
  • src/Sylius/Behat/Element/Admin/Account/ResetElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/CatalogPromotion/FilterElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/CatalogPromotion/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Channel/DiscountedProductsCheckingPeriodInputElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Channel/ExcludeTaxonsFromShowingLowestPriceInputElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Channel/LowestPriceFlagElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Channel/ShippingAddressInCheckoutRequiredElement.php (3 hunks)
  • src/Sylius/Behat/Element/Admin/Channel/ShopBillingDataElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Crud/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Currency/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/CustomerGroup/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/ExchangeRate/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Locale/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/NotificationsElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Product/AssociationsFormElement.php (2 hunks)
  • src/Sylius/Behat/Element/Admin/Product/AttributesFormElement.php (4 hunks)
  • src/Sylius/Behat/Element/Admin/Product/ChannelPricingsFormElement.php (3 hunks)
  • src/Sylius/Behat/Element/Admin/Product/MediaFormElement.php (2 hunks)
  • src/Sylius/Behat/Element/Admin/Product/TaxonomyFormElement.php (2 hunks)
  • src/Sylius/Behat/Element/Admin/Product/TranslationsFormElement.php (3 hunks)
  • src/Sylius/Behat/Element/Admin/ProductAttribute/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Promotion/FormElement.php (3 hunks)
  • src/Sylius/Behat/Element/Admin/PromotionCoupon/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/ShippingMethod/FormElement.php (2 hunks)
  • src/Sylius/Behat/Element/Admin/TaxCategory/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Taxon/FormElement.php (2 hunks)
  • src/Sylius/Behat/Element/Admin/Taxon/ImageFormElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/Taxon/TreeElement.php (1 hunks)
  • src/Sylius/Behat/Element/Admin/TopBarElement.php (2 hunks)
  • src/Sylius/Behat/Element/Admin/Zone/FormElement.php (1 hunks)
  • src/Sylius/Behat/Element/BrowserElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/IndexPage/VerticalMenuElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/AssociationsElement.php (2 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/AttributesElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/DetailsElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/LowestPriceInformationElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/MediaElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/OptionsElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/PricingElement.php (2 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/ShippingElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/TaxonomyElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/TranslationsElement.php (1 hunks)
  • src/Sylius/Behat/Element/Product/ShowPage/VariantsElement.php (2 hunks)
  • src/Sylius/Behat/Element/Shop/Account/RegisterElement.php (2 hunks)
  • src/Sylius/Behat/Element/Shop/CartWidgetElement.php (1 hunks)
  • src/Sylius/Behat/Element/Shop/CheckoutSubtotalElement.php (1 hunks)
  • src/Sylius/Behat/Element/Shop/MenuElement.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Administrator/UpdatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Channel/CreatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Channel/FormTrait.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Channel/UpdatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Country/CreatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Country/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Country/UpdatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Crud/CreatePage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Crud/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Crud/UpdatePage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Currency/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Customer/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/DashboardPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Inventory/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Order/IndexPage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Order/ShowPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Order/UpdatePage.php (3 hunks)
  • src/Sylius/Behat/Page/Admin/Payment/IndexPage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Payment/PaymentRequest/IndexPage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Product/CreateConfigurableProductPage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Product/CreateSimpleProductPage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Product/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Product/ShowPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Product/UpdateConfigurableProductPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Product/UpdateSimpleProductPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/ProductReview/IndexPage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Promotion/CreatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Promotion/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Admin/Shipment/IndexPage.php (3 hunks)
  • src/Sylius/Behat/Page/Admin/ShippingMethod/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Account/DashboardPage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Account/Order/IndexPage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Account/Order/ShowPage.php (2 hunks)
  • src/Sylius/Behat/Page/Shop/Account/ProfileUpdatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Account/RegisterThankYouPage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Cart/SummaryPage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php (6 hunks)
  • src/Sylius/Behat/Page/Shop/Checkout/CompletePage.php (4 hunks)
  • src/Sylius/Behat/Page/Shop/HomePage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Page.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Product/ShowPage.php (3 hunks)
  • src/Sylius/Behat/Page/Shop/ProductReview/CreatePage.php (1 hunks)
  • src/Sylius/Behat/Page/TestPlugin/MainPage.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (39)
  • src/Sylius/Behat/Element/Product/ShowPage/DetailsElement.php
  • src/Sylius/Behat/Element/Product/IndexPage/VerticalMenuElement.php
  • src/Sylius/Behat/Element/Admin/ExchangeRate/FormElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/AttributesElement.php
  • src/Sylius/Behat/Element/Admin/Channel/ShopBillingDataElement.php
  • src/Sylius/Behat/Page/Shop/Account/RegisterThankYouPage.php
  • src/Sylius/Behat/Element/Product/ShowPage/TranslationsElement.php
  • src/Sylius/Behat/Element/Shop/CartWidgetElement.php
  • src/Sylius/Behat/Element/BrowserElement.php
  • src/Sylius/Behat/Element/Admin/Taxon/ImageFormElement.php
  • src/Sylius/Behat/Element/Admin/TaxCategory/FormElement.php
  • src/Sylius/Behat/Element/Shop/CheckoutSubtotalElement.php
  • src/Sylius/Behat/Element/Admin/CustomerGroup/FormElement.php
  • src/Sylius/Behat/Element/Admin/Channel/LowestPriceFlagElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/MediaElement.php
  • src/Sylius/Behat/Element/Admin/Taxon/TreeElement.php
  • src/Sylius/Behat/Page/TestPlugin/MainPage.php
  • src/Sylius/Behat/Element/Admin/PromotionCoupon/FormElement.php
  • src/Sylius/Behat/Element/Admin/CatalogPromotion/FilterElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/TaxonomyElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/ShippingElement.php
  • src/Sylius/Behat/Element/Admin/Channel/DiscountedProductsCheckingPeriodInputElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/AssociationsElement.php
  • src/Sylius/Behat/Element/Admin/TopBarElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/LowestPriceInformationElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/OptionsElement.php
  • src/Sylius/Behat/Element/Admin/Locale/FormElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/PricingElement.php
  • src/Sylius/Behat/Element/Admin/NotificationsElement.php
  • src/Sylius/Behat/Element/Admin/Product/MediaFormElement.php
  • src/Sylius/Behat/Element/Admin/Product/ChannelPricingsFormElement.php
  • src/Sylius/Behat/Element/Admin/Channel/ExcludeTaxonsFromShowingLowestPriceInputElement.php
  • src/Sylius/Behat/Element/Admin/Product/TaxonomyFormElement.php
  • src/Sylius/Behat/Element/Admin/Product/AssociationsFormElement.php
  • src/Sylius/Behat/Element/Admin/Account/ResetElement.php
  • src/Sylius/Behat/Element/Product/ShowPage/VariantsElement.php
  • src/Sylius/Behat/Element/Admin/Channel/ShippingAddressInCheckoutRequiredElement.php
  • src/Sylius/Behat/Element/Admin/Product/AttributesFormElement.php
  • src/Sylius/Behat/Element/Shop/MenuElement.php
🧰 Additional context used
🪛 GitHub Actions: Continuous Integration (Minimal)
src/Sylius/Behat/Page/Admin/Order/UpdatePage.php

[error] 138-138: PHP Fatal error: Declaration of Sylius\Behat\Page\Admin\Order\UpdatePage::getFieldElement(string $element): ?Behat\Mink\Element\NodeElement must be compatible with Sylius\Behat\Page\Admin\Crud\UpdatePage::getFieldElement(string $element): Behat\Mink\Element\NodeElement

🔇 Additional comments (94)
src/Sylius/Behat/Element/Admin/Product/TranslationsFormElement.php (3)

19-19: LGTM - Enhances extensibility

Removing the final keyword allows this class to be extended, which increases its flexibility.


90-103: LGTM - Improves inheritance capabilities

Changing this method from private to protected allows subclasses to access and override this functionality, which is necessary for proper inheritance.


105-112: LGTM - Improves inheritance capabilities

Changing this method from private to protected allows subclasses to access and override this functionality, which is necessary for proper inheritance.

src/Sylius/Behat/Element/Admin/Zone/FormElement.php (1)

113-123: LGTM! Good visibility change for extensibility.

Changing the visibility from private to protected for the waitForElement method enables subclasses to reuse this functionality, which aligns with the PR's goal of improving extensibility. The method is well-implemented with clear parameter typing and return type declarations.

src/Sylius/Behat/Page/Shop/Account/Order/IndexPage.php (1)

28-28: LGTM! Appropriate visibility change for constructor property.

Changing the visibility of the $tableAccessor property from private to protected allows subclasses to access this dependency directly, enhancing the extensibility of this class.

src/Sylius/Behat/Page/Admin/Country/CreatePage.php (1)

63-69: LGTM! Good visibility change for extensibility.

Changing the visibility from private to protected for the getProvinceItems method is consistent with the PR's goal of improving class extensibility. This allows subclasses to reuse this functionality without having to reimplement it.

src/Sylius/Behat/Page/Admin/ShippingMethod/IndexPage.php (1)

69-72: LGTM! Good visibility change for extensibility.

Changing the visibility from private to protected for the getRowFor method enables subclasses to leverage this functionality, which is consistent with the goal of enhancing class extensibility in this PR.

src/Sylius/Behat/Page/Shop/Account/AddressBook/UpdatePage.php (1)

120-120: Access modifier change improves extensibility

Changing the visibility from private to protected makes the waitForElement method accessible to subclasses. This is a good change that promotes better inheritance capabilities and will allow future extension and customization of this page object.

src/Sylius/Behat/Page/Shop/Account/ProfileUpdatePage.php (1)

91-91: Access modifier change improves extensibility

Changing the visibility from private to protected makes the waitForElementToBeReady method accessible to subclasses. This aligns with the PR's objective to improve extensibility and will allow for better inheritance capabilities.

src/Sylius/Behat/Page/Admin/Channel/CreatePage.php (1)

39-39: Access modifier change improves extensibility

Changing the visibility of the $autocompleteHelper property from private to protected in the constructor parameter declaration makes this dependency accessible to subclasses. This is a good enhancement that allows for better inheritance and potential overriding in custom implementations.

src/Sylius/Behat/Page/Shop/Cart/SummaryPage.php (1)

254-254: Access modifier change improves extensibility

Changing the visibility from private to protected makes the waitForComponentsUpdate method accessible to subclasses. This is a positive change that promotes better inheritance capabilities and allows for potential customization of this functionality in extended classes.

src/Sylius/Behat/Page/Admin/Administrator/UpdatePage.php (1)

47-47: Visibility change enhances extensibility.

The change from private to protected for the getAvatarImagePath() method allows subclasses to access or override this method, which aligns with the PR's goal of improving extensibility.

src/Sylius/Behat/Page/Admin/Country/IndexPage.php (1)

31-31: Visibility change enhances extensibility.

The change from private to protected for the checkCountryStatus() method allows subclasses to access or override this method, which aligns with the PR's goal of improving extensibility.

src/Sylius/Behat/Page/Shop/HomePage.php (1)

116-116: Visibility change enhances extensibility.

The change from private to protected for the getProductsNames() method allows subclasses to access or override this method, which aligns with the PR's goal of improving extensibility.

src/Sylius/Behat/Page/Admin/Order/ShowPage.php (1)

30-30: Visibility change enhances extensibility.

The change from private readonly to protected readonly for the $tableAccessor property allows subclasses to access this property, which aligns with the PR's goal of improving extensibility.

src/Sylius/Behat/Page/Admin/Promotion/CreatePage.php (1)

51-51: Visibility modifier changed appropriately

The visibility of the getFieldElement method has been changed from private to protected, which allows subclasses to access this method. This change aligns with the PR objective of improving extensibility by removing restrictions on extending these classes.

src/Sylius/Behat/Page/Shop/Account/DashboardPage.php (1)

59-59: Visibility modifier changed appropriately

The visibility of the hasValueInCustomerSection method has been changed from private to protected, which allows subclasses to access this method. This change enhances extensibility by enabling derived classes to reuse this functionality.

src/Sylius/Behat/Page/Admin/Customer/IndexPage.php (1)

31-31: Visibility modifier changed appropriately

The visibility of the $autocompleteHelper property has been changed from private to protected, which allows subclasses to access this property. This change enhances extensibility and is consistent with similar changes in other files within this PR.

src/Sylius/Behat/Element/Admin/ProductAttribute/FormElement.php (1)

112-112: Visibility modifier changed appropriately

The visibility of the getLastChoiceElement method has been changed from private to protected, which allows subclasses to access this method. This change is consistent with the PR objective of enhancing extensibility across element classes.

src/Sylius/Behat/Page/Admin/Channel/UpdatePage.php (1)

35-35: Enhanced extensibility with protected property.

Changing the $autocompleteHelper property from private to protected aligns with the PR objective of improving extensibility. This allows subclasses to access this helper directly, making it easier to extend the functionality of this class.

src/Sylius/Behat/Page/Admin/Channel/FormTrait.php (1)

129-129: Improved trait extensibility.

Changing the getSelectedOptionText method from private to protected is a good improvement. This allows classes using this trait to access and potentially override this method, enhancing the extensibility of the codebase as intended by the PR.

src/Sylius/Behat/Page/Admin/Promotion/IndexPage.php (1)

71-71: Better inheritance support with protected method.

Changing getPromotionFieldsWithHeader from private to protected is appropriate and aligns with the PR's goal of improving extensibility. This change allows subclasses to access and potentially customize this functionality when extending the IndexPage class.

src/Sylius/Behat/Page/Shop/Page.php (1)

56-56: Enhanced abstract class extensibility.

Changing the getFieldElement method from private to protected is particularly valuable in this abstract class. This modification enables concrete page implementations to access this helper method, facilitating better code reuse and customization in derived classes.

src/Sylius/Behat/Page/Admin/Product/UpdateSimpleProductPage.php (1)

35-35: Visibility change enhances extensibility.

Changing the $autocompleteHelper property from private to protected allows subclasses to access this dependency while maintaining its immutability with the readonly modifier. This aligns with the PR's goal of making element classes more extensible.

src/Sylius/Behat/Page/Admin/Currency/IndexPage.php (1)

34-34: Method visibility change improves class extensibility.

Changing the checkCurrencyStatus method from private to protected enables subclasses to access and potentially override this method, enhancing the extensibility of the class.

src/Sylius/Behat/Page/Admin/DashboardPage.php (1)

158-163: LGTM: Good change to improve extensibility.

Changing the visibility from private to protected for waitForStatisticsUpdate() allows subclasses to access this method, which enhances extensibility while maintaining proper encapsulation. This aligns with the PR objective of making the codebase more flexible for extension.

src/Sylius/Behat/Element/Admin/Crud/FormElement.php (1)

82-90: LGTM: Good change to improve extensibility.

Changing the visibility from private to protected for getFieldElement() allows subclasses to access this method, which is important for extending form functionality in custom implementations. This change aligns with the PR objective of enhancing extensibility.

src/Sylius/Behat/Page/Shop/ProductReview/CreatePage.php (1)

88-94: LGTM: Good change to improve extensibility.

Changing the visibility from private to protected for getValidationMessageFor() allows subclasses to access this validation method, which is beneficial for specialized product review page implementations. This change aligns with the PR objective of enhancing extensibility.

src/Sylius/Behat/Element/Admin/ShippingMethod/FormElement.php (2)

20-20: LGTM: Appropriate removal of final keyword.

Removing the final keyword from the FormElement class allows it to be extended, enabling customization via inheritance. This is in line with the PR objective of making element classes extensible.


173-180: LGTM: Good change to improve extensibility.

Changing the visibility from private to protected for selectCalculatorConfigurationChannelTab() allows subclasses to access this method, which is useful for customizing shipping method form behavior in extensions. This properly complements the removal of the final keyword from the class.

src/Sylius/Behat/Element/Shop/Account/RegisterElement.php (2)

20-20: LGTM: Improved class extensibility by removing final keyword.

The change allows the class to be extended, improving the flexibility of the codebase while maintaining its functionality.


117-117: LGTM: Improved method accessibility for subclasses.

Changing visibility from private to protected for the getFieldElement method allows subclasses to reuse this functionality, which aligns well with the extensibility improvements in this PR.

src/Sylius/Behat/Page/Admin/Crud/IndexPage.php (1)

30-31: LGTM: Enhanced inheritance support with protected property visibility.

Changing the visibility of $tableAccessor and $routeName from private readonly to protected readonly maintains immutability while allowing subclasses to access these properties, which is essential for extending the class's functionality.

src/Sylius/Behat/Element/Admin/Currency/FormElement.php (2)

21-21: LGTM: Improved class extensibility by removing final keyword.

This change allows the FormElement class to be extended, which aligns with the PR objective of enhancing flexibility in the codebase.


26-26: LGTM: Enhanced property accessibility for inheritance.

Changing $autocompleteHelper visibility from private readonly to protected readonly maintains immutability while allowing subclasses to access this property, which is valuable for extending functionality.

src/Sylius/Behat/Page/Admin/Product/IndexPage.php (1)

31-32: LGTM: Improved property accessibility for subclasses.

Changing the visibility of $imageExistenceChecker and $autocompleteHelper from private to protected allows these properties to be accessed by subclasses, which enhances the extensibility of the IndexPage class and is consistent with the changes made in other files.

src/Sylius/Behat/Element/Admin/CatalogPromotion/FormElement.php (2)

22-22: Approved: Removal of final keyword enhances extensibility

The removal of the final keyword from the FormElement class declaration allows this class to be extended, which aligns with the PR's objective of enhancing the extensibility of element classes.


27-27: Approved: Property visibility change enhances extensibility

Changing the visibility of $autocompleteHelper from private to protected allows subclasses to access this dependency, which is necessary for proper extension of the class. This change complements the removal of the final keyword.

src/Sylius/Behat/Page/Shop/Account/Order/ShowPage.php (2)

28-28: Approved: Property visibility change improves extensibility

Changing the visibility of $tableAccessor from private to protected allows subclasses to access this dependency, which improves the class's extensibility. This is a good practice when classes are designed to be extended.


180-180: Approved: Method visibility change enhances override capabilities

Changing the visibility of the hasAddress method from private to protected enables subclasses to override or reuse this functionality, which is essential for proper inheritance. This change aligns well with making the class more extensible.

src/Sylius/Behat/Page/Admin/ProductReview/IndexPage.php (2)

31-31: Approved: Property visibility change supports inheritance

Changing the visibility of $autocompleteHelper from private to protected allows subclasses to access this dependency, which is consistent with the rest of the changes in this PR that aim to improve extensibility.


67-67: Approved: Method visibility change improves extensibility

Changing the visibility of the changeState method from private to protected allows subclasses to override or reuse this method, which is beneficial for creating specialized versions of this class through inheritance.

src/Sylius/Behat/Page/Admin/Country/UpdatePage.php (2)

127-127: Approved: Method visibility change enhances extensibility

Changing the visibility of the getProvinceItems method from private to protected allows subclasses to access and potentially override this method, which improves the extensibility of the class.


135-135: Approved: Method visibility change supports inheritance

Changing the visibility of the getProvinceElement method from private to protected enables subclasses to reuse or extend this functionality, which is essential for creating specialized versions of this page through inheritance.

src/Sylius/Behat/Page/Admin/Product/ShowPage.php (2)

113-113: Visibility changed from private to protected to enhance extensibility

The visibility change of this method allows subclasses to override or access the catalog promotions retrieval logic, which improves the extension capabilities of this class.


120-120: Visibility changed from private to protected to enhance extensibility

Changing this method's visibility from private to protected allows subclasses to access or override the pricing row retrieval logic, which is consistent with the PR's goal of making element classes more extensible.

src/Sylius/Behat/Page/Admin/Order/IndexPage.php (2)

31-31: Visibility changed from private to protected to enhance extensibility

The visibility change for this property allows subclasses to access the autocomplete helper directly, which is aligned with the PR's goal of improving extensibility of element classes.


104-104: Visibility changed from private to protected to enhance extensibility

This method's visibility change allows subclasses to reuse or override the autocomplete filter functionality, which is important for customizing filtering behavior in specialized index pages.

src/Sylius/Behat/Page/Admin/Product/CreateSimpleProductPage.php (2)

39-39: Visibility changed from private to protected to enhance extensibility

The visibility change for this property maintains the readonly attribute while making it accessible to subclasses, which is consistent with the PR's goal of improving extensibility.


68-68: Visibility changed from private to protected to enhance extensibility

This method's visibility change allows subclasses to reuse or override the tab switching functionality, which is essential for customizations that need to interact with different sections of the product form.

src/Sylius/Behat/Page/Admin/Payment/IndexPage.php (2)

76-76: Visibility changed from private to protected to enhance extensibility

The visibility change allows subclasses to access or override the order link retrieval logic, which is aligned with the PR's goal of making element classes more extensible.


86-86: Visibility changed from private to protected to enhance extensibility

This method's visibility change allows subclasses to reuse or override the field retrieval logic, which is important for specialized payment pages that need to interact with table elements in a customized way.

src/Sylius/Behat/Element/Admin/Taxon/FormElement.php (3)

26-26: LGTM: Class now extensible

Removing the final keyword allows this class to be extended, which enhances flexibility for customization scenarios.


34-34: Appropriate visibility change

Changing the visibility of $autocompleteHelper from private to protected allows subclasses to access this property, which is necessary for proper inheritance.


128-128: Good visibility adjustment

Making the expandTranslationAccordion method protected instead of private is appropriate, as it allows subclasses to leverage this functionality when extending the class.

src/Sylius/Behat/Page/Admin/Crud/UpdatePage.php (2)

32-32: Appropriate visibility change for inheritance

The $routeName property is now protected, allowing subclasses to access this important configuration value, which is crucial for proper inheritance scenarios.


127-127: Good accessibility improvement

Making the getFieldElement method protected allows subclasses to utilize this method, enhancing the extensibility of the class while maintaining encapsulation from external code.

src/Sylius/Behat/Page/Admin/Crud/CreatePage.php (2)

31-31: Proper visibility adjustment

Changing $routeName to protected allows subclasses to access this important property, which is consistent with good inheritance practices.


116-116: Accessibility improvement for inheritance

Making the getFieldElement method protected rather than private allows subclasses to leverage this utility method, which enhances the extensibility of the class.

src/Sylius/Behat/Page/Admin/Payment/PaymentRequest/IndexPage.php (2)

31-31: Good visibility change for extension

Making the $autocompleteHelper property protected allows subclasses to access this helper, which is essential for proper inheritance.


60-60: Appropriate method visibility adjustment

Changing the specifyAutocompleteFilter method from private to protected improves extensibility by allowing subclasses to access and override this functionality if needed.

src/Sylius/Behat/Page/Admin/Shipment/IndexPage.php (3)

31-31: LGTM - Improved extensibility by changing property visibility.

Changing the visibility of the $autocompleteHelper property from private to protected now allows subclasses to access this property, enhancing extensibility.


107-115: LGTM - Enhanced extensibility by exposing utility method.

Making the getField method protected instead of private allows subclasses to leverage this utility method, which is beneficial for extending functionality.


117-125: LGTM - Improved subclass access to helper method.

The visibility change from private to protected for getOrderLinkForRow method is appropriate and allows for better inheritance support.

src/Sylius/Behat/Page/Shop/Product/ShowPage.php (3)

33-33: LGTM - Improved property accessibility for subclasses.

Changing the $summaryPage property from private to protected while maintaining the readonly modifier is appropriate for better inheritance support.


385-390: LGTM - Enhanced method extensibility.

Making the waitForElementToBeReady method protected allows subclasses to access or override this behavior, which is useful for customizing wait behavior in derived page objects.


402-410: LGTM - Improved utility method accessibility.

Changing getFieldElement from private to protected allows subclasses to leverage this helper method for their own element location needs.

src/Sylius/Behat/Page/Admin/Product/CreateConfigurableProductPage.php (2)

39-39: LGTM - Enhanced property accessibility with readonly retained.

The change from private to protected for the $autocompleteHelper property maintains the readonly attribute while allowing subclasses to access this dependency.


101-108: LGTM - Improved method extensibility.

The visibility change for the changeTab method from private to protected allows subclasses to reuse or override this functionality as needed.

src/Sylius/Behat/Page/Admin/Order/UpdatePage.php (4)

37-46: LGTM - Enhanced utility method accessibility.

Making specifyAddress method protected instead of private allows subclasses to leverage or customize this address specification logic.


91-94: LGTM - Improved helper method extensibility.

The visibility change for getOptionTextsFor method is appropriate for better inheritance support.


122-125: LGTM - Enhanced method accessibility for subclasses.

Changing specifyElementValue method to protected visibility enables derived classes to use this helper method.


130-133: LGTM - Improved utility method accessibility.

Making the chooseCountry method protected instead of private enhances extensibility by allowing subclasses to access this functionality.

src/Sylius/Behat/Element/Admin/Promotion/FormElement.php (6)

23-23: Good change to enhance extensibility.

The removal of the final keyword from the class declaration allows for inheritance and extension of this class, which aligns with the PR objective.


28-28: Good visibility change for property inheritance.

Changing the visibility of $autocompleteHelper from private to protected allows subclasses to access this property, which is essential for proper inheritance.


247-250: Appropriately modified method visibility for inheritance.

Making getLastAction() method protected instead of private allows subclasses to access or override this method, enhancing extensibility.


252-261: Good visibility change for helper method.

Changing getChannelConfigurationOfLastAction() to protected allows subclasses to reuse this functionality, which is important for extending the class behavior.


263-266: Appropriate visibility modification for inheritance.

Making getLastRule() protected is consistent with the other visibility changes and supports the extensibility goal.


268-278: Good method visibility change for inheritance.

Changing getChannelConfigurationOfLastRule() to protected allows this utility method to be accessible by subclasses, ensuring proper extensibility.

src/Sylius/Behat/Page/Shop/Checkout/CompletePage.php (7)

33-33: Good visibility change for property inheritance.

Changing $tableAccessor from private to protected allows subclasses to access this essential property, which is consistent with the extensibility goal.


281-294: Appropriate visibility change for method inheritance.

Making isAddressValid() protected allows subclasses to access or override this validation logic, enhancing the extensibility of the class.


296-303: Good visibility modification for helper method.

Changing hasAddressPart() to protected allows derived classes to reuse this utility method, which is important for proper inheritance.


305-308: Appropriate visibility change for inheritance support.

Making getCountryName() protected allows subclasses to access this utility method, which is consistent with the PR's goal.


310-313: Good method visibility change for extensibility.

Changing getPriceFromString() to protected allows subclasses to access or override this parsing logic, enhancing the flexibility of the class.


315-320: Appropriate visibility modification for inheritance.

Making getTotalFromString() protected enables subclasses to access or modify this functionality, which supports the extensibility goal.


322-327: Good visibility change for helper method.

Changing getBaseTotalFromString() to protected allows derived classes to reuse this functionality, which is essential for proper inheritance.

src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php (8)

37-37: Good visibility change for property inheritance.

Changing $addressFactory from private to protected allows subclasses to access this property, which is consistent with the extensibility goal.


320-327: Appropriate visibility change for helper method.

Making getOptionsFromSelect() protected allows subclasses to access this utility method, enhancing the extensibility of the class.


329-351: Good method visibility change for inheritance.

Changing getPreFilledAddress() to protected allows derived classes to access or override this complex functionality, which is important for extending the class behavior.


353-372: Appropriate visibility modification for extensibility.

Making specifyAddress() protected enables subclasses to access or modify this core functionality, which supports proper inheritance.


374-382: Good visibility change for helper method.

Changing getFieldElement() to protected allows subclasses to reuse this utility method, which is essential for extending form interaction logic.


384-387: Appropriate visibility change for helper method.

Making waitForLoginAction() protected allows derived classes to access this wait logic, which is important for proper inheritance.


389-394: Good method visibility change for inheritance.

Changing assertAddressType() to protected allows subclasses to access this validation logic, enhancing the flexibility of the class.


396-401: Appropriate visibility modification for UI interaction method.

Making chooseDifferentAddress() protected enables subclasses to access or modify this UI interaction functionality, which supports the extensibility goal.

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/Page/Shop/Checkout/AddressPage.php (1)

375-377: Clarify variable naming to improve readability.

Overwriting the $element parameter can be slightly confusing. Consider using a different variable name within the method body for clarity.

-protected function getFieldElement(string $element, array $parameters = []): NodeElement
-{
-    $element = $this->getElement($element, $parameters);
+protected function getFieldElement(string $fieldName, array $parameters = []): NodeElement
+{
+    $element = $this->getElement($fieldName, $parameters);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 86d1b22 and 19025fa.

📒 Files selected for processing (7)
  • src/Sylius/Behat/Element/Product/ShowPage/PricingElement.php (3 hunks)
  • src/Sylius/Behat/Page/Admin/Crud/CreatePage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Crud/UpdatePage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Order/ShowPage.php (2 hunks)
  • src/Sylius/Behat/Page/Admin/Order/UpdatePage.php (3 hunks)
  • src/Sylius/Behat/Page/Admin/Promotion/CreatePage.php (1 hunks)
  • src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/Sylius/Behat/Page/Admin/Order/ShowPage.php
  • src/Sylius/Behat/Page/Admin/Crud/CreatePage.php
  • src/Sylius/Behat/Element/Product/ShowPage/PricingElement.php
  • src/Sylius/Behat/Page/Admin/Order/UpdatePage.php
  • src/Sylius/Behat/Page/Admin/Crud/UpdatePage.php
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Static checks / PHP 8.3, Symfony ^7.1
  • GitHub Check: Static checks / PHP 8.2, Symfony ^6.4
🔇 Additional comments (9)
src/Sylius/Behat/Page/Admin/Promotion/CreatePage.php (1)

51-53: Improved extensibility by making the method protected and adding parameter support.

The changes to getFieldElement from private to protected with an additional optional parameter array enhances extensibility in two ways:

  1. Subclasses can now access and override this method
  2. The new parameter allows for more flexible element targeting when extending this class

This change aligns well with the PR objective of removing the final keyword and making classes more extensible.

src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php (8)

37-37: Make the property accessible for inheritance.

Changing $addressFactory from private to protected is consistent with the PR's aim of removing final-like constraints and facilitating extension by subclasses.


90-91: Robust exception handling.

Wrapping getFieldElement in a try-catch block ensures that missing elements are handled gracefully, improving readability and reliability.


321-321: Improved extensibility for option retrieval.

By making getOptionsFromSelect protected, subclasses can now override or reuse this functionality with ease.


330-330: Good approach to retrieving a prefabricated address.

Exposing getPreFilledAddress as protected aligns with the PR’s aim of allowing subclasses to override or extend address-creation logic.


354-354: Address specification workflow is now extensible.

Allowing subclasses to override specifyAddress fosters more customized address handling in descendant classes.


385-385: Consistent wait pattern.

waitForLoginAction properly uses Mink’s wait feature to detect when the login form disappears, improving test stability.


390-390: Well-scoped validation logic.

assertAddressType provides a clear guard for valid address types, avoiding silent misconfigurations.


397-397: Enabling granular address selection logic.

Exposing chooseDifferentAddress as protected allows subclasses to tweak or extend the logic of toggling different address options.

{
/** @var NodeElement $validationElement */
$validationElement = $this->getDocument()->waitFor(3, function (): ?NodeElement {
$validationElement = $this->getDocument()->waitFor(3, function (): NodeElement {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix return type mismatch in closure.

The closure is declared to return NodeElement, but it can return null when an ElementNotFoundException is thrown. This can lead to runtime type errors.

Consider adjusting the return type to nullable:

-$validationElement = $this->getDocument()->waitFor(3, function (): NodeElement {
+$validationElement = $this->getDocument()->waitFor(3, function (): ?NodeElement {
📝 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.

Suggested change
$validationElement = $this->getDocument()->waitFor(3, function (): NodeElement {
$validationElement = $this->getDocument()->waitFor(3, function (): ?NodeElement {

@Rafikooo Rafikooo changed the base branch from 2.1 to 2.0 March 3, 2025 08:22
@GSadee GSadee added DX Issues and PRs aimed at improving Developer eXperience. Behat Issues and PRs aimed at improving Behat usage. labels Mar 3, 2025
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. DX Issues and PRs aimed at improving Developer eXperience.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants