Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -224,21 +224,21 @@ public function iAddANewCatalogPromotionScope(): void
*/
public function iAddScopeThatAppliesOnVariants(ProductVariantInterface ...$variants): void
{
$variantCodes = array_map(fn (ProductVariantInterface $variant) => $variant->getCode(), $variants);
$variantNames = array_map(fn (ProductVariantInterface $variant) => $variant->getName(), $variants);

$this->formElement->addScope(InForVariantsScopeVariantChecker::TYPE);
$this->formElement->selectScopeOption($variantCodes);
$this->formElement->selectScopeOption($variantNames);
}

/**
* @When /^I add scope that applies on ("[^"]+" taxon)$/
*/
public function iAddScopeThatAppliesOnTaxons(TaxonInterface ...$taxons): void
{
$taxonsCodes = array_map(fn (TaxonInterface $taxon) => $taxon->getCode(), $taxons);
$taxonNames = array_map(fn (TaxonInterface $taxon) => $taxon->getName(), $taxons);

$this->formElement->addScope(InForTaxonsScopeVariantChecker::TYPE);
$this->formElement->selectScopeOption($taxonsCodes);
$this->formElement->selectScopeOption($taxonNames);
}

/**
Expand All @@ -247,23 +247,23 @@ public function iAddScopeThatAppliesOnTaxons(TaxonInterface ...$taxons): void
public function iAddScopeThatAppliesOnProduct(ProductInterface $product): void
{
$this->formElement->addScope(InForProductScopeVariantChecker::TYPE);
$this->formElement->selectScopeOption([$product->getCode()]);
$this->formElement->selectScopeOption([$product->getName()]);
}

/**
* @When I add :productVariant variant to its scope
*/
public function iAddVariantToItsScope(ProductVariantInterface $productVariant): void
{
$this->formElement->selectScopeOption([$productVariant->getCode()]);
$this->formElement->selectScopeOption([$productVariant->getName()]);
}

/**
* @When I remove :productVariant variant from its scope
*/
public function iRemoveVariantFromItsScope(ProductVariantInterface $productVariant): void
{
$this->formElement->removeScopeOption([$productVariant->getCode()]);
$this->formElement->removeScopeOption([$productVariant->getName()]);
}

/**
Expand Down Expand Up @@ -754,10 +754,10 @@ public function itShouldHaveVariantBasedScope(
): void {
$this->updatePage->open(['id' => $catalogPromotion->getId()]);

$selectedVariants = $this->formElement->getLastScopeCodes();
$selectedVariants = $this->formElement->getLastScopeNames();

foreach ($variants as $productVariant) {
Assert::inArray($productVariant->getCode(), $selectedVariants);
Assert::inArray($productVariant->getName(), $selectedVariants);
}

$this->sharedStorage->set('catalog_promotion', $catalogPromotion);
Expand All @@ -772,10 +772,10 @@ public function itShouldHaveTaxonsBasedScope(
): void {
$this->updatePage->open(['id' => $catalogPromotion->getId()]);

$selectedTaxons = $this->formElement->getLastScopeCodes();
$selectedTaxons = $this->formElement->getLastScopeNames();

foreach ($taxons as $taxon) {
Assert::inArray($taxon->getCode(), $selectedTaxons);
Assert::inArray($taxon->getName(), $selectedTaxons);
}
}

Expand All @@ -784,9 +784,9 @@ public function itShouldHaveTaxonsBasedScope(
*/
public function thisCatalogPromotionShouldBeAppliedOnTaxon(TaxonInterface $taxon): void
{
$selectedTaxons = $this->formElement->getLastScopeCodes();
$selectedTaxons = $this->formElement->getLastScopeNames();

Assert::inArray($taxon->getCode(), $selectedTaxons);
Assert::inArray($taxon->getName(), $selectedTaxons);
}

/**
Expand All @@ -806,8 +806,8 @@ public function theCatalogPromotionShouldApplyToAllVariantsOfProduct(
*/
public function thisCatalogPromotionShouldBeAppliedOnProduct(ProductInterface $product): void
{
$selectedProducts = $this->formElement->getLastScopeCodes();
Assert::inArray($product->getCode(), $selectedProducts);
$selectedProducts = $this->formElement->getLastScopeNames();
Assert::inArray($product->getName(), $selectedProducts);
}

/**
Expand All @@ -816,10 +816,10 @@ public function thisCatalogPromotionShouldBeAppliedOnProduct(ProductInterface $p
*/
public function itShouldApplyToVariants(ProductVariantInterface ...$variants): void
{
$selectedVariants = $this->formElement->getLastScopeCodes();
$selectedVariants = $this->formElement->getLastScopeNames();

foreach ($variants as $productVariant) {
Assert::inArray($productVariant->getCode(), $selectedVariants);
Assert::inArray($productVariant->getName(), $selectedVariants);
}
}

Expand All @@ -828,10 +828,10 @@ public function itShouldApplyToVariants(ProductVariantInterface ...$variants): v
*/
public function itShouldNotApplyToVariants(ProductVariantInterface ...$variants): void
{
$selectedVariants = $this->formElement->getLastScopeCodes();
$selectedVariants = $this->formElement->getLastScopeNames();

foreach ($variants as $productVariant) {
Assert::false(in_array($productVariant->getCode(), $selectedVariants));
Assert::false(in_array($productVariant->getName(), $selectedVariants, true));
}
}

Expand Down Expand Up @@ -1191,7 +1191,7 @@ private function createCatalogPromotion(
$this->formElement->setExclusiveness($exclusive);
$this->formElement->checkChannel($channel);
$this->formElement->addScope(InForProductScopeVariantChecker::TYPE);
$this->formElement->selectScopeOption([$product->getCode()]);
$this->formElement->selectScopeOption([$product->getName()]);
$this->formElement->addAction(PercentageDiscountPriceCalculator::TYPE);
$this->formElement->fillActionOption('Amount', $discount);
$this->createPage->create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,14 @@ public function thisProductShouldHaveAnImageWithType(string $type): void
*/
public function itsImageShouldHaveVariantSelected(ProductVariantInterface $productVariant): void
{
Assert::true($this->mediaFormElement->hasImageWithVariant($productVariant));
Assert::true(
$this->mediaFormElement->hasImageWithVariant($productVariant),
sprintf(
'Expected variant "%s" to be selected, but got "%s".',
$productVariant->getName(),
$this->mediaFormElement->getFirstImageSelectedVariantName() ?? 'none',
),
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public function theProductShouldNoLongerHaveAMainTaxon(ProductInterface $product
*/
public function thisTaxonShouldBelongsTo(TaxonInterface $taxon): void
{
Assert::same($this->formElement->getParent(), $taxon->getCode());
Assert::same($this->formElement->getParent(), $taxon->getName());
}

/**
Expand Down
20 changes: 10 additions & 10 deletions src/Sylius/Behat/Element/Admin/CatalogPromotion/FormElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ public function addAction(string $type): void
$this->waitForFormUpdate();
}

public function selectScopeOption(array $values): void
public function selectScopeOption(array $names): void
{
$lastScope = $this->getElement('last_scope');
foreach ($values as $value) {
$this->autocompleteHelper->selectByValue(
foreach ($names as $name) {
$this->autocompleteHelper->selectByName(
$this->getDriver(),
$lastScope->find('css', 'select')->getXpath(),
$value,
$name,
);
}

Expand All @@ -128,12 +128,12 @@ public function fillActionOptionForChannel(string $channelCode, string $option,
$lastAction->find('css', sprintf('[id$="_configuration_%s"]', $channelCode))->fillField($option, $value);
}

public function getLastScopeCodes(): array
public function getLastScopeNames(): array
{
$lastScope = $this->getElement('last_scope');

return array_map(
fn (NodeElement $element) => $element->getValue(),
fn (NodeElement $element) => $element->getText(),
$lastScope->findAll('css', 'option[selected="selected"]'),
);
}
Expand Down Expand Up @@ -176,14 +176,14 @@ public function getValidationMessages(): array
return array_map(fn (NodeElement $element) => $element->getText(), $errors);
}

public function removeScopeOption(array $values): void
public function removeScopeOption(array $names): void
{
$lastScope = $this->getElement('last_scope');
foreach ($values as $value) {
$this->autocompleteHelper->removeByValue(
foreach ($names as $name) {
$this->autocompleteHelper->removeByName(
$this->getDriver(),
$lastScope->find('css', 'select')->getXpath(),
$value,
$name,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ public function addScope(string $type): void;

public function addAction(string $type): void;

public function selectScopeOption(array $values): void;
public function selectScopeOption(array $names): void;

public function fillActionOption(string $option, string $value): void;

public function fillActionOptionForChannel(string $channelCode, string $option, string $value): void;

public function getLastScopeCodes(): array;
public function getLastScopeNames(): array;

public function getLastActionOption(string $option): string;

Expand All @@ -61,7 +61,7 @@ public function checkIfActionConfigurationFormIsVisible(): bool;

public function getValidationMessages(): array;

public function removeScopeOption(array $values): void;
public function removeScopeOption(array $names): void;

public function removeLastAction(): void;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ public function excludeTaxon(TaxonInterface $taxon): void
{
$excludeTaxonElement = $this->getElement('taxons_excluded_from_showing_lowest_price');

$this->autocompleteHelper->selectByValue(
$this->autocompleteHelper->selectByName(
$this->getDriver(),
$excludeTaxonElement->getXpath(),
$taxon->getCode(),
$taxon->getName(),
);
$this->waitForFormUpdate();
}
Expand All @@ -45,10 +45,10 @@ public function removeExcludedTaxon(TaxonInterface $taxon): void
{
$excludeTaxonElement = $this->getElement('taxons_excluded_from_showing_lowest_price');

$this->autocompleteHelper->removeByValue(
$this->autocompleteHelper->removeByName(
$this->getDriver(),
$excludeTaxonElement->getXpath(),
$taxon->getCode(),
$taxon->getName(),
);
$this->waitForFormUpdate();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ public function removeAssociatedProduct(ProductInterface $product, ProductAssoci
$this->changeTab();
$associationField = $this->getElement('associations', ['%association%' => $productAssociationType->getCode()]);

$this->autocompleteHelper->removeByValue(
$this->autocompleteHelper->removeByName(
$this->getDriver(),
$associationField->getXpath(),
$product->getCode(),
$product->getName(),
);
}

Expand All @@ -62,7 +62,12 @@ public function hasAssociatedProduct(ProductInterface $product, ProductAssociati
$this->changeTab();
$associationField = $this->getElement('associations', ['%association%' => $productAssociationType->getCode()]);

return in_array($product->getCode(), $associationField->getValue(), true);
$selectedItems = $this->autocompleteHelper->getSelectedItems(
$this->getDriver(),
$associationField->getXpath(),
);

return in_array($product->getName(), $selectedItems, true);
}

protected function getDefinedElements(): array
Expand Down
30 changes: 24 additions & 6 deletions src/Sylius/Behat/Element/Admin/Product/MediaFormElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ public function attachImage(string $path, ?string $type = null, ?ProductVariantI
}

if (null !== $productVariant) {
$this->autocompleteHelper->selectByValue(
$this->autocompleteHelper->selectByName(
$this->getDriver(),
$imageSubform->find('css', '[data-test-product-variant]')->getXpath(),
$productVariant->getCode(),
$productVariant->getName(),
);
}

Expand Down Expand Up @@ -106,12 +106,30 @@ public function hasImageWithType(string $type): bool
}

public function hasImageWithVariant(ProductVariantInterface $productVariant): bool
{
$selectedVariantName = $this->getFirstImageSelectedVariantName();

if (null === $selectedVariantName) {
return false;
}

return str_contains($selectedVariantName, $productVariant->getName());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Functional High

The hasImageWithVariant method uses str_contains() to check if a product variant name matches the selected variant, but this performs a substring match rather than an exact match. This causes false positives where checking for variant "Small" incorrectly returns true when "Extra Small" is selected, since "Small" is a substring of "Extra Small". This leads to unreliable test assertions in image-variant validation.

Suggested change
return str_contains($selectedVariantName, $productVariant->getName());
return $selectedVariantName === $productVariant->getName();
Prompt for AI assistance

Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:

You are an expert bash developer with deep knowledge of security, performance, and best practices.

### Context

File: src/Sylius/Behat/Element/Admin/Product/MediaFormElement.php
Lines: 116-116
Issue Type: functional-high
Severity: high

Issue Description:
The `hasImageWithVariant` method uses `str_contains()` to check if a product variant name matches the selected variant, but this performs a substring match rather than an exact match. This causes false positives where checking for variant "Small" incorrectly returns true when "Extra Small" is selected, since "Small" is a substring of "Extra Small". This leads to unreliable test assertions in image-variant validation.

Current Code:
        return str_contains($selectedVariantName, $productVariant->getName());

---

### Instructions

1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed

### Constraints

- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready

---


Like Dislike Create Issue

Comment on lines +110 to +116

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Functional High

The hasImageWithVariant() method has two bugs: (1) it only checks the first image subform via getFirstImageSelectedVariantName(), missing variants assigned to other images, and (2) it uses str_contains() for substring matching instead of exact comparison, causing false positives (e.g., 'Red' matches 'Red Large'). The fix must iterate through all image subforms and use exact string comparison.

Suggested fix
        public function hasImageWithVariant(ProductVariantInterface $productVariant): bool
            {
                $this->changeTab();

                $images = $this->getElement('images');
                $imageSubforms = $images->findAll('css', '[data-test-image-subform]');

                foreach ($imageSubforms as $imageSubform) {
                    $variantField = $imageSubform->find('css', '[data-test-product-variant]');
                    if (null === $variantField) {
                        continue;
                    }

                    $selectedOption = $variantField->find('css', 'option[selected]');
                    if (null === $selectedOption) {
                        continue;
                    }

                    if ($selectedOption->getText() === $productVariant->getName()) {
                        return true;
                    }
                }

                return false;
            }
Prompt for AI assistance

Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:

You are an expert bash developer with deep knowledge of security, performance, and best practices.

### Context

File: src/Sylius/Behat/Element/Admin/Product/MediaFormElement.php
Lines: 110-116
Issue Type: functional-high
Severity: high

Issue Description:
The `hasImageWithVariant()` method has two bugs: (1) it only checks the first image subform via `getFirstImageSelectedVariantName()`, missing variants assigned to other images, and (2) it uses `str_contains()` for substring matching instead of exact comparison, causing false positives (e.g., 'Red' matches 'Red Large'). The fix must iterate through all image subforms and use exact string comparison.

Current Code:
        $selectedVariantName = $this->getFirstImageSelectedVariantName();

        if (null === $selectedVariantName) {
            return false;
        }

        return str_contains($selectedVariantName, $productVariant->getName());

---

### Instructions

1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed

### Constraints

- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready

---


Like Dislike Create Issue

}
Comment on lines 108 to +117

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Functional High

The hasImageWithVariant method only checks the first image's selected variant instead of searching all images in the gallery for a matching variant. This causes the method to incorrectly return false when a variant is attached to any image other than the first one. The original implementation used a CSS selector to search the entire images container, but the refactored version delegates to getFirstImageSelectedVariantName which explicitly retrieves only the first image subform.

Suggested fix
    public function hasImageWithVariant(ProductVariantInterface $productVariant): bool
        {
            $this->changeTab();
        
            $images = $this->getElement('images');
            $variantElements = $images->findAll('css', '[data-test-product-variant*="' . $productVariant->getName() . '"]');
        
            return count($variantElements) > 0;
        }
Prompt for AI assistance

Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:

You are an expert bash developer with deep knowledge of security, performance, and best practices.

### Context

File: src/Sylius/Behat/Element/Admin/Product/MediaFormElement.php
Lines: 108-117
Issue Type: functional-high
Severity: high

Issue Description:
The `hasImageWithVariant` method only checks the first image's selected variant instead of searching all images in the gallery for a matching variant. This causes the method to incorrectly return `false` when a variant is attached to any image other than the first one. The original implementation used a CSS selector to search the entire images container, but the refactored version delegates to `getFirstImageSelectedVariantName` which explicitly retrieves only the first image subform.

Current Code:
    public function hasImageWithVariant(ProductVariantInterface $productVariant): bool
    {
        $selectedVariantName = $this->getFirstImageSelectedVariantName();

        if (null === $selectedVariantName) {
            return false;
        }

        return str_contains($selectedVariantName, $productVariant->getName());
    }

---

### Instructions

1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed

### Constraints

- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready

---


Like Dislike Create Issue


public function getFirstImageSelectedVariantName(): ?string
{
$this->changeTab();

$images = $this->getElement('images');
$imageSubform = $this->getFirstImageSubform();
$variantField = $imageSubform->find('css', '[data-test-product-variant]');

$selectedOption = $variantField->find('css', 'option[selected]');

if (null === $selectedOption) {
return null;
}

return $images->has('css', sprintf('[data-test-product-variant*="%s"]', $productVariant->getCode()));
return $selectedOption->getText();
}

public function countImages(): int
Expand Down Expand Up @@ -189,10 +207,10 @@ public function selectVariantForFirstImage(ProductVariantInterface $productVaria
$this->changeTab();

$imageSubform = $this->getFirstImageSubform();
$this->autocompleteHelper->selectByValue(
$this->autocompleteHelper->selectByName(
$this->getDriver(),
$imageSubform->find('css', '[data-test-product-variant]')->getXpath(),
$productVariant->getCode(),
$productVariant->getName(),
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public function hasImageWithType(string $type): bool;

public function hasImageWithVariant(ProductVariantInterface $productVariant): bool;

public function getFirstImageSelectedVariantName(): ?string;

public function countImages(): int;

public function getImages(): array;
Expand Down
15 changes: 14 additions & 1 deletion src/Sylius/Behat/Element/Admin/Taxon/FormElement.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,20 @@ public function describeItAs(string $description, string $localeCode): void

public function getParent(): string
{
return $this->getElement('parent')->getValue();
$parentElement = $this->getElement('parent');

if (DriverHelper::isJavascript($this->getDriver())) {
$items = $this->autocompleteHelper->getSelectedItems(
$this->getDriver(),
$parentElement->getXpath(),
);

return count($items) > 0 ? reset($items) : '';
}

$selectedOption = $parentElement->find('css', 'option[selected]');

return $selectedOption?->getText() ?? '';
}

public function chooseParent(TaxonInterface $taxon): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
'required' => false,
'multiple' => true,
'expanded' => false,
'choice_value' => 'code',
])
;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ public function configureOptions(OptionsResolver $resolver): void
$resolver->setDefaults([
'class' => $this->productAttributeClass,
'choice_name' => 'name',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Functional Critical

The ProductAttributeAutocompleteType is missing the 'choice_value' => 'code' configuration option. Without it, the autocomplete API endpoint returns entity IDs instead of attribute codes as option values. This causes the product form to send numeric IDs to Product/FormComponent::addAttributes(), which then fails to find attributes by code and throws a TypeError when calling isTranslatable() on a null object.

Suggested fix
            $resolver->setDefaults([
                'class' => $this->productAttributeClass,
                'choice_name' => 'name',
                'choice_value' => 'code',
            ]);
Prompt for AI assistance

Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:

You are an expert bash developer with deep knowledge of security, performance, and best practices.

### Context

File: src/Sylius/Bundle/AdminBundle/Form/Type/ProductAttributeAutocompleteType.php
Lines: 35-35
Issue Type: functional-critical
Severity: critical

Issue Description:
The `ProductAttributeAutocompleteType` is missing the `'choice_value' => 'code'` configuration option. Without it, the autocomplete API endpoint returns entity IDs instead of attribute codes as option values. This causes the product form to send numeric IDs to `Product/FormComponent::addAttributes()`, which then fails to find attributes by code and throws a TypeError when calling `isTranslatable()` on a null object.

Current Code:
            'choice_name' => 'name',

---

### Instructions

1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed

### Constraints

- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready

---


Like Dislike Create Issue

'choice_value' => 'code',
]);
}

Expand Down
Loading