Commit 68c1dbbbcb0 for woocommerce
commit 68c1dbbbcb00870ac5fb8339495f58600f4e9a72
Author: Rostislav Wolný <1082140+costasovo@users.noreply.github.com>
Date: Thu Sep 24 15:03:56 2026 +0200
[Email Editor] Fix duplicated block when applying a template update (#69007)
* Pair blocks through the base when applying email template updates
The selective applier paired core and merchant blocks with its own two-way LCS, while the change summary shown to the merchant pairs them through the last core render. When the merchant had deleted a block, the two pairings could disagree: core's edited block was left unmatched and inserted next to the merchant's copy, and any block the merchant deleted was re-inserted. The applier now uses the same base-anchored pairing as the summary, so Apply writes what the review drawer showed.
diff --git a/plugins/woocommerce/changelog/68898-fix-email-template-apply-duplicate-block b/plugins/woocommerce/changelog/68898-fix-email-template-apply-duplicate-block
new file mode 100644
index 00000000000..265b8f29bb7
--- /dev/null
+++ b/plugins/woocommerce/changelog/68898-fix-email-template-apply-duplicate-block
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Email editor: applying a template update no longer duplicates a block or restores a block the merchant deleted.
diff --git a/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateChangeSummary.php b/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateChangeSummary.php
index eefb3135ad0..f429496c6fd 100644
--- a/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateChangeSummary.php
+++ b/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateChangeSummary.php
@@ -699,19 +699,7 @@ class WCEmailTemplateChangeSummary {
* @since 10.9.0
*/
public static function diff_records_three_way( array $core_records, array $base_records, array $post_records ): array {
- $core_to_base = self::lcs_matches( $core_records, $base_records );
- $post_to_base = self::lcs_matches( $post_records, $base_records );
-
- // Invert into base-keyed lookups so a single iteration over base records
- // can decide each block's fate against both sides.
- $base_to_core = array();
- foreach ( $core_to_base as $pair ) {
- $base_to_core[ $pair[1] ] = $pair[0];
- }
- $base_to_post = array();
- foreach ( $post_to_base as $pair ) {
- $base_to_post[ $pair[1] ] = $pair[0];
- }
+ $alignment = self::align_to_base( $core_records, $base_records, $post_records );
$matched_core_indices = array();
$matched_post_indices = array();
@@ -726,8 +714,8 @@ class WCEmailTemplateChangeSummary {
// Pass 1: classify each base-anchored block by what changed relative to base.
foreach ( $base_records as $b_idx => $base ) {
- $core_idx = $base_to_core[ $b_idx ] ?? null;
- $post_idx = $base_to_post[ $b_idx ] ?? null;
+ $core_idx = $alignment[ $b_idx ]['core'];
+ $post_idx = $alignment[ $b_idx ]['post'];
if ( null !== $core_idx ) {
$matched_core_indices[ $core_idx ] = true;
@@ -864,6 +852,44 @@ class WCEmailTemplateChangeSummary {
);
}
+ /**
+ * Match each base block to its counterpart in core and in the post. A core
+ * block and a post block are the same logical block only when both match
+ * the same base block. `null` means that side no longer has the block.
+ *
+ * Public so {@see WCEmailTemplateSelectiveApplier} pairs blocks exactly the
+ * way the change summary shown to the merchant does.
+ *
+ * @internal
+ *
+ * @param array<int, array{path:array<int|string>, parent_name:?string, name:string, inner_text:string}> $core_records Core side (current canonical).
+ * @param array<int, array{path:array<int|string>, parent_name:?string, name:string, inner_text:string}> $base_records Base side (canonical at last system write).
+ * @param array<int, array{path:array<int|string>, parent_name:?string, name:string, inner_text:string}> $post_records Post side (merchant's current post_content).
+ *
+ * @return array<int, array{core:?int, post:?int}> Keyed by base record index; values are core and post record indices.
+ *
+ * @since 11.3.0
+ */
+ public static function align_to_base( array $core_records, array $base_records, array $post_records ): array {
+ $base_to_core = array();
+ foreach ( self::lcs_matches( $core_records, $base_records ) as $pair ) {
+ $base_to_core[ $pair[1] ] = $pair[0];
+ }
+ $base_to_post = array();
+ foreach ( self::lcs_matches( $post_records, $base_records ) as $pair ) {
+ $base_to_post[ $pair[1] ] = $pair[0];
+ }
+
+ $alignment = array();
+ foreach ( array_keys( $base_records ) as $b_idx ) {
+ $alignment[ $b_idx ] = array(
+ 'core' => $base_to_core[ $b_idx ] ?? null,
+ 'post' => $base_to_post[ $b_idx ] ?? null,
+ );
+ }
+ return $alignment;
+ }
+
/**
* Drop entries at the given indices and return a re-indexed list. Used by
* the reorder pass to remove paired entries from added/removed without
diff --git a/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplier.php b/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplier.php
index 6c4d50083a3..1aa6ebb8944 100644
--- a/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplier.php
+++ b/plugins/woocommerce/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplier.php
@@ -39,11 +39,11 @@ use Automattic\WooCommerce\Internal\EmailEditor\Logger;
* Three-way payload consumption (since 10.9.0): when the post has
* {@see WCEmailTemplateDivergenceDetector::LAST_CORE_RENDER_META_KEY} meta,
* `apply_selectively()` passes the change-summary's payload through to
- * `merge()`, which uses it to gate matched-pair classification:
+ * `merge()`, together with that base render:
*
- * - LCS pairs whose paths the summary classified as separate add+remove are
- * rejected by Pass 1 (preventing false yours+core pairings on parallel
- * additions); Pass 2/3 then handle them as two independent adds.
+ * - Core and post blocks are paired through the base, exactly as the summary
+ * pairs them, so the merge writes what the merchant reviewed. Blocks the
+ * merchant deleted are not re-inserted.
* - Matched pairs whose paths are NOT in `copy_changes` are silently
* preserved (yours-only edits aren't conflicts; the `use_core` decision
* is ignored on those paths).
@@ -188,13 +188,12 @@ class WCEmailTemplateSelectiveApplier {
}//end try
// When the post has `last_core_render` meta, the change-summary already classified
- // each block via three-way attribution (yours-vs-base, core-vs-base) and the merge
- // can consume that payload directly — gating use_core decisions to "real" conflicts
- // only and rejecting LCS pairs that the summary classified as separate add+remove.
+ // each block via three-way attribution (yours-vs-base, core-vs-base). The merge pairs
+ // blocks through the same base and only accepts use_core on the summary's conflicts.
$base_render_for_merge = (string) get_post_meta( $post_id, WCEmailTemplateDivergenceDetector::LAST_CORE_RENDER_META_KEY, true );
$summary_for_merge = '' !== $base_render_for_merge ? $summary : null;
- $merged_result = self::merge( $post_content, $core_content, $choices, $summary_for_merge );
+ $merged_result = self::merge( $post_content, $core_content, $choices, $summary_for_merge, $base_render_for_merge );
$merged_content = $merged_result['content'];
$structural_skipped = $merged_result['structural_skipped'];
$aliases_migrated = $merged_result['aliases_migrated'];
@@ -453,29 +452,28 @@ class WCEmailTemplateSelectiveApplier {
* layering on core's changes per the v1 algorithm.
*
* When `$precomputed_summary` is provided (the caller's `last_core_render`
- * meta was set, so the change-summary ran three-way attribution), the merge
- * defers to the summary's classification:
+ * meta was set, so the change-summary ran three-way attribution), core and
+ * post blocks are paired through `$base_content` with
+ * {@see WCEmailTemplateChangeSummary::align_to_base()}, the same pairing the
+ * summary shown to the merchant uses:
*
- * - Matched pairs whose path is in `removed_blocks` (yours-only) or
- * `added_blocks` (core-only) are REJECTED — the summary correctly
- * identified them as separate adds; the local LCS may have falsely
- * paired them by name. The reject lets Pass 2 / Pass 3 handle them.
- * - Matched pairs not in `copy_changes` are silent — Pass 1 skips them
- * even if a `use_core` decision was passed (yours-only edit, no
- * conflict to resolve).
+ * - Only pairs whose path is in `copy_changes` accept `use_core`; yours-only
+ * edits are preserved even if a `use_core` decision was passed.
+ * - Core blocks the merchant deleted are not re-inserted.
*
- * Without `$precomputed_summary` (legacy two-way fallback), the existing
- * behavior is preserved: every matched pair with differing inner_text is
- * eligible for `use_core`, and the local LCS drives matched-set tracking.
+ * Without `$precomputed_summary` (legacy two-way fallback), core and post
+ * are paired with a direct LCS and every matched pair with differing
+ * inner_text is eligible for `use_core`.
*
* @param string $post_content Merchant's current `post_content`.
* @param string $core_content Canonical core render.
* @param array<int, array{path:array<int|string>, decision:string}> $choices Per-conflict choices.
* @param array<string, mixed>|null $precomputed_summary Optional three-way summary payload from {@see WCEmailTemplateChangeSummary::summarize()}; pass `null` to use the legacy two-way merge.
+ * @param string $base_content The `last_core_render` the summary was computed against. Used only with `$precomputed_summary`.
*
* @return array{content:string, structural_skipped:bool, aliases_migrated:string[]}
*/
- private static function merge( string $post_content, string $core_content, array $choices, ?array $precomputed_summary = null ): array {
+ private static function merge( string $post_content, string $core_content, array $choices, ?array $precomputed_summary = null, string $base_content = '' ): array {
$post_blocks = parse_blocks( $post_content );
$core_blocks = parse_blocks( $core_content );
@@ -489,7 +487,24 @@ class WCEmailTemplateSelectiveApplier {
$post_records = WCEmailTemplateChangeSummary::flatten_blocks( $post_blocks );
$core_records = WCEmailTemplateChangeSummary::flatten_blocks( $core_blocks );
- $matches = WCEmailTemplateChangeSummary::lcs_matches( $core_records, $post_records );
+
+ $merchant_removed_core_set = array();
+ if ( null !== $precomputed_summary ) {
+ $base_records = WCEmailTemplateChangeSummary::flatten_blocks( parse_blocks( $base_content ) );
+ $matches = array();
+ foreach ( WCEmailTemplateChangeSummary::align_to_base( $core_records, $base_records, $post_records ) as $sides ) {
+ if ( null === $sides['core'] ) {
+ continue;
+ }
+ if ( null === $sides['post'] ) {
+ $merchant_removed_core_set[ $sides['core'] ] = true;
+ continue;
+ }
+ $matches[] = array( $sides['core'], $sides['post'] );
+ }
+ } else {
+ $matches = WCEmailTemplateChangeSummary::lcs_matches( $core_records, $post_records );
+ }
$choice_map = array();
foreach ( $choices as $choice ) {
@@ -503,11 +518,8 @@ class WCEmailTemplateSelectiveApplier {
$choice_map[ self::path_key( $choice['path'] ) ] = $decision;
}
- // Three-way overrides derived from the precomputed summary. `null`
- // signals the legacy two-way path (no gating).
+ // `null` signals the legacy two-way path (every differing pair is eligible).
$copy_change_paths = null;
- $added_path_keys = array();
- $removed_path_keys = array();
if ( null !== $precomputed_summary ) {
$copy_change_paths = array();
foreach ( $precomputed_summary['copy_changes'] ?? array() as $cc ) {
@@ -515,16 +527,6 @@ class WCEmailTemplateSelectiveApplier {
$copy_change_paths[ self::path_key( $cc['path'] ) ] = true;
}
}
- foreach ( $precomputed_summary['added_blocks'] ?? array() as $ab ) {
- if ( isset( $ab['path'] ) && is_array( $ab['path'] ) ) {
- $added_path_keys[ self::path_key( $ab['path'] ) ] = true;
- }
- }
- foreach ( $precomputed_summary['removed_blocks'] ?? array() as $rb ) {
- if ( isset( $rb['path'] ) && is_array( $rb['path'] ) ) {
- $removed_path_keys[ self::path_key( $rb['path'] ) ] = true;
- }
- }
}
// Pass 1: matched pairs. Apply use_core decisions on copy changes;
@@ -536,18 +538,8 @@ class WCEmailTemplateSelectiveApplier {
foreach ( $matches as $pair ) {
$core_rec = $core_records[ $pair[0] ];
$post_rec = $post_records[ $pair[1] ];
- $core_key = self::path_key( $core_rec['path'] );
$post_key = self::path_key( $post_rec['path'] );
- // Three-way reject: applier's LCS paired these but the summary
- // classified them as separate add+remove. Don't track as matched
- // (so Pass 2 / Pass 3 will handle them) and don't apply.
- if ( null !== $precomputed_summary
- && ( isset( $added_path_keys[ $core_key ] ) || isset( $removed_path_keys[ $post_key ] ) )
- ) {
- continue;
- }
-
$matched_core_set[ $pair[0] ] = true;
$matched_post_set[ $pair[1] ] = true;
@@ -589,6 +581,9 @@ class WCEmailTemplateSelectiveApplier {
$structural_skipped = true;
continue;
}
+ if ( isset( $merchant_removed_core_set[ $i ] ) ) {
+ continue;
+ }
$core_block = self::block_at_path( $core_blocks, $rec['path'] );
if ( null === $core_block ) {
continue;
diff --git a/plugins/woocommerce/tests/php/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplierTest.php b/plugins/woocommerce/tests/php/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplierTest.php
index 79b2f043cc7..8edb3ab7990 100644
--- a/plugins/woocommerce/tests/php/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplierTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/EmailEditor/WCTransactionalEmails/WCEmailTemplateSelectiveApplierTest.php
@@ -401,6 +401,57 @@ class WCEmailTemplateSelectiveApplierTest extends \WC_Unit_Test_Case {
);
}
+ /**
+ * @testdox Three-way: when the merchant deleted one block and core edited another, apply updates the right block and adds no duplicate.
+ *
+ * @testWith [[], ["OLD BLOCK A", "MERCHANT EDITED C"]]
+ * [[{"path": [0], "decision": "use_core"}], ["NEW CORE A", "MERCHANT EDITED C"]]
+ *
+ * @param array<int, array{path:array<int>, decision:string}> $choices Choices sent to apply.
+ * @param string[] $expected Paragraph texts expected in the merged content, in order.
+ */
+ public function test_apply_selectively_three_way_merchant_deletion_and_core_edit( array $choices, array $expected ): void {
+ $email_id = 'sa_three_way_deletion_and_edit';
+ $this->register_fixture_email( $email_id );
+
+ $this->use_canonical_content( $email_id, $this->paragraphs( array( 'NEW CORE A', 'OLD BLOCK B', 'OLD BLOCK C' ) ) );
+ $post_id = $this->create_woo_email_post( $email_id, $this->paragraphs( array( 'OLD BLOCK A', 'MERCHANT EDITED C' ) ) );
+ update_post_meta(
+ $post_id,
+ WCEmailTemplateDivergenceDetector::LAST_CORE_RENDER_META_KEY,
+ $this->paragraphs( array( 'OLD BLOCK A', 'OLD BLOCK B', 'OLD BLOCK C' ) )
+ );
+
+ $result = WCEmailTemplateSelectiveApplier::apply_selectively( $post_id, $choices );
+
+ $this->assertIsArray( $result );
+ $this->assertSame( $expected, $this->paragraph_texts( $result['merged_content'] ) );
+ }
+
+ /**
+ * @testdox Three-way: a block the merchant deleted is not re-added when core adds a new block elsewhere.
+ */
+ public function test_apply_selectively_three_way_does_not_restore_merchant_deleted_block(): void {
+ $email_id = 'sa_three_way_deletion_and_addition';
+ $this->register_fixture_email( $email_id );
+
+ $this->use_canonical_content( $email_id, $this->paragraphs( array( 'Thanks for your order.', 'Here are the details.', 'See you soon.', 'PS from core.' ) ) );
+ $post_id = $this->create_woo_email_post( $email_id, $this->paragraphs( array( 'Thanks for your order.', 'See you soon.' ) ) );
+ update_post_meta(
+ $post_id,
+ WCEmailTemplateDivergenceDetector::LAST_CORE_RENDER_META_KEY,
+ $this->paragraphs( array( 'Thanks for your order.', 'Here are the details.', 'See you soon.' ) )
+ );
+
+ $result = WCEmailTemplateSelectiveApplier::apply_selectively( $post_id, array() );
+
+ $this->assertIsArray( $result );
+ $this->assertSame(
+ array( 'Thanks for your order.', 'See you soon.', 'PS from core.' ),
+ $this->paragraph_texts( $result['merged_content'] )
+ );
+ }
+
/**
* @testdox Should stamp _wc_email_template_last_core_render with current canonical (not merged content) after apply.
*
@@ -1087,6 +1138,38 @@ class WCEmailTemplateSelectiveApplierTest extends \WC_Unit_Test_Case {
return (int) $post_id;
}
+ /**
+ * Build block markup with one paragraph block per text.
+ *
+ * @param string[] $texts Paragraph texts.
+ * @return string Serialized block markup.
+ */
+ private function paragraphs( array $texts ): string {
+ return implode(
+ "\n\n",
+ array_map(
+ static fn( string $text ): string => "<!-- wp:paragraph -->\n<p>{$text}</p>\n<!-- /wp:paragraph -->",
+ $texts
+ )
+ );
+ }
+
+ /**
+ * Extract the text of each top-level paragraph block, in order.
+ *
+ * @param string $content Serialized block markup.
+ * @return string[] Paragraph texts.
+ */
+ private function paragraph_texts( string $content ): array {
+ $texts = array();
+ foreach ( parse_blocks( $content ) as $block ) {
+ if ( 'core/paragraph' === $block['blockName'] ) {
+ $texts[] = trim( wp_strip_all_tags( (string) $block['innerHTML'] ) );
+ }
+ }
+ return $texts;
+ }
+
/**
* Remove any stubs we injected into WC_Emails::$emails during the test.
*/