[Behat] Remove final keyword from element classes#17700
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request updates a series of class declarations by removing the Changes
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Preview Environment deleted from BunnyshellAvailable commands:
|
|
@Rafikooo thank you very much for this usefull Pr, can you also check if there is some private properties and methods? |
835b137 to
1eb9b55
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/Sylius/Behat/Page/Admin/Order/UpdatePage.php (1)
138-146:⚠️ Potential issueFix 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 parentUpdatePage::getFieldElementreturnsNodeElementwithout allowing null.Apply this diff to fix the compatibility issue:
-protected function getFieldElement(string $element): ?NodeElement +protected function getFieldElement(string $element): NodeElementThen 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
$autocompleteHelperproperty from private to protected allows subclasses to access this dependency, improving the extensibility of this class.Consider adding the
readonlymodifier 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
$autocompleteHelperproperty from private to protected enables subclasses to access this dependency, which aligns with making the class structure more extensible.For consistency, consider adopting the
readonlymodifier for this property similar to other classes in the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 extensibilityRemoving the
finalkeyword allows this class to be extended, which increases its flexibility.
90-103: LGTM - Improves inheritance capabilitiesChanging this method from
privatetoprotectedallows subclasses to access and override this functionality, which is necessary for proper inheritance.
105-112: LGTM - Improves inheritance capabilitiesChanging this method from
privatetoprotectedallows 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
privatetoprotectedfor thewaitForElementmethod 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
$tableAccessorproperty fromprivatetoprotectedallows 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
privatetoprotectedfor thegetProvinceItemsmethod 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
privatetoprotectedfor thegetRowFormethod 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 extensibilityChanging the visibility from
privatetoprotectedmakes thewaitForElementmethod 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 extensibilityChanging the visibility from
privatetoprotectedmakes thewaitForElementToBeReadymethod 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 extensibilityChanging the visibility of the
$autocompleteHelperproperty fromprivatetoprotectedin 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 extensibilityChanging the visibility from
privatetoprotectedmakes thewaitForComponentsUpdatemethod 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
privatetoprotectedfor thegetAvatarImagePath()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
privatetoprotectedfor thecheckCountryStatus()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
privatetoprotectedfor thegetProductsNames()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 readonlytoprotected readonlyfor the$tableAccessorproperty 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 appropriatelyThe visibility of the
getFieldElementmethod has been changed fromprivatetoprotected, 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 appropriatelyThe visibility of the
hasValueInCustomerSectionmethod has been changed fromprivatetoprotected, 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 appropriatelyThe visibility of the
$autocompleteHelperproperty has been changed fromprivatetoprotected, 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 appropriatelyThe visibility of the
getLastChoiceElementmethod has been changed fromprivatetoprotected, 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
$autocompleteHelperproperty 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
getSelectedOptionTextmethod 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
getPromotionFieldsWithHeaderfrom 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 theIndexPageclass.src/Sylius/Behat/Page/Shop/Page.php (1)
56-56: Enhanced abstract class extensibility.Changing the
getFieldElementmethod 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
$autocompleteHelperproperty 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
checkCurrencyStatusmethod 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
privatetoprotectedforwaitForStatisticsUpdate()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
privatetoprotectedforgetFieldElement()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
privatetoprotectedforgetValidationMessageFor()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
finalkeyword from theFormElementclass 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
privatetoprotectedforselectCalculatorConfigurationChannelTab()allows subclasses to access this method, which is useful for customizing shipping method form behavior in extensions. This properly complements the removal of thefinalkeyword from the class.src/Sylius/Behat/Element/Shop/Account/RegisterElement.php (2)
20-20: LGTM: Improved class extensibility by removingfinalkeyword.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
privatetoprotectedfor thegetFieldElementmethod 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
$tableAccessorand$routeNamefromprivate readonlytoprotected readonlymaintains 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 removingfinalkeyword.This change allows the
FormElementclass to be extended, which aligns with the PR objective of enhancing flexibility in the codebase.
26-26: LGTM: Enhanced property accessibility for inheritance.Changing
$autocompleteHelpervisibility fromprivate readonlytoprotected readonlymaintains 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
$imageExistenceCheckerand$autocompleteHelperfromprivatetoprotectedallows these properties to be accessed by subclasses, which enhances the extensibility of theIndexPageclass and is consistent with the changes made in other files.src/Sylius/Behat/Element/Admin/CatalogPromotion/FormElement.php (2)
22-22: Approved: Removal offinalkeyword enhances extensibilityThe removal of the
finalkeyword from theFormElementclass 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 extensibilityChanging the visibility of
$autocompleteHelperfrom private to protected allows subclasses to access this dependency, which is necessary for proper extension of the class. This change complements the removal of thefinalkeyword.src/Sylius/Behat/Page/Shop/Account/Order/ShowPage.php (2)
28-28: Approved: Property visibility change improves extensibilityChanging the visibility of
$tableAccessorfrom 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 capabilitiesChanging the visibility of the
hasAddressmethod 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 inheritanceChanging the visibility of
$autocompleteHelperfrom 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 extensibilityChanging the visibility of the
changeStatemethod 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 extensibilityChanging the visibility of the
getProvinceItemsmethod 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 inheritanceChanging the visibility of the
getProvinceElementmethod 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 extensibilityThe 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 extensibilityChanging 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 extensibilityThe 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 extensibilityThis 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 extensibilityThe 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 extensibilityThis 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 extensibilityThe 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 extensibilityThis 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 extensibleRemoving the
finalkeyword allows this class to be extended, which enhances flexibility for customization scenarios.
34-34: Appropriate visibility changeChanging the visibility of
$autocompleteHelperfrom private to protected allows subclasses to access this property, which is necessary for proper inheritance.
128-128: Good visibility adjustmentMaking the
expandTranslationAccordionmethod 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 inheritanceThe
$routeNameproperty is now protected, allowing subclasses to access this important configuration value, which is crucial for proper inheritance scenarios.
127-127: Good accessibility improvementMaking the
getFieldElementmethod 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 adjustmentChanging
$routeNameto protected allows subclasses to access this important property, which is consistent with good inheritance practices.
116-116: Accessibility improvement for inheritanceMaking the
getFieldElementmethod 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 extensionMaking the
$autocompleteHelperproperty protected allows subclasses to access this helper, which is essential for proper inheritance.
60-60: Appropriate method visibility adjustmentChanging the
specifyAutocompleteFiltermethod 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
$autocompleteHelperproperty fromprivatetoprotectednow allows subclasses to access this property, enhancing extensibility.
107-115: LGTM - Enhanced extensibility by exposing utility method.Making the
getFieldmethodprotectedinstead ofprivateallows 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
privatetoprotectedforgetOrderLinkForRowmethod 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
$summaryPageproperty fromprivatetoprotectedwhile maintaining thereadonlymodifier is appropriate for better inheritance support.
385-390: LGTM - Enhanced method extensibility.Making the
waitForElementToBeReadymethodprotectedallows 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
getFieldElementfromprivatetoprotectedallows 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
privatetoprotectedfor the$autocompleteHelperproperty maintains thereadonlyattribute while allowing subclasses to access this dependency.
101-108: LGTM - Improved method extensibility.The visibility change for the
changeTabmethod fromprivatetoprotectedallows 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
specifyAddressmethodprotectedinstead ofprivateallows subclasses to leverage or customize this address specification logic.
91-94: LGTM - Improved helper method extensibility.The visibility change for
getOptionTextsFormethod is appropriate for better inheritance support.
122-125: LGTM - Enhanced method accessibility for subclasses.Changing
specifyElementValuemethod toprotectedvisibility enables derived classes to use this helper method.
130-133: LGTM - Improved utility method accessibility.Making the
chooseCountrymethodprotectedinstead ofprivateenhances 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
finalkeyword 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
$autocompleteHelperfromprivatetoprotectedallows 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
$tableAccessorfrom 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
$addressFactoryfrom 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.
1eb9b55 to
86d1b22
Compare
There was a problem hiding this comment.
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
$elementparameter 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
📒 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
getFieldElementfrom private to protected with an additional optional parameter array enhances extensibility in two ways:
- Subclasses can now access and override this method
- The new parameter allows for more flexible element targeting when extending this class
This change aligns well with the PR objective of removing the
finalkeyword and making classes more extensible.src/Sylius/Behat/Page/Shop/Checkout/AddressPage.php (8)
37-37: Make the property accessible for inheritance.Changing
$addressFactoryfrom 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
getFieldElementin a try-catch block ensures that missing elements are handled gracefully, improving readability and reliability.
321-321: Improved extensibility for option retrieval.By making
getOptionsFromSelectprotected, subclasses can now override or reuse this functionality with ease.
330-330: Good approach to retrieving a prefabricated address.Exposing
getPreFilledAddressas 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
specifyAddressfosters more customized address handling in descendant classes.
385-385: Consistent wait pattern.
waitForLoginActionproperly uses Mink’s wait feature to detect when the login form disappears, improving test stability.
390-390: Well-scoped validation logic.
assertAddressTypeprovides a clear guard for valid address types, avoiding silent misconfigurations.
397-397: Enabling granular address selection logic.Exposing
chooseDifferentAddressas 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 { |
There was a problem hiding this comment.
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.
| $validationElement = $this->getDocument()->waitFor(3, function (): NodeElement { | |
| $validationElement = $this->getDocument()->waitFor(3, function (): ?NodeElement { |
19025fa to
4680d73
Compare
Summary by CodeRabbit