Convert Behat annotations to PHP attributes in UI Admin contexts#18828
Conversation
📝 WalkthroughWalkthroughThis pull request systematically migrates Behat step declarations across approximately 50+ Admin UI context files from PHPDoc-style annotations ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
❗ Preview Environment delete from Bunnyshell failedCheck https://github.com/Sylius/Sylius/actions/runs/22102104804 for details. See: Environment Details Available commands:
|
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/Sylius/Behat/Context/Ui/Admin/NavigatingBetweenProductShowAndEditPagesContext.php (1)
109-116:⚠️ Potential issue | 🔴 CriticalFix two issues: wrong page object checked and redundant enum conversion
Two bugs need to be fixed:
Line 113 – wrong page object for simple product type: When
iWantToCreateANewProduct('simple')is called, it opens$this->createSimpleProductPage, not the update page. The assertion should check$this->createSimpleProductPage->hasShowPageButton()to match the configurable case which correctly checks$this->createConfigurableProductPage.Line 112 – redundant enum conversion:
$this->currentProductTypeis already aProductTypeEnum, so theProductTypeEnum::from($this->currentProductType->value)round-trip is unnecessary. Use$this->currentProductTypedirectly in the match expression.Proposed fix
public function iShouldNotBeAbleToAccessTheProductShowPage(): void { - match (ProductTypeEnum::from($this->currentProductType->value)) { - ProductTypeEnum::simple => Assert::false($this->updateSimpleProductPage->hasShowPageButton()), + match ($this->currentProductType) { + ProductTypeEnum::simple => Assert::false($this->createSimpleProductPage->hasShowPageButton()), ProductTypeEnum::configurable => Assert::false($this->createConfigurableProductPage->hasShowPageButton()), }; }src/Sylius/Behat/Context/Ui/Admin/ManagingShipmentsContext.php (1)
59-61:⚠️ Potential issue | 🔴 CriticalFix channel filter to merge with existing parameters instead of replacing them.
Line 60 replaces the entire
$parametersarray (losingorder,state, andcustomerkeys) when a channel is provided, instead of adding thechannelkey to the existing filters. This bug affects test assertions since scenarios likethe shipment of the "#00000001" order should be "Shipped" for "donald@duck.com" in "United States" channelwould pass as long as any shipment with the matching channel exists, regardless of order, state, or customer.if ($channel !== null) { - $parameters = ['channel' => $channel->getName()]; + $parameters['channel'] = $channel->getName(); }src/Sylius/Behat/Context/Ui/Admin/DashboardContext.php (1)
55-76:⚠️ Potential issue | 🟠 MajorStep-text regex does not match the available dashboard visualization methods.
The regex at line 55 allows
split by dayas the interval, which produces the step text "I view statistics for … year split by day". However, line 67 dispatches this tochooseMonthSplitByDaysInterval(), which provides monthly granularity split by days — not yearly data split by days. NochooseYearSplitByDaysInterval()method exists in the DashboardPageInterface. Update the regex to reflect the actual available visualizations: either(year split by month|month split by day)or add a missing dashboard page method for yearly daily granularity.src/Sylius/Behat/Context/Ui/Admin/ManagingInventoryContext.php (1)
36-43:⚠️ Potential issue | 🟠 MajorAdd type declarations to method parameters and return type.
The method
iFilterTrackedVariantsWithCodeContainingon line 37 lacks type declarations for parameters$field,$value, and the return type. Per coding guidelines, add:public function iFilterTrackedVariantsWithCodeContaining(string $field, string $value): voidThe method
iShouldSeeThatTheProductVariantHasQuantityOnHandon line 66 has the same issue and should be updated to:public function iShouldSeeThatTheProductVariantHasQuantityOnHand(string $productVariantName, string $quantity): voidsrc/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php (1)
87-96:⚠️ Potential issue | 🟡 MinorFix property type to match actual usage —
$indexPageshould beCustomerIndexPageInterface.The property is declared as
IndexPageInterface(line 38) but the code callssetFilterGroup()(line 92), which only exists onCustomerIndexPageInterface. While the constructor PHPDoc correctly hints atCustomerIndexPageInterface, the property type doesn't match the implementation or the static type contract. Change line 38 fromprivate IndexPageInterface $indexPagetoprivate CustomerIndexPageInterface $indexPage.
🤖 Fix all issues with AI agents
In `@src/Sylius/Behat/Context/Ui/Admin/LoginContext.php`:
- Around line 76-80: The step attribute on the assertion method
iShouldBeOnLoginPage is semantically wrong: change the attribute from #[Given('I
should be on login page')] to the appropriate #[Then('I should be on the login
page')] (or to match the existing #[Then] variant at the other definition) so
the assertion method uses a Then annotation; update the attribute string to
match the existing Then step text and remove or consolidate duplicate step
definitions if both methods now collide.
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php`:
- Around line 292-298: The Assert::true call in
ManagingCustomersContext::thisCustomerShouldBeEnabled passes a boolean as the
message argument; change it to call Assert::true with a proper boolean
expression and an optional string message (or no message) — e.g., replace the
current Assert::true($this->indexPage->isCustomerEnabled($customer), true) by
Assert::true($this->indexPage->isCustomerEnabled($customer)) or
Assert::true($this->indexPage->isCustomerEnabled($customer), 'Customer should be
enabled') so the second parameter is a string if present; refer to the
thisCustomerShouldBeEnabled method and indexPage->isCustomerEnabled to locate
the spot.
- Around line 320-324: The step definition declares two placeholders but the
method iSortTheOrderByField currently only accepts $field; add a second
parameter $sortType to the method signature (e.g., iSortTheOrderByField(string
$sortType, string $field): void) so Behat binds placeholders correctly, leaving
$sortType unused if not needed, and keep the existing call to
$this->ordersIndexPage->sort(ucfirst($field)).
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php`:
- Around line 627-632: The methods theOrderShouldHaveShippingState and
theOrderShouldHavePaymentState declare an OrderInterface $order parameter that
is not used; either remove the unused $order parameter if Behat doesn't require
it, or rename it to _order (or add a suppression annotation) to signal
intentional unused parameter for Behat regex argument matching—update both
method signatures accordingly and keep the rest of each method body unchanged.
- Around line 608-612: The step definition attribute for
thisOrderShipmentStateShouldBe declares two placeholders ('order :orderNumber
should have shipment state :shippingState') but the method signature only
accepts string $shippingState, causing a parameter mismatch; fix by aligning
signature and pattern: either add the missing first parameter (e.g.
OrderInterface $order or string $orderNumber) to thisOrderShipmentStateShouldBe
to accept the order placeholder, or remove ':orderNumber' from the attribute if
the order isn't needed—use theOrderShouldHavePaymentState as a reference for the
correct two-parameter pattern and ordering.
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingProductOptionsContext.php`:
- Around line 188-192: The method iDoNotAddAnOptionValue in
ManagingProductOptionsContext lacks the explicit return type; update its
signature to declare a void return type (i.e., add ": void") to match the
analogous iDoNotNameIt method and comply with the project's type declaration
guideline, ensuring consistency across context methods.
- Around line 54-58: The step definition iSpecifyATooLong in class
ManagingProductOptionsContext duplicates the identical step in
Ui/Admin/Helper/ValidationTrait, causing ambiguous Behat matches; fix by
removing the iSpecifyATooLong method from ManagingProductOptionsContext and
relying on ValidationTrait (ensure the trait is used by the class), or
alternatively rename/change the step pattern in ManagingProductOptionsContext to
a unique pattern and update any scenarios that reference it; reference the
method name iSpecifyATooLong and the trait ValidationTrait when making the
change.
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingPromotionsContext.php`:
- Around line 16-18: The three Behat import statements (Given, Then, When) in
ManagingPromotionsContext are out of alphabetical order; reorder the `use`
imports so they are sorted alphabetically (Given before Then before When) and
ensure imports are grouped by type (classes, functions, constants) per
guidelines so the block reads: Given, Then, When.
- Around line 250-258: The step definition
iAddTheItemPercentageDiscountActionConfiguredWithAPercentageValue has a
leftover/malformed docblock annotation and lacks the corresponding PHP attribute
for the variant without a percentage; remove the malformed "@WhenI ..." docblock
entry and add a second #[When(...)] attribute matching the no-percentage regex
(like the analogous Order percentage method uses) so both variants (with and
without percentage) are defined for
UnitPercentageDiscountPromotionActionCommand::TYPE and the method continues to
call formElement->addAction(...) and fillActionOption('Percentage', ...).
- Around line 127-132: Change the step attribute on the assertion method
thePromotionShouldHaveLabelInLocale from #[When('the :promotion promotion should
have a label :label in :localeCode locale')] to #[Then('the :promotion promotion
should have a label :label in :localeCode locale')] because this is a
verification step (it calls $this->formElement->hasLabel). Also make the
identical change for the matching assertion method in the Api Admin
ManagingPromotionsContext class (the method with the same step
text/thePromotionShouldHaveLabelInLocale) so both contexts use #[Then].
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingTaxRateContext.php`:
- Around line 77-81: The step implementation iSetTheEndDateTo currently calls
$this->createPage->specifyStartDate(new \DateTime($endDate)) which overwrites
the start date; change the call to $this->createPage->specifyEndDate(new
\DateTime($endDate)) so the end date is set correctly, leaving the method
signature iSetTheEndDateTo(string $endDate): void and using the same DateTime
construction.
In `@src/Sylius/Behat/Context/Ui/Admin/ProductVariantsCreationContext.php`:
- Around line 16-17: Reorder the PHP use imports in
ProductVariantsCreationContext so they are alphabetized and grouped by type:
place "use Behat\Behat\Context\Context;" before "use Behat\Step\When;" (and
adjust any other class/function/constant imports in this file to follow the same
rule) to comply with the project's import-sorting guideline.
🧹 Nitpick comments (40)
src/Sylius/Behat/Context/Ui/Admin/OrderHistoryContext.php (1)
16-18: Imports are not sorted alphabetically.The new
usestatements should be ordered alphabetically per the coding guidelines.Behat\Behat\Context\Contextshould come beforeBehat\Step\*.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; use Behat\Behat\Context\Context; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)."src/Sylius/Behat/Context/Ui/Admin/SearchFilterContext.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Behat\Context\Contextshould appear beforeBehat\Step\When. As per coding guidelines, "Sort PHPuseimports alphabetically and group by type."Proposed fix
-use Behat\Step\When; use Behat\Behat\Context\Context; +use Behat\Step\When; use Sylius\Behat\Element\Admin\Crud\Index\SearchFilterElementInterface;src/Sylius/Behat/Context/Ui/Admin/ManagingProductVariantsPricesContext.php (1)
16-17: Import order:Behat\Behat\…should precedeBehat\Step\….As per coding guidelines,
useimports should be sorted alphabetically.Proposed fix
-use Behat\Step\When; use Behat\Behat\Context\Context; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ManagingAdministratorLocalesContext.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Behat\Context\Contextshould come beforeBehat\Step\When.Proposed fix
-use Behat\Step\When; use Behat\Behat\Context\Context; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".src/Sylius/Behat/Context/Ui/Admin/Helper/ValidationTrait.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Step\Thenshould come beforeBehat\Step\When.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)."src/Sylius/Behat/Context/Ui/Admin/ProductCreationContext.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Behat\Context\Contextshould come beforeBehat\Step\When.Proposed fix
-use Behat\Step\When; use Behat\Behat\Context\Context; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".src/Sylius/Behat/Context/Ui/Admin/ManagingTranslatableEntitiesContext.php (1)
16-17: Imports are not in alphabetical order.
Behat\Step\Thenshould precedeBehat\Step\Whenalphabetically.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)."src/Sylius/Behat/Context/Ui/Admin/RemovingTaxonContext.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Step\Thenshould come beforeBehat\Step\When.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".src/Sylius/Behat/Context/Ui/Admin/ManagingCurrenciesContext.php (1)
16-18: Imports are not sorted alphabetically.The newly added
usestatements should be ordered so thatBehat\Behat\Context\Contextcomes before theBehat\Step\*imports. As per coding guidelines: "Sort PHPuseimports alphabetically and group by type."Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; use Behat\Behat\Context\Context; +use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ManagingProductReviewsContext.php (1)
16-18: Imports are not sorted alphabetically.
Behat\Step\Thenshould come beforeBehat\Step\When.Proposed fix
use Behat\Step\Given; -use Behat\Step\When; use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)."src/Sylius/Behat/Context/Ui/Admin/ManagingZonesContext.php (3)
16-18: Imports are not sorted alphabetically.The
Behat\Step\*imports should be orderedGiven,Then,When.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; use Behat\Step\Given; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type."
111-114: Missing type declaration for$scopeparameter.- public function iSelectItsScopeAs($scope): void + public function iSelectItsScopeAs(string $scope): voidAs per coding guidelines: "Add type declarations for all properties, arguments, and return values in PHP code."
313-319: Missing return type ontheZoneShouldHavePriority.- public function theZoneShouldHavePriority(ZoneInterface $zone, int $priority) + public function theZoneShouldHavePriority(ZoneInterface $zone, int $priority): voidAs per coding guidelines: "Add type declarations for all properties, arguments, and return values in PHP code."
src/Sylius/Behat/Context/Ui/Admin/ManagingProductTaxonsContext.php (1)
16-17: Import order:Thenshould come beforeWhenalphabetically.As per coding guidelines, "Sort PHP
useimports alphabetically and group by type."Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; +use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/RemovingProductContext.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Step\Thenshould come beforeBehat\Step\When.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type".src/Sylius/Behat/Context/Ui/Admin/ResettingPasswordContext.php (1)
16-18: Imports are not sorted alphabetically.
Behat\Behat\Context\Contextshould precedeBehat\Step\*, andThenshould come beforeWhen. As per coding guidelines, "Sort PHPuseimports alphabetically and group by type."Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; use Behat\Behat\Context\Context; +use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ManagingExchangeRatesContext.php (1)
16-19: Imports are not sorted alphabetically.
Behat\Behat\Context\Contextshould precede theBehat\Step\*imports, and within the Step group the order should beGiven,Then,When.Proposed fix
-use Behat\Step\When; -use Behat\Step\Given; -use Behat\Step\Then; use Behat\Behat\Context\Context; +use Behat\Step\Given; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".src/Sylius/Behat/Context/Ui/Admin/ManagingPaymentRequestsContext.php (2)
16-18: Imports are not sorted alphabetically.
Behat\Behat\Context\Context(line 18) should precede theBehat\Step\*imports, andThenshould come beforeWhen.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; use Behat\Behat\Context\Context; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".
49-52:findOneBymay returnnull, leading to a potential NPE on line 52.Pre-existing, but since you're touching this file — consider adding a null assertion or guard:
$paymentRequest = $this->paymentRequestRepository->findOneBy(['payment' => $payment]); +Assert::notNull($paymentRequest, 'No payment request found for this payment.');src/Sylius/Behat/Context/Ui/Admin/ManagingCountriesContext.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Step\ThenandBehat\Step\Whenshould be placed afterBehat\Mink\Exception\ElementNotFoundExceptionto maintain alphabetical order across allBehat\*imports.Suggested order
-use Behat\Step\When; -use Behat\Step\Then; use Behat\Behat\Context\Context; use Behat\Mink\Exception\ElementNotFoundException; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)."src/Sylius/Behat/Context/Ui/Admin/ManagingTaxonsContext.php (2)
206-210: Unused$taxonNameparameter.PHPMD correctly flags that
$taxonNameis never used in the method body — only$slugis forwarded tothisTaxonElementShouldBe. The parameter is required by Behat's step pattern for matching purposes, but consider prefixing it or adding a suppression if this triggers CI warnings.Suggested suppression
#[Then('the slug of the :taxonName taxon should( still) be :slug')] - public function theSlugOfTheTaxonShouldBe(string $taxonName, string $slug): void + /** `@phpstan-ignore-next-line` */ + public function theSlugOfTheTaxonShouldBe(string $taxonName, string $slug): voidAlternatively, if PHPMD is the linter in question, you could use
@SuppressWarnings(PHPMD.UnusedFormalParameter).
349-356: Redundant(int)cast on an already-typedintparameter.On line 355,
$countis already declared asintin the method signature, making(int) $counta no-op cast.Suggested fix
- Assert::same($this->imageFormElement->countImages(), (int) $count); + Assert::same($this->imageFormElement->countImages(), $count);src/Sylius/Behat/Context/Ui/Admin/ManagingTaxRateContext.php (1)
16-18: Import order: Behat step attributes are not sorted alphabetically.As per coding guidelines,
useimports should be sorted alphabetically:Given,Then,When.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; -use Behat\Step\Given; +use Behat\Step\Given; +use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/NavigatingBetweenProductShowAndEditPagesContext.php (1)
16-17: Import order:Thenshould precedeWhenalphabetically.As per coding guidelines, "Sort PHP
useimports alphabetically and group by type."Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; +use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ManagingShippingCategoriesContext.php (1)
16-17: Imports are not sorted alphabetically.
Behat\Step\Thenshould precedeBehat\Step\When.Proposed fix
-use Behat\Step\When; -use Behat\Step\Then; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".src/Sylius/Behat/Context/Ui/Admin/ManagingProductOptionsContext.php (2)
16-18: Imports are not sorted alphabetically.The Behat step imports should be ordered alphabetically:
Given,Then,When.Suggested fix
-use Behat\Step\When; -use Behat\Step\Given; -use Behat\Step\Then; +use Behat\Step\Given; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type."
74-78: Missing type declarations on$nameand$languageparameters.The sibling method
iRenameItToInLanguage(line 82) correctly types its parameters asstring. Consider adding type hints here for consistency.Suggested fix
- public function iNameItInLanguage($name, $language): void + public function iNameItInLanguage(string $name, string $language): voidAs per coding guidelines: "Add type declarations for all properties, arguments, and return values in PHP code."
src/Sylius/Behat/Context/Ui/Admin/ImpersonatingCustomersContext.php (1)
16-18: Nit: Imports are not sorted alphabetically.
Behat\Step\Thenshould come beforeBehat\Step\When. As per coding guidelines,useimports should be sorted alphabetically.Suggested fix
use Behat\Step\Given; -use Behat\Step\When; use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ManagingTaxCategoriesContext.php (1)
16-18: Nit: Imports are not sorted alphabetically.Should be
Given,Then,When. As per coding guidelines,useimports should be sorted alphabetically.Suggested fix
-use Behat\Step\When; use Behat\Step\Given; use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ProductShowPageContext.php (1)
16-18: Nit: Imports are not sorted alphabetically.
Thenshould come beforeWhen. As per coding guidelines,useimports should be sorted alphabetically.Suggested fix
use Behat\Step\Given; -use Behat\Step\When; use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/LoginContext.php (1)
16-18: Nit: Imports are not sorted alphabetically.Should be
Given,Then,When. As per coding guidelines,useimports should be sorted alphabetically.Suggested fix
-use Behat\Step\When; -use Behat\Step\Then; use Behat\Step\Given; +use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ManagingShippingMethodsContext.php (3)
16-19: Nit:Behat\Step\*imports are split and not sorted alphabetically.The
Whenimport ended up separated fromGivenandThenby theBehat\Behat\Context\Contextimport. Group them together and sort alphabetically. As per coding guidelines,useimports should be sorted alphabetically and grouped by type.Suggested fix
-use Behat\Step\Then; use Behat\Step\Given; -use Behat\Behat\Context\Context; +use Behat\Step\Then; use Behat\Step\When; +use Behat\Behat\Context\Context;
346-352: Unused parameter$sortType.The
$sortTypeparameter is accepted but never used — sorting direction is not passed tosortBy(). This is a pre-existing issue flagged by static analysis (PHPMD).
473-481: Unused variable$channel.
$channelis fetched from shared storage on line 476 but never referenced in the method body.Suggested fix
public function iShouldBeNotifiedThatTheWeightRuleHasAnInvalidConfiguration(): void { - $channel = $this->sharedStorage->get('channel'); Assert::same( $this->shippingMethodFormElement->getValidationMessage('last_rule_weight'), 'Please enter a number.', ); }src/Sylius/Behat/Context/Ui/Admin/ManagingLocalesContext.php (1)
16-18: Nit: Imports are not sorted alphabetically.
Thenshould come beforeWhen. As per coding guidelines,useimports should be sorted alphabetically.Suggested fix
use Behat\Step\Given; -use Behat\Step\When; use Behat\Step\Then; +use Behat\Step\When;src/Sylius/Behat/Context/Ui/Admin/ManagingPaymentsContext.php (1)
16-18: Imports not sorted alphabetically.The
Behat\Step\*imports should be in alphabetical order:Given,Then,When. This same ordering issue appears consistently across all files in this PR.Proposed fix
use Behat\Step\Given; -use Behat\Step\When; use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".src/Sylius/Behat/Context/Ui/Admin/BrowsingProductVariantsContext.php (1)
16-17: Same alphabetical import ordering nit as noted inManagingPaymentsContext.php.
Thenshould come beforeWhen.Proposed fix
-use Behat\Step\When; use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type".src/Sylius/Behat/Context/Ui/Admin/ManagingProductsContext.php (1)
691-696: Unused$productparameter flagged by static analysis.PHPMD flags
$productas unused. The parameter is required by Behat's step argument transformation (it resolves(this product)/(product "[^"]+")from the regex), so it can't simply be removed. However, it might be worth navigating to the product's update page here to make the method self-contained, similar to how other assertion methods in this file call$this->updateSimpleProductPage->open(...)first. This is a pre-existing issue, not introduced by this PR.src/Sylius/Behat/Context/Ui/Admin/ManagingInventoryContext.php (1)
37-37: Consider adding type declarations to untyped parameters.
iFilterTrackedVariantsWithCodeContaining($field, $value)(Line 37) andiShouldSeeThatTheProductVariantHasQuantityOnHand($productVariantName, $quantity)(Line 66) are missing type hints and return types. While pre-existing, this violates the guideline to add type declarations for all arguments and return values. As per coding guidelines, "Add type declarations for all properties, arguments, and return values in PHP code".Proposed type hints
- public function iFilterTrackedVariantsWithCodeContaining($field, $value) + public function iFilterTrackedVariantsWithCodeContaining(string $field, string $value): void- public function iShouldSeeThatTheProductVariantHasQuantityOnHand($productVariantName, $quantity) + public function iShouldSeeThatTheProductVariantHasQuantityOnHand(string $productVariantName, string $quantity): voidAlso applies to: 66-66
src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php (1)
16-18: Imports are not sorted alphabetically.
Behat\Step\Then(line 16) is placed beforeBehat\Behat\Context\Context(line 17). Per coding guidelines,useimports should be sorted alphabetically.Proposed fix
-use Behat\Step\Then; use Behat\Behat\Context\Context; use Behat\Step\When; +use Behat\Step\Then; +use Behat\Step\When;As per coding guidelines: "Sort PHP
useimports alphabetically and group by type (classes, functions, constants)".
| #[Given('I should be on login page')] | ||
| public function iShouldBeOnLoginPage() | ||
| { | ||
| Assert::true($this->loginPage->isOpen()); | ||
| } |
There was a problem hiding this comment.
#[Given] used on an assertion step — should this be #[Then]?
iShouldBeOnLoginPage asserts page state (Assert::true), which is a Then concern. Line 104 already defines #[Then('I should be on the login page')] for a nearly identical step. Using #[Given] here is semantically misleading and could cause confusion. If the original annotation was also @Given, this might be a good opportunity to correct it during the migration.
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/LoginContext.php` around lines 76 - 80, The
step attribute on the assertion method iShouldBeOnLoginPage is semantically
wrong: change the attribute from #[Given('I should be on login page')] to the
appropriate #[Then('I should be on the login page')] (or to match the existing
#[Then] variant at the other definition) so the assertion method uses a Then
annotation; update the attribute string to match the existing Then step text and
remove or consolidate duplicate step definitions if both methods now collide.
| #[Then('/^(this customer) should be enabled$/')] | ||
| public function thisCustomerShouldBeEnabled(CustomerInterface $customer): void | ||
| { | ||
| $this->indexPage->open(); | ||
|
|
||
| Assert::true($this->indexPage->isCustomerEnabled($customer), true); | ||
| } |
There was a problem hiding this comment.
Assert::true() called with true as the message argument.
Line 297: Assert::true($this->indexPage->isCustomerEnabled($customer), true) — the second parameter of Assert::true expects a string message, but true (boolean) is passed. This is a pre-existing issue but should be cleaned up.
Proposed fix
- Assert::true($this->indexPage->isCustomerEnabled($customer), true);
+ Assert::true($this->indexPage->isCustomerEnabled($customer));🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php` around lines
292 - 298, The Assert::true call in
ManagingCustomersContext::thisCustomerShouldBeEnabled passes a boolean as the
message argument; change it to call Assert::true with a proper boolean
expression and an optional string message (or no message) — e.g., replace the
current Assert::true($this->indexPage->isCustomerEnabled($customer), true) by
Assert::true($this->indexPage->isCustomerEnabled($customer)) or
Assert::true($this->indexPage->isCustomerEnabled($customer), 'Customer should be
enabled') so the second parameter is a string if present; refer to the
thisCustomerShouldBeEnabled method and indexPage->isCustomerEnabled to locate
the spot.
| #[When('I sort the orders :sortType by :field')] | ||
| public function iSortTheOrderByField(string $field): void | ||
| { | ||
| $this->ordersIndexPage->sort(ucfirst($field)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the current file first to see exact code
cat -n src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | sed -n '315,330p'Repository: Sylius/Sylius
Length of output: 764
🏁 Script executed:
#!/bin/bash
# Check git history for this method
git log -p --all -- 'src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php' | grep -B5 -A5 'sort the orders' | head -40Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for usages of this step in feature files
find . -name "*.feature" -type f -exec grep -l "sort the orders" {} \;Repository: Sylius/Sylius
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Read the feature file to see how the step is actually called
cat features/admin/user/managing_customers/browsing_orders_of_customer.feature | grep -A2 -B2 "sort the orders"Repository: Sylius/Sylius
Length of output: 348
🏁 Script executed:
#!/bin/bash
# Try a broader git log search for the method name
git log -p --all -- 'src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php' | grep -B10 -A10 'iSortTheOrderByField' | head -60Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check if there's any other usage of :sortType in this file
rg "sortType" src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.phpRepository: Sylius/Sylius
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Check git log for the attribute migration - look for #[When( changes
git log --oneline -p --all -- 'src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php' | grep -B5 -A15 '#\[When.*sort the orders' | head -80Repository: Sylius/Sylius
Length of output: 805
🏁 Script executed:
#!/bin/bash
# Search for the ordersIndexPage interface/implementation to understand sort() method
find . -name "*.php" -type f | xargs grep -l "ordersIndexPage" | head -10Repository: Sylius/Sylius
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Get more complete git history - look for the commit that added this
git log --oneline --all -- 'src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php' | head -20Repository: Sylius/Sylius
Length of output: 197
🏁 Script executed:
#!/bin/bash
# Find the ordersIndexPage class definition
find . -path ./vendor -prune -o -name "*.php" -type f -print | xargs grep -l "class.*OrdersIndexPage" | head -5Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Also search for the interface
find . -path ./vendor -prune -o -name "*.php" -type f -print | xargs grep -l "OrdersIndexPageInterface" | head -5Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Check the commit that converted annotations to attributes
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | grep -B15 -A15 'sort the orders' | head -60Repository: Sylius/Sylius
Length of output: 1133
🏁 Script executed:
#!/bin/bash
# Search more broadly for ordersIndexPage property definition
rg "ordersIndexPage" src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php -A3 -B3Repository: Sylius/Sylius
Length of output: 1110
🏁 Script executed:
#!/bin/bash
# Search for sort method in context files
rg "function sort\(" src/Sylius/Behat/ -A3 -B1Repository: Sylius/Sylius
Length of output: 2451
🏁 Script executed:
#!/bin/bash
# Check what the old annotation looked like before the conversion
git show ac55dda16e~1 -- src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | grep -B5 -A10 'sort the orders' | head -40Repository: Sylius/Sylius
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Look at the specific line in the previous commit
git show ac55dda16e~1:src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | sed -n '310,330p'Repository: Sylius/Sylius
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Get the full diff for this specific commit to see before/after
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | grep -B20 -A20 'iSortTheOrderByField'Repository: Sylius/Sylius
Length of output: 1514
🏁 Script executed:
#!/bin/bash
# Check if there was an original method with `@When` annotation
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | head -200Repository: Sylius/Sylius
Length of output: 6870
🏁 Script executed:
#!/bin/bash
# The commit shows the file as new (mode 100644). Let's check if there's an older version
git log --all --full-history -- src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | head -20Repository: Sylius/Sylius
Length of output: 878
🏁 Script executed:
#!/bin/bash
# Check if file exists at all in an earlier commit
git show ac55dda16e:src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php | grep -B10 -A10 'iSortTheOrderByField' | head -50Repository: Sylius/Sylius
Length of output: 819
🏁 Script executed:
#!/bin/bash
# Let's check if this feature test actually runs and what it expects
cat features/admin/user/managing_customers/browsing_orders_of_customer.featureRepository: Sylius/Sylius
Length of output: 1720
🏁 Script executed:
#!/bin/bash
# Check if sortType parameter should be used somewhere or if it's unused
# Look for how other similar sorting steps are implemented
rg "When.*sort.*by" src/Sylius/Behat/Context/ -A5 | grep -E "(When|function|sortType)" | head -40Repository: Sylius/Sylius
Length of output: 5405
🏁 Script executed:
#!/bin/bash
# Let's look at the ManagingProductsContext case that also seems to have the issue
rg "I sort the products :sortType by :field" src/Sylius/Behat/Context/Ui/Admin/ManagingProductsContext.php -B2 -A8Repository: Sylius/Sylius
Length of output: 496
🏁 Script executed:
#!/bin/bash
# And check a correct implementation to compare
rg "I sort the shipping methods :sortType by :field" src/Sylius/Behat/Context/Ui/Admin/ManagingShippingMethodsContext.php -B2 -A8Repository: Sylius/Sylius
Length of output: 477
Add $sortType parameter to match the step pattern placeholders.
The step pattern #[When('I sort the orders :sortType by :field')] declares two placeholders (:sortType and :field), but the method signature only has a $field parameter. With Behat's positional argument binding, the :sortType value ("ascending") will be received as $field, and the actual field name ("channel") will be lost. This breaks the step matching and causes ordersIndexPage->sort(ucfirst("ascending")) instead of ordersIndexPage->sort(ucfirst("channel")).
Add the $sortType parameter to the method signature:
#[When('I sort the orders :sortType by :field')]
-public function iSortTheOrderByField(string $field): void
+public function iSortTheOrderByField(string $sortType, string $field): void(The parameter may not be used internally, similar to other sorting methods in the codebase like ManagingShippingMethodsContext::iSortShippingMethodsBy, but it must be present for step matching to work correctly.)
📝 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.
| #[When('I sort the orders :sortType by :field')] | |
| public function iSortTheOrderByField(string $field): void | |
| { | |
| $this->ordersIndexPage->sort(ucfirst($field)); | |
| } | |
| #[When('I sort the orders :sortType by :field')] | |
| public function iSortTheOrderByField(string $sortType, string $field): void | |
| { | |
| $this->ordersIndexPage->sort(ucfirst($field)); | |
| } |
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php` around lines
320 - 324, The step definition declares two placeholders but the method
iSortTheOrderByField currently only accepts $field; add a second parameter
$sortType to the method signature (e.g., iSortTheOrderByField(string $sortType,
string $field): void) so Behat binds placeholders correctly, leaving $sortType
unused if not needed, and keep the existing call to
$this->ordersIndexPage->sort(ucfirst($field)).
| #[Then('order :orderNumber should have shipment state :shippingState')] | ||
| public function thisOrderShipmentStateShouldBe(string $shippingState): void | ||
| { | ||
| Assert::true($this->indexPage->isSingleResourceOnPage(['shippingState' => $shippingState])); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check git history for the original annotation before attribute conversion
cd "$(git rev-parse --show-toplevel)"
git log --all --oneline -S "thisOrderShipmentStateShouldBe" -- "src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php" | head -5Repository: Sylius/Sylius
Length of output: 197
🏁 Script executed:
#!/bin/bash
# Look at the current file to see context around the flagged methods
cd "$(git rev-parse --show-toplevel)"
sed -n '600,635p' "src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php"Repository: Sylius/Sylius
Length of output: 1754
🏁 Script executed:
#!/bin/bash
# Search for other similar patterns with order parameter in the same file
cd "$(git rev-parse --show-toplevel)"
rg -n "order :orderNumber" --type php "src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php"Repository: Sylius/Sylius
Length of output: 209
🏁 Script executed:
#!/bin/bash
# Get the full diff for the relevant commit
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e:src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | sed -n '600,635p'Repository: Sylius/Sylius
Length of output: 1754
🏁 Script executed:
#!/bin/bash
# Also check if there were any changes to the method signature or attribute in that commit
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | grep -A5 -B5 "thisOrderShipmentStateShouldBe"Repository: Sylius/Sylius
Length of output: 510
🏁 Script executed:
#!/bin/bash
# Check the state of the file BEFORE the attribute conversion
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e^:src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | sed -n '600,635p'Repository: Sylius/Sylius
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Also get more context - find the method in the previous version
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e^:src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | grep -n "thisOrderShipmentStateShouldBe" -A 5 -B 5Repository: Sylius/Sylius
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Find the commit before ac55dda16e
cd "$(git rev-parse --show-toplevel)"
git log --oneline --all | grep -A1 "ac55dda16e"Repository: Sylius/Sylius
Length of output: 197
🏁 Script executed:
#!/bin/bash
# Try to see the full diff of the conversion commit
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | head -300 | tail -150Repository: Sylius/Sylius
Length of output: 5764
🏁 Script executed:
#!/bin/bash
# Look for the original annotation format by searching in git log before the conversion
cd "$(git rev-parse --show-toplevel)"
git log --all -p --diff-filter=M -S "@Then" -- "src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php" | grep -A3 -B3 "thisOrderShipmentStateShouldBe" | head -40Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Get the full diff of that commit focusing on the method
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | grep -A10 "thisOrderShipmentStateShouldBe"Repository: Sylius/Sylius
Length of output: 641
🏁 Script executed:
#!/bin/bash
# Try looking for `@Then` annotations from before the conversion
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | grep -B2 "@Then.*orderNumber.*shipment"Repository: Sylius/Sylius
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for the entire diff section with both - and + lines to see the change
cd "$(git rev-parse --show-toplevel)"
git diff ac55dda16e~1 ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | grep -A5 -B5 "thisOrderShipmentStateShouldBe"Repository: Sylius/Sylius
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Get the full diff for the commit to see both - and + lines
cd "$(git rev-parse --show-toplevel)"
git show ac55dda16e -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php > /tmp/commit_diff.txt
grep -A10 -B10 "thisOrderShipmentStateShouldBe" /tmp/commit_diff.txtRepository: Sylius/Sylius
Length of output: 1039
🏁 Script executed:
#!/bin/bash
# Check if the original file exists in a parent commit - look at git log
cd "$(git rev-parse --show-toplevel)"
git log --oneline --all -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php | head -10Repository: Sylius/Sylius
Length of output: 197
🏁 Script executed:
#!/bin/bash
# Check the file from the commit before the conversion
cd "$(git rev-parse --show-toplevel)"
git show 40c1e05e78:src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php 2>/dev/null | grep -A5 -B5 "thisOrderShipmentStateShouldBe"Repository: Sylius/Sylius
Length of output: 355
🏁 Script executed:
#!/bin/bash
# Also search for the method in any earlier commits
cd "$(git rev-parse --show-toplevel)"
git log --all -p --follow -S "thisOrderShipmentStateShouldBe" -- src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php 2>/dev/null | grep -A8 -B2 "@Then.*order" | head -30Repository: Sylius/Sylius
Length of output: 987
🏁 Script executed:
#!/bin/bash
# Examine the specific lines flagged for unused $order parameter
cd "$(git rev-parse --show-toplevel)"
sed -n '614,632p' src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.phpRepository: Sylius/Sylius
Length of output: 1110
Correct the step definition to match method parameters.
The attribute pattern 'order :orderNumber should have shipment state :shippingState' defines two placeholders but the method only accepts string $shippingState. Behat will pass both :orderNumber and :shippingState as arguments, causing a parameter count mismatch.
Either add OrderInterface $orderNumber (or similar) as the first parameter, or remove :orderNumber from the step pattern if the order number is not needed. Compare with similar methods like theOrderShouldHavePaymentState (lines 614–619) which correctly capture both :order and :orderPaymentState parameters.
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php` around lines 608
- 612, The step definition attribute for thisOrderShipmentStateShouldBe declares
two placeholders ('order :orderNumber should have shipment state
:shippingState') but the method signature only accepts string $shippingState,
causing a parameter mismatch; fix by aligning signature and pattern: either add
the missing first parameter (e.g. OrderInterface $order or string $orderNumber)
to thisOrderShipmentStateShouldBe to accept the order placeholder, or remove
':orderNumber' from the attribute if the order isn't needed—use
theOrderShouldHavePaymentState as a reference for the correct two-parameter
pattern and ordering.
| #[Then('the order :order should have order shipping state :orderShippingState')] | ||
| #[Then('/^(this order) should have order shipping state "([^"]+)"$/')] | ||
| public function theOrderShouldHaveShippingState(OrderInterface $order, string $orderShippingState): void | ||
| { | ||
| Assert::true($this->indexPage->isSingleResourceOnPage(['shippingState' => $orderShippingState])); | ||
| } |
There was a problem hiding this comment.
Unused $order parameter — confirmed by static analysis.
The $order parameter is accepted but never used in the method body. The same issue applies to theOrderShouldHavePaymentState at line 616. If the parameter is required for Behat's argument matching from the regex group (this order), consider prefixing with an underscore or adding a suppression annotation to signal intent. Otherwise, remove it.
Proposed fix if the parameter is only needed for Behat matching
If Behat requires the positional parameter but the method doesn't use it, you could at minimum suppress the warning. Alternatively, if there's intended behavior (e.g., navigating to the order first), consider using it:
- public function theOrderShouldHaveShippingState(OrderInterface $order, string $orderShippingState): void
+ public function theOrderShouldHaveShippingState(OrderInterface $_order, string $orderShippingState): void
{
Assert::true($this->indexPage->isSingleResourceOnPage(['shippingState' => $orderShippingState]));
}Apply the same pattern to theOrderShouldHavePaymentState at line 616.
📝 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.
| #[Then('the order :order should have order shipping state :orderShippingState')] | |
| #[Then('/^(this order) should have order shipping state "([^"]+)"$/')] | |
| public function theOrderShouldHaveShippingState(OrderInterface $order, string $orderShippingState): void | |
| { | |
| Assert::true($this->indexPage->isSingleResourceOnPage(['shippingState' => $orderShippingState])); | |
| } | |
| #[Then('the order :order should have order shipping state :orderShippingState')] | |
| #[Then('/^(this order) should have order shipping state "([^"]+)"$/')] | |
| public function theOrderShouldHaveShippingState(OrderInterface $_order, string $orderShippingState): void | |
| { | |
| Assert::true($this->indexPage->isSingleResourceOnPage(['shippingState' => $orderShippingState])); | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 629-629: Avoid unused parameters such as '$order'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingOrdersContext.php` around lines 627
- 632, The methods theOrderShouldHaveShippingState and
theOrderShouldHavePaymentState declare an OrderInterface $order parameter that
is not used; either remove the unused $order parameter if Behat doesn't require
it, or rename it to _order (or add a suppression annotation) to signal
intentional unused parameter for Behat regex argument matching—update both
method signatures accordingly and keep the rest of each method body unchanged.
| use Behat\Step\When; | ||
| use Behat\Step\Then; | ||
| use Behat\Step\Given; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Imports are not sorted alphabetically.
Per coding guidelines, use imports should be sorted alphabetically. Given should come before Then and When.
Proposed fix
-use Behat\Step\When;
-use Behat\Step\Then;
-use Behat\Step\Given;
+use Behat\Step\Given;
+use Behat\Step\Then;
+use Behat\Step\When;As per coding guidelines: "Sort PHP use imports alphabetically and group by type (classes, functions, constants)."
📝 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.
| use Behat\Step\When; | |
| use Behat\Step\Then; | |
| use Behat\Step\Given; | |
| use Behat\Step\Given; | |
| use Behat\Step\Then; | |
| use Behat\Step\When; |
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingPromotionsContext.php` around lines
16 - 18, The three Behat import statements (Given, Then, When) in
ManagingPromotionsContext are out of alphabetical order; reorder the `use`
imports so they are sorted alphabetically (Given before Then before When) and
ensure imports are grouped by type (classes, functions, constants) per
guidelines so the block reads: Given, Then, When.
| #[When('the :promotion promotion should have a label :label in :localeCode locale')] | ||
| public function thePromotionShouldHaveLabelInLocale(PromotionInterface $promotion, string $label, string $localeCode): void | ||
| { | ||
| $this->updatePage->open(['id' => $promotion->getId()]); | ||
| $this->formElement->hasLabel($label, $localeCode); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usage of this step in feature files
echo "=== Searching for step usage in feature files ==="
rg -n "the .* promotion should have a label" . 2>/dev/null || echo "Pattern search returned no results"
echo ""
echo "=== Searching for broader pattern ==="
rg "should have a label" . --type-add 'feature:*.feature' --type feature 2>/dev/null || echo "No feature files found with this pattern"
echo ""
echo "=== Finding all .feature files ==="
find . -name "*.feature" -type f | head -20Repository: Sylius/Sylius
Length of output: 2429
🏁 Script executed:
#!/bin/bash
# Examine the context file for assertion patterns and similar steps
echo "=== Looking at the ManagingPromotionsContext file ==="
rg -n "assertion|hasLabel|#\[When\]|#\[Then\]" "src/Sylius/Behat/Context/Ui/Admin/ManagingPromotionsContext.php" -A 2Repository: Sylius/Sylius
Length of output: 188
🏁 Script executed:
#!/bin/bash
# Check the immediate context around the method to understand its usage pattern
echo "=== Examining the method and its assertions ==="
sed -n '120,140p' "src/Sylius/Behat/Context/Ui/Admin/ManagingPromotionsContext.php"Repository: Sylius/Sylius
Length of output: 1165
🏁 Script executed:
#!/bin/bash
# Check if similar assertion methods in the file use #[Then] or #[When]
echo "=== Looking for other assertion-like methods in the file ==="
rg -n "should|have" "src/Sylius/Behat/Context/Ui/Admin/ManagingPromotionsContext.php" | head -30Repository: Sylius/Sylius
Length of output: 2692
Change #[When] to #[Then] for this assertion step.
The step text "should have a label" and the method implementation (which calls hasLabel()) both indicate this is a verification step, not an action. It should use #[Then] to match the semantic pattern used by other assertion steps in this file (e.g., lines 97, 99, 371, 391, 421, 485).
Note: The same issue exists in src/Sylius/Behat/Context/Api/Admin/ManagingPromotionsContext.php:546 with the identical step definition.
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingPromotionsContext.php` around lines
127 - 132, Change the step attribute on the assertion method
thePromotionShouldHaveLabelInLocale from #[When('the :promotion promotion should
have a label :label in :localeCode locale')] to #[Then('the :promotion promotion
should have a label :label in :localeCode locale')] because this is a
verification step (it calls $this->formElement->hasLabel). Also make the
identical change for the matching assertion method in the Api Admin
ManagingPromotionsContext class (the method with the same step
text/thePromotionShouldHaveLabelInLocale) so both contexts use #[Then].
| /** | ||
| * @When /^I add the "Item percentage discount" action configured with a percentage value of "(?:|-)([^"]+)%"$/ | ||
| * @When I add the "Item percentage discount" action configured without a percentage value | ||
| * @WhenI add the "Item percentage discount" action configured without a percentage value | ||
| */ | ||
| #[When('/^I add the "Item percentage discount" action configured with a percentage value of "(?:|-)([^"]+)%"$/')] | ||
| public function iAddTheItemPercentageDiscountActionConfiguredWithAPercentageValue($percentage = null) | ||
| { | ||
| $this->formElement->addAction(UnitPercentageDiscountPromotionActionCommand::TYPE); | ||
| $this->formElement->fillActionOption('Percentage', $percentage ?? ''); | ||
| } |
There was a problem hiding this comment.
Leftover annotation not converted to PHP attribute — step definition is broken.
The @WhenI add the "Item percentage discount" action configured without a percentage value annotation on Line 251 was not converted to a #[When(...)] attribute. Additionally, the annotation itself is malformed (missing space between @When and I), so this step was likely already broken before this PR.
Compare with the analogous "Order percentage discount" method at Lines 242–248, which correctly has both variants as attributes. This method should follow the same pattern.
Proposed fix
- /**
- * `@WhenI` add the "Item percentage discount" action configured without a percentage value
- */
#[When('/^I add the "Item percentage discount" action configured with a percentage value of "(?:|-)([^"]+)%"$/')]
+ #[When('I add the "Item percentage discount" action configured without a percentage value')]
public function iAddTheItemPercentageDiscountActionConfiguredWithAPercentageValue($percentage = null)📝 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.
| /** | |
| * @When /^I add the "Item percentage discount" action configured with a percentage value of "(?:|-)([^"]+)%"$/ | |
| * @When I add the "Item percentage discount" action configured without a percentage value | |
| * @WhenI add the "Item percentage discount" action configured without a percentage value | |
| */ | |
| #[When('/^I add the "Item percentage discount" action configured with a percentage value of "(?:|-)([^"]+)%"$/')] | |
| public function iAddTheItemPercentageDiscountActionConfiguredWithAPercentageValue($percentage = null) | |
| { | |
| $this->formElement->addAction(UnitPercentageDiscountPromotionActionCommand::TYPE); | |
| $this->formElement->fillActionOption('Percentage', $percentage ?? ''); | |
| } | |
| #[When('/^I add the "Item percentage discount" action configured with a percentage value of "(?:|-)([^"]+)%"$/')] | |
| #[When('I add the "Item percentage discount" action configured without a percentage value')] | |
| public function iAddTheItemPercentageDiscountActionConfiguredWithAPercentageValue($percentage = null) | |
| { | |
| $this->formElement->addAction(UnitPercentageDiscountPromotionActionCommand::TYPE); | |
| $this->formElement->fillActionOption('Percentage', $percentage ?? ''); | |
| } |
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingPromotionsContext.php` around lines
250 - 258, The step definition
iAddTheItemPercentageDiscountActionConfiguredWithAPercentageValue has a
leftover/malformed docblock annotation and lacks the corresponding PHP attribute
for the variant without a percentage; remove the malformed "@WhenI ..." docblock
entry and add a second #[When(...)] attribute matching the no-percentage regex
(like the analogous Order percentage method uses) so both variants (with and
without percentage) are defined for
UnitPercentageDiscountPromotionActionCommand::TYPE and the method continues to
call formElement->addAction(...) and fillActionOption('Percentage', ...).
| #[When('I set the end date to :endDate')] | ||
| public function iSetTheEndDateTo(string $endDate): void | ||
| { | ||
| $this->createPage->specifyStartDate(new \DateTime($endDate)); | ||
| } |
There was a problem hiding this comment.
Bug: iSetTheEndDateTo calls specifyStartDate instead of specifyEndDate.
This is a copy-paste error from iSetTheStartDateTo above. The end date step will incorrectly overwrite the start date.
🐛 Proposed fix
#[When('I set the end date to :endDate')]
public function iSetTheEndDateTo(string $endDate): void
{
- $this->createPage->specifyStartDate(new \DateTime($endDate));
+ $this->createPage->specifyEndDate(new \DateTime($endDate));
}🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ManagingTaxRateContext.php` around lines 77
- 81, The step implementation iSetTheEndDateTo currently calls
$this->createPage->specifyStartDate(new \DateTime($endDate)) which overwrites
the start date; change the call to $this->createPage->specifyEndDate(new
\DateTime($endDate)) so the end date is set correctly, leaving the method
signature iSetTheEndDateTo(string $endDate): void and using the same DateTime
construction.
| use Behat\Step\When; | ||
| use Behat\Behat\Context\Context; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Imports are not sorted alphabetically.
Behat\Behat\Context\Context should precede Behat\Step\When alphabetically.
Proposed fix
-use Behat\Step\When;
use Behat\Behat\Context\Context;
+use Behat\Step\When;As per coding guidelines: "Sort PHP use imports alphabetically and group by type (classes, functions, constants)".
📝 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.
| use Behat\Step\When; | |
| use Behat\Behat\Context\Context; | |
| use Behat\Behat\Context\Context; | |
| use Behat\Step\When; |
🤖 Prompt for AI Agents
In `@src/Sylius/Behat/Context/Ui/Admin/ProductVariantsCreationContext.php` around
lines 16 - 17, Reorder the PHP use imports in ProductVariantsCreationContext so
they are alphabetized and grouped by type: place "use
Behat\Behat\Context\Context;" before "use Behat\Step\When;" (and adjust any
other class/function/constant imports in this file to follow the same rule) to
comply with the project's import-sorting guideline.
Summary by CodeRabbit