Skip to content

Commit fdaf300

Browse files
committed
Fix: variant selector missing on freshly-added price tier rows
The PRE_SET_DATA listener in PriceTierType added the productVariant field only when $priceTier->getProduct() was non-null. Existing tiers loaded from the DB had it set; brand-new tiers added via the LiveCollectionType "Add" button did not, because Live Components builds the new row's form *before* ProductTrait::addPriceTier() assigns the backreference. Mirror Sylius core's product-images pattern: forward the parent product down through entry_options. - PriceTierType: drop the PRE_SET_DATA listener; add a `product` option (ProductInterface|null, default null); add the productVariant field unconditionally whenever the option is set. - PriceTierCollectionType: replace the static entry_options default with a normalizer that always merges `label => false` with whatever the caller passes (so an externally-passed `product` doesn't wipe the label default). - ProductTypeExtension: pass `entry_options.product => $options['data']` when adding the priceTiers field; falls back to null on the create page where the parent product hasn't been instantiated yet. Test updates (form tests): - PriceTierTypeTest: drop the PRE_SET_DATA-based tests, replace with option-driven ones (product passed -> variant field present; option absent or null -> field absent). - PriceTierCollectionTypeTest: add a normalizer-merge test ensuring caller-passed entry_options (e.g. `product`) round-trip alongside `label => false`. - ProductTypeExtensionTest: rewrite around a captured-args helper so both the no-data create-page case and the data-bound update-page case can verify entry_options.product is forwarded. CLAUDE.md gains a convention bullet warning against the PRE_SET_DATA pattern for context-dependent per-entry fields. 61 tests / 118 assertions, ECS + PHPStan max all green. Browser- verified in Playwright: clicking Add on product 37's price-tiers tab produces a second row that renders the variant selector identically to the first.
1 parent c18a348 commit fdaf300

8 files changed

Lines changed: 114 additions & 51 deletions

CLAUDE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ Wiring:
5454

5555
- `SetonoSyliusTierPricingPlugin` extends `AbstractResourceBundle` + `SyliusPluginTrait`, ORM-only. It overrides **both** `getPath()` (returns `dirname(__DIR__)` so Sylius sees the repo root) **and** `getConfigFilesPath()` (returns `<root>/config/doctrine/<format>` — without this override, Sylius' `AbstractResourceBundle::getConfigFilesPath()` would still look under `Resources/config/doctrine/`).
5656
- DI services live in `config/services/*.php` (PHP DSL via `ContainerConfigurator`), imported from `config/services.php`, loaded by the extension via `PhpFileLoader`. **Do not** ship XML service config — the Setono Sylius-2 convention is PHP for IDE refactoring, PHPStan analysis at compile time, and FQCN service ids. Each leaf file starts with `namespace Symfony\Component\DependencyInjection\Loader\Configurator;` so `service()`, `param()`, etc. resolve as bare function calls. The extension implements `PrependExtensionInterface`: its `prepend()` injects the plugin's `sylius_twig_hooks` configuration directly via `prependExtensionConfig()` so consumers don't need to import anything. **Convention:** other-bundle configuration belongs in `prepend()` and **must be inlined as a PHP array** — do not read/parse a YAML file from disk and forward it.
57-
- `Form\Extension\ProductTypeExtension` adds the `priceTiers` field to the Sylius `ProductType`. `Form\Type\PriceTierCollectionType` extends `Symfony\UX\LiveComponent\Form\Type\LiveCollectionType` (mirroring how Sylius admin handles product images) so add/delete fire server-side via Symfony UX Live Components — no client-side prototype-cloning JS. To survive LiveCollectionType's empty-bind cycle on `addCollectionItem`, `PriceTier::setQuantity()` and `setDiscount()` are nullable-and-no-op-on-null rather than using form-level `empty_data` defaults — keeps the workaround out of `PriceTierType` and makes the model the single source of truth for its defaults (`quantity = 1`, `discount = '0.0'`).
57+
- `Form\Extension\ProductTypeExtension` adds the `priceTiers` field to the Sylius `ProductType` and forwards the parent product down as `entry_options.product` so each per-row `PriceTierType` (including freshly-added rows that aren't yet linked to a product) can render the variant selector for the current product. `Form\Type\PriceTierCollectionType` extends `Symfony\UX\LiveComponent\Form\Type\LiveCollectionType` (mirroring how Sylius admin handles product images) so add/delete fire server-side via Symfony UX Live Components — no client-side prototype-cloning JS. It uses an `entry_options` normalizer to always merge `label => false` with whatever the caller passes (so the product passed down by `ProductTypeExtension` doesn't wipe the label default). To survive LiveCollectionType's empty-bind cycle on `addCollectionItem`, `PriceTier::setQuantity()` and `setDiscount()` are nullable-and-no-op-on-null rather than using form-level `empty_data` defaults — keeps the workaround out of `PriceTierType` and makes the model the single source of truth for its defaults (`quantity = 1`, `discount = '0.0'`).
5858
- The admin product tab is rendered by Twig hooks (`templates/admin/product/form/{side_navigation,sections}/price_tiers.html.twig`) registered against `sylius_admin.product.{update,create}.content.form.{side_navigation,sections}` — the canonical Sylius 2 hook points (declared by the core in `vendor/sylius/sylius/src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/product/update.yaml`). `ProductFormMenuSubscriber` is gone in v2 — Sylius 2 has no `ProductMenuBuilderEvent`.
5959

6060
## Conventions
@@ -65,6 +65,7 @@ Wiring:
6565
- CI matrix exercises PHP 8.2/8.3/8.4 × Symfony 6.4/7.4 × lowest/highest deps via the `setono/sylius-plugin/<job>@v2` composite actions. Keep changes compatible across that matrix.
6666
- For Doctrine: ORM 3 is in play (Sylius 2 pulls it). Use `#[ORM\*]` attributes — not `@ORM\*` PHPDoc tags, which are silently ignored in attribute-mapped entities.
6767
- Required scalar setters on entities that participate in a `LiveCollectionType` should accept null and treat it as a no-op (keeping the existing value), instead of forcing `empty_data` defaults at the form level. LiveCollectionType re-binds the parent form on `addCollectionItem` before the user types anything; non-nullable setters explode with `InvalidTypeException`, and pushing the default to the form means the model has two sources of truth for its initial state.
68+
- Per-entry form fields inside a `LiveCollectionType` that depend on **parent-entity context** (e.g. the variant selector needing to know which product owns the tier) must receive that context via `entry_options` from the outer form extension, not via a `PRE_SET_DATA` listener on the entry type that reads `$entry->getParent()`. The PRE_SET_DATA approach fires for entities loaded from the DB but silently omits the field on freshly-added rows because Live Components build the empty entry's form *before* the parent's `add*()` assigns the backreference.
6869
- Do **not** revert `getConfigFilesPath()` to default — Doctrine mapping discovery breaks immediately.
6970
- Configuration of other bundles lives in `SetonoSyliusTierPricingExtension::prepend()` as inlined PHP arrays passed to `prependExtensionConfig()`. Do not read YAML files from disk inside `prepend()` and forward the parsed result.
7071
- DI service configuration is **PHP DSL**, not XML. New services go under `config/services/<topic>.php` using `ContainerConfigurator`. Use the FQCN as the service id and alias `*Interface` to it for forward compatibility.
40.9 KB
Loading

src/Form/Extension/ProductTypeExtension.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Setono\SyliusTierPricingPlugin\Form\Type\PriceTierCollectionType;
88
use Sylius\Bundle\ProductBundle\Form\Type\ProductType;
9+
use Sylius\Component\Product\Model\ProductInterface;
910
use Symfony\Component\Form\AbstractTypeExtension;
1011
use Symfony\Component\Form\FormBuilderInterface;
1112
use Symfony\Component\Validator\Constraints\Valid;
@@ -14,9 +15,16 @@ final class ProductTypeExtension extends AbstractTypeExtension
1415
{
1516
public function buildForm(FormBuilderInterface $builder, array $options): void
1617
{
18+
$product = $options['data'] ?? null;
19+
1720
$builder->add('priceTiers', PriceTierCollectionType::class, [
1821
'label' => false,
1922
'constraints' => [new Valid()],
23+
'entry_options' => [
24+
// Forward the parent product down so each PriceTierType (including freshly-added rows the
25+
// user hasn't yet linked) can render the variant-selection field for *this* product.
26+
'product' => $product instanceof ProductInterface ? $product : null,
27+
],
2028
]);
2129
}
2230

src/Form/Type/PriceTierCollectionType.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Setono\SyliusTierPricingPlugin\Form\Type;
66

77
use Symfony\Component\Form\AbstractType;
8+
use Symfony\Component\OptionsResolver\Options;
89
use Symfony\Component\OptionsResolver\OptionsResolver;
910
use Symfony\UX\LiveComponent\Form\Type\LiveCollectionType;
1011

@@ -18,7 +19,7 @@ public function configureOptions(OptionsResolver $resolver): void
1819
$resolver
1920
->setDefaults([
2021
'entry_type' => PriceTierType::class,
21-
'entry_options' => ['label' => false],
22+
'entry_options' => [],
2223
'allow_add' => true,
2324
'allow_delete' => true,
2425
'by_reference' => false,
@@ -27,6 +28,10 @@ public function configureOptions(OptionsResolver $resolver): void
2728
'label' => 'setono_sylius_tier_pricing.ui.add_price_tier',
2829
],
2930
])
31+
->setNormalizer('entry_options', static function (Options $options, mixed $value): array {
32+
// Always set label=false; merge with anything the caller passed (e.g. `product` from ProductTypeExtension).
33+
return array_merge(['label' => false], is_array($value) ? $value : []);
34+
})
3035
;
3136
}
3237

src/Form/Type/PriceTierType.php

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,14 @@
44

55
namespace Setono\SyliusTierPricingPlugin\Form\Type;
66

7-
use Setono\SyliusTierPricingPlugin\Model\PriceTierInterface;
87
use Sylius\Bundle\ChannelBundle\Form\Type\ChannelChoiceType;
98
use Sylius\Bundle\ProductBundle\Form\Type\ProductVariantChoiceType;
109
use Sylius\Bundle\ResourceBundle\Form\Type\AbstractResourceType;
11-
use Symfony\Component\Form\Event\PreSetDataEvent;
10+
use Sylius\Component\Product\Model\ProductInterface;
1211
use Symfony\Component\Form\Extension\Core\Type\IntegerType;
1312
use Symfony\Component\Form\Extension\Core\Type\NumberType;
1413
use Symfony\Component\Form\FormBuilderInterface;
15-
use Symfony\Component\Form\FormEvents;
16-
use Webmozart\Assert\Assert;
14+
use Symfony\Component\OptionsResolver\OptionsResolver;
1715

1816
final class PriceTierType extends AbstractResourceType
1917
{
@@ -32,26 +30,24 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
3230
'required' => false,
3331
]);
3432

35-
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (PreSetDataEvent $event): void {
36-
/** @var PriceTierInterface|null $priceTier */
37-
$priceTier = $event->getData();
38-
Assert::nullOrIsInstanceOf($priceTier, PriceTierInterface::class);
39-
40-
if (null === $priceTier) {
41-
return;
42-
}
43-
44-
$product = $priceTier->getProduct();
45-
if (null === $product) {
46-
return;
47-
}
48-
49-
$event->getForm()->add('productVariant', ProductVariantChoiceType::class, [
33+
$product = $options['product'];
34+
if ($product instanceof ProductInterface) {
35+
$builder->add('productVariant', ProductVariantChoiceType::class, [
5036
'label' => 'sylius.ui.variant',
5137
'product' => $product,
5238
'required' => false,
5339
]);
54-
});
40+
}
41+
}
42+
43+
public function configureOptions(OptionsResolver $resolver): void
44+
{
45+
parent::configureOptions($resolver);
46+
47+
$resolver
48+
->setDefault('product', null)
49+
->setAllowedTypes('product', [ProductInterface::class, 'null'])
50+
;
5551
}
5652

5753
public function getBlockPrefix(): string

tests/Form/Extension/ProductTypeExtensionTest.php

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Prophecy\PhpUnit\ProphecyTrait;
1111
use Setono\SyliusTierPricingPlugin\Form\Extension\ProductTypeExtension;
1212
use Setono\SyliusTierPricingPlugin\Form\Type\PriceTierCollectionType;
13+
use Setono\SyliusTierPricingPlugin\Tests\Model\Fixture\ProductTraitFixture;
1314
use Sylius\Bundle\ProductBundle\Form\Type\ProductType;
1415
use Symfony\Component\Form\FormBuilderInterface;
1516
use Symfony\Component\Validator\Constraints\Valid;
@@ -29,28 +30,69 @@ public function it_extends_the_sylius_product_form_type(): void
2930
}
3031

3132
#[Test]
32-
public function it_adds_the_price_tiers_field_with_a_valid_constraint_and_no_label(): void
33+
public function it_adds_the_price_tiers_field_with_no_label_a_valid_constraint_and_the_parent_product_forwarded(): void
3334
{
35+
$product = new ProductTraitFixture();
36+
37+
$captured = $this->captureAddCall(['data' => $product]);
38+
39+
self::assertSame('priceTiers', $captured['name']);
40+
self::assertSame(PriceTierCollectionType::class, $captured['type']);
41+
42+
$options = $captured['options'];
43+
self::assertFalse($options['label']);
44+
45+
$constraints = $options['constraints'];
46+
self::assertIsArray($constraints);
47+
self::assertCount(1, $constraints);
48+
self::assertInstanceOf(Valid::class, $constraints[0]);
49+
50+
$entryOptions = $options['entry_options'];
51+
self::assertIsArray($entryOptions);
52+
self::assertSame($product, $entryOptions['product']);
53+
}
54+
55+
#[Test]
56+
public function it_passes_null_product_when_the_parent_form_has_no_data_yet_eg_on_a_create_page(): void
57+
{
58+
$captured = $this->captureAddCall([]);
59+
60+
$entryOptions = $captured['options']['entry_options'];
61+
self::assertIsArray($entryOptions);
62+
self::assertArrayHasKey('product', $entryOptions);
63+
self::assertNull($entryOptions['product']);
64+
}
65+
66+
/**
67+
* @param array<string, mixed> $formOptions
68+
*
69+
* @return array{name: string, type: string, options: array<string, mixed>}
70+
*/
71+
private function captureAddCall(array $formOptions): array
72+
{
73+
/** @var array{name: string, type: string, options: array<string, mixed>}|null $captured */
74+
$captured = null;
3475
$builder = $this->prophesize(FormBuilderInterface::class);
3576
$builder
36-
->add(
37-
'priceTiers',
38-
PriceTierCollectionType::class,
39-
Argument::that(static function (array $options): bool {
40-
if (false !== $options['label']) {
41-
return false;
42-
}
43-
44-
if (!isset($options['constraints']) || !\is_array($options['constraints'])) {
45-
return false;
46-
}
47-
48-
return 1 === count($options['constraints']) && $options['constraints'][0] instanceof Valid;
49-
}),
50-
)
77+
->add(Argument::cetera())
5178
->shouldBeCalledOnce()
52-
->willReturn($builder->reveal());
79+
->will(function (array $args) use (&$captured, $builder): FormBuilderInterface {
80+
$name = $args[0];
81+
$type = $args[1];
82+
/** @var array<string, mixed> $options */
83+
$options = $args[2];
84+
assert(is_string($name));
85+
assert(is_string($type));
86+
87+
$captured = ['name' => $name, 'type' => $type, 'options' => $options];
88+
89+
return $builder->reveal();
90+
});
91+
92+
(new ProductTypeExtension())->buildForm($builder->reveal(), $formOptions);
93+
94+
self::assertNotNull($captured);
5395

54-
(new ProductTypeExtension())->buildForm($builder->reveal(), []);
96+
return $captured;
5597
}
5698
}

tests/Form/Type/PriceTierCollectionTypeTest.php

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Setono\SyliusTierPricingPlugin\Form\Type\PriceTierCollectionType;
1010
use Setono\SyliusTierPricingPlugin\Form\Type\PriceTierType;
1111
use Setono\SyliusTierPricingPlugin\Model\PriceTier;
12+
use Setono\SyliusTierPricingPlugin\Tests\Model\Fixture\ProductTraitFixture;
1213
use Sylius\Bundle\ChannelBundle\Form\Type\ChannelChoiceType;
1314
use Sylius\Bundle\ProductBundle\Form\Type\ProductVariantChoiceType;
1415
use Sylius\Component\Core\Model\Channel;
@@ -51,6 +52,18 @@ public function it_uses_price_tier_type_as_the_entry_type_with_a_blank_per_row_l
5152
self::assertSame(['label' => false], $options['entry_options']);
5253
}
5354

55+
#[Test]
56+
public function entry_options_normalizer_keeps_label_false_when_the_caller_passes_extra_options(): void
57+
{
58+
$product = new ProductTraitFixture();
59+
$options = $this->resolveOptions(['entry_options' => ['product' => $product]]);
60+
61+
$entryOptions = $options['entry_options'];
62+
self::assertIsArray($entryOptions);
63+
self::assertFalse($entryOptions['label']);
64+
self::assertSame($product, $entryOptions['product']);
65+
}
66+
5467
#[Test]
5568
public function it_allows_adding_and_deleting_rows(): void
5669
{
@@ -175,15 +188,17 @@ protected function getTypes(): array
175188
}
176189

177190
/**
191+
* @param array<string, mixed> $overrides
192+
*
178193
* @return array<string, mixed>
179194
*/
180-
private function resolveOptions(): array
195+
private function resolveOptions(array $overrides = []): array
181196
{
182197
$resolver = new OptionsResolver();
183198
(new PriceTierCollectionType())->configureOptions($resolver);
184199

185200
/** @var array<string, mixed> $resolved */
186-
$resolved = $resolver->resolve();
201+
$resolved = $resolver->resolve($overrides);
187202

188203
return $resolved;
189204
}

tests/Form/Type/PriceTierTypeTest.php

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,33 +76,29 @@ public function empty_quantity_and_discount_are_no_ops_so_live_collection_re_bin
7676
}
7777

7878
#[Test]
79-
public function it_adds_the_product_variant_field_when_the_price_tier_has_a_product(): void
79+
public function it_adds_the_product_variant_field_when_a_product_option_is_passed(): void
8080
{
8181
// A product with no variants keeps the test independent of Sylius's translatable variant labelling
8282
// (ProductVariantChoiceType labels by name, which requires a current locale on the variant).
8383
$product = new ProductTraitFixture();
84-
$priceTier = new PriceTier();
85-
$priceTier->setProduct($product);
8684

87-
$form = $this->factory->create(PriceTierType::class, $priceTier);
85+
$form = $this->factory->create(PriceTierType::class, null, ['product' => $product]);
8886

8987
self::assertTrue($form->has('productVariant'));
9088
}
9189

9290
#[Test]
93-
public function it_does_not_add_the_product_variant_field_when_no_initial_data_is_given(): void
91+
public function it_does_not_add_the_product_variant_field_when_no_product_option_is_passed(): void
9492
{
9593
$form = $this->factory->create(PriceTierType::class);
9694

9795
self::assertFalse($form->has('productVariant'));
9896
}
9997

10098
#[Test]
101-
public function it_does_not_add_the_product_variant_field_when_the_price_tier_has_no_product(): void
99+
public function it_does_not_add_the_product_variant_field_when_the_product_option_is_null(): void
102100
{
103-
$priceTier = new PriceTier();
104-
105-
$form = $this->factory->create(PriceTierType::class, $priceTier);
101+
$form = $this->factory->create(PriceTierType::class, null, ['product' => null]);
106102

107103
self::assertFalse($form->has('productVariant'));
108104
}

0 commit comments

Comments
 (0)