[AdminBundle] Pricing table on the product show page renders `<td>` outside of `<tr>` for channel pricings without a matching channel

Sylius version(s) affected

All 2.X

Description

On the admin product show page, the Pricing table iterates over variant.channelPricings and looks each pricing's channel up in the channels map built from product.channels. When a ChannelPricing exists for a channel the product is not assigned to, the lookup yields null and the template takes an {% else %} branch that renders the pricing#body hook without a surrounding <tr>.

src/Sylius/Bundle/AdminBundle/templates/product/show/content/page_body/general/simple_product/details/pricing.html.twig (lines 16-29):

{% for channel_pricing in variant.channelPricings %}
    {% set channel = attribute(channels, channel_pricing.channelCode) is defined ? attribute(channels, channel_pricing.channelCode) : null %}

    {% if channel %}
        {% set currency_code = channel.baseCurrency.code %}
        {% set channel = attribute(channels, channel_pricing.channelCode) %}

        <tr {{ sylius_test_html_attribute('simple-product', "%s"|format(channel.code)) }} {{ sylius_test_html_attribute('pricing', "%s"|format(channel.code)) }} >
            {% hook 'pricing#body' with { variant, channel, channel_pricing, currency_code } %}
        </tr>
    {% else %}
        {% hook 'pricing#body' with { variant, channel_pricing } %}   {# <-- no <tr> wrapper #}
    {% endif %}
{% endfor %}

.../general/configurable_product/details/pricing.html.twig (lines 16-28) has the same defect.

All six hookables registered for pricing#body (AdminBundle/Resources/config/app/twig_hooks/product/show.yaml, lines 203-221 and 309-327) emit exactly one <td> each, so the {% else %} branch injects six <td> elements directly into <tbody>. That is invalid HTML, and the browser's error recovery produces visibly wrong output:

Simple product — DEMO_EU (live), GHOST_A + GHOST_B (no matching channel), DEMO_UK (live). Chrome parses <tbody> into rows of 6, 12 and 6 cells:

Configurable product variant — DEMO_EU (live), GHOST_A … GHOST_D (no matching channel), DEMO_UK (live). Rows of 6, 24 and 6 cells; the merged row runs past the card and is cut off:

Orphan rows also show prices in the wrong currency

The orphaned cells are not reliably blank either. channel is re-assigned (to null) on every iteration, and hookable_metadata.context is a DataBag whose offsetExists() uses isset(), so is defined is false for it and the channel-dependent cells (channel, lowest_price_before_the_discount, price_history) fall back to <td></td> as intended.

currency_code, however, is only assigned inside {% if channel %} and never reset. {% hook %} without only merges the calling template's variables into the hookable context (HooksRuntime::getContext(): array_merge($twigVars, $context, $hookContext)), so an orphan that follows a live row still sees that row's currency_code, and price.html.twig / original_price.html.twig render the orphan's amount in that currency. In the first screenshot GHOST_A (1000) and GHOST_B (2000) are shown as €10.00 and €20.00, the currency carried over from DEMO_EU. Only orphans that come before any live row render empty price cells.

Why a channel pricing can have no matching channel

channels is built in .../page_body/general.html.twig (lines 2-5) from product.channels, i.e. only the channels the product is currently assigned to. ChannelPricing rows are never cleaned up when that assignment goes away:

• Product removed from a channel — ProductVariantInterface::removeChannelPricing() is never called anywhere in application code (it only exists on the model), so the pricing rows survive the unassignment.
• Channel deleted — ChannelPricing.orm.xml maps channelCode as a plain string column with no foreign key to sylius_channel, so deleting a channel orphans its pricing rows permanently.

How to reproduce

1. Install Sylius 2.2 with the standard fixtures.
2. In the admin, create two extra channels (e.g. GHOST_A, GHOST_B) and assign a simple product to them, giving it a price in each.
3. Delete both channels (Admin → Configuration → Channels), or edit the product and remove it from both channels.
4. Open Admin → Products → (that product) → show.

The Pricing card now renders one row containing 12 cells under a 6-column header instead of two rows of 6 (first screenshot), with the orphaned prices formatted in the currency of the preceding live row. With more consecutive orphans the table outgrows the card (second screenshot).

For a deterministic reproduction without going through the UI, insert two orphaned pricings directly (adjust product_variant_id):

INSERT INTO sylius_channel_pricing (product_variant_id, channel_code, price, minimum_price)
VALUES (1, 'GHOST_A', 1000, 0), (1, 'GHOST_B', 2000, 0);

then open the show page for that product. Inspecting <tbody> in the browser dev tools shows the merged row; viewing the raw response shows the <td> elements sitting directly inside <tbody>.

The same applies to configurable products via .../configurable_product/details/pricing.html.twig.

Possible Solution

Two options, depending on the intended behaviour:

A. Always emit the row wrapper — smallest, purely presentational fix. Move the <tr> outside the conditional so both branches are wrapped, keeping the current "show the pricing with blank cells" behaviour:

{% for channel_pricing in variant.channelPricings %}
    {% set channel = attribute(channels, channel_pricing.channelCode) is defined ? attribute(channels, channel_pricing.channelCode) : null %}

    <tr {{ sylius_test_html_attribute('simple-product', "%s"|format(channel_pricing.channelCode)) }} {{ sylius_test_html_attribute('pricing', "%s"|format(channel_pricing.channelCode)) }}>
        {% if channel %}
            {% hook 'pricing#body' with { variant, channel, channel_pricing, currency_code: channel.baseCurrency.code } %}
        {% else %}
            {% hook 'pricing#body' with { variant, channel_pricing } %}
        {% endif %}
    </tr>
{% endfor %}

Passing currency_code inline instead of {% set %}-ing it in the loop also removes the currency leak described above. Verified with this change applied to both templates on the two scenarios from the screenshots: every pricing gets its own 6-cell <tr> (4 and 6 rows respectively), no <td> sits outside a <tr>, and the orphan rows no longer show a price.

B. Skip orphaned pricings entirely — arguably more correct, since a row of six blank cells conveys nothing to the admin. It does, however, hide the fact that stale pricing data exists.

A middle ground would be A plus rendering the raw channel_pricing.channelCode in the first cell so the row is identifiable.

Whichever is chosen, the fix should be applied to both simple_product/details/pricing.html.twig and configurable_product/details/pricing.html.twig, since they are identical in this respect.

Unrelated, but in the same block: simple_product/details/pricing.html.twig line 21 re-assigns channel to the value it already holds from line 17 and can be dropped.

I am happy to open a PR for whichever option the team prefers.

Additional Context

• Affected templates:
  • src/Sylius/Bundle/AdminBundle/templates/product/show/content/page_body/general/simple_product/details/pricing.html.twig
  • src/Sylius/Bundle/AdminBundle/templates/product/show/content/page_body/general/configurable_product/details/pricing.html.twig
• Hook configuration: src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/product/show.yaml (lines 203-221, 309-327)
• Context merging that carries currency_code into later iterations: sylius/twig-hooks src/Twig/Runtime/HooksRuntime.php (getContext()), and src/Bag/DataBag.php (offsetExists() uses isset())