Issue #3588582: Fix selection criteria duplicated on legacy patterns

Symptom

After upgrading to 8.x-1.15, re-saving a pathauto_pattern whose selection_criteria array contains a non-UUID key (e.g. the integer 0) duplicates each "Content type" / language condition. Reported by kenwest.

Cause

PatternEditForm::buildEntity() finds existing conditions by plugin id so it can update them in place, then guards each save with a truthy check if ($bundle_condition_id). When the existing condition lives at the integer index 0, if (0) is falsy in PHP, so the form falls through to addSelectionCondition() and adds a duplicate under a new UUID instead of updating the existing one. Same problem for $language_condition_id.

This is a regression introduced by #2895873 — the "preserve UUID across saves" commit shipped in 8.x-1.15.

Where do non-UUID keys come from?

Investigated and found a known in-tree source: src/Plugin/migrate/source/PathautoPattern.php at line 132 wraps the single migrated condition in another array via

$row->setSourceProperty('selection_criteria', [$selection_criteria]);

which produces a list with one entry at index 0 on the destination side. Any pathauto pattern that came over via a D7 → D8/9/10/11 migration has that shape on disk. Before #2895873, every form save would silently regenerate the key to a fresh UUID (because the form removed and re-added each condition on every save), which masked the shape mismatch. 1.15's stable-key behaviour exposes it.

Other paths (manual YAML edits, install-profile config, custom code calling $config->set('selection_criteria', array_values(...))) can also produce non-UUID keys, so the fix is defensive about any non-UUID shape rather than targeting the migration source specifically.

Fix

Two parts:

1. Use !== NULL against the variables' initial NULL sentinel so the integer 0 is correctly recognised as "condition found at index 0" rather than "not found". This fixes the duplication.

-       if ($bundle_condition_id) {
+       if ($bundle_condition_id !== NULL) {
          $entity->getSelectionCondition($bundle_condition_id)->setConfiguration($bundle_config);
        }
...
-     elseif ($bundle_condition_id) {
+     elseif ($bundle_condition_id !== NULL) {
        $entity->removeSelectionCondition($bundle_condition_id);
      }

2. Defensively normalize non-UUID keys to UUIDs on first save. After the lookup loop, if the captured condition id is not a valid UUID, drop the existing entry so the addSelectionCondition() path below re-creates it under a fresh UUID. The entity-form API expects UUID-keyed instance ids and the rest of the codebase assumes that shape. Without this normalization, patterns whose keys come from the migration path (or any other non-UUID source) would keep their odd shape forever and remain a latent risk for similar bugs.

+ if ($bundle_condition_id !== NULL && !Uuid::isValid((string) $bundle_condition_id)) {
+   $entity->removeSelectionCondition($bundle_condition_id);
+   $bundle_condition_id = NULL;
+ }
+ if ($language_condition_id !== NULL && !Uuid::isValid((string) $language_condition_id)) {
+   $entity->removeSelectionCondition($language_condition_id);
+   $language_condition_id = NULL;
+ }

Test

testLegacyNumericKeysDoNotDuplicate() creates a pattern through the normal edit-form flow, rewrites selection_criteria with array_values() to mimic the on-disk shape produced by the migrate source plugin, re-saves the pattern through the edit form (no field changes), and asserts:

• still exactly one selection criterion (no duplication)
• still exactly one entity_bundle:node condition (no plugin-level duplication)
• the criterion's key is a valid UUID (rekey happened)

Verified locally:

• Without the fix: Test Failed: testLegacyNumericKeysDoNotDuplicate — Selection criteria should not duplicate after re-save. Failed asserting that actual size 2 matches expected size 1.
• With the fix: OK, 15 assertions, 0 failures.

Related follow-ups (separate scope)

1. Fix the migrate source: src/Plugin/migrate/source/PathautoPattern.php should produce UUID keys on the destination side rather than a numerically-indexed list. That eliminates the main known source of non-UUID keys for future migrations. Doesn't help existing post-migration sites — those still need either a form re-save (this MR) or a one-shot post-update hook to clean up.
2. Optional post-update hook that iterates all pathauto patterns and rekeys non-UUID entries to UUIDs at update.php time. Would normalize patterns the user never re-edits. Not bundled here because (a) this MR's reviewable scope is the user-visible regression, (b) post-update hooks are tracked by name so a separate hook + a separate review cycle is the safer way to ship a config-rewriting migration.