From 388526e27fa4914a727e9c9777f73f6f65ca01f3 Mon Sep 17 00:00:00 2001 From: "John R. D'Orazio" Date: Mon, 8 Jun 2026 07:02:28 -0400 Subject: [PATCH] fix(submissions): atomic Polylang group save to eliminate lost-update race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Observed on production 2026-06-08 when publishing the "Interior Castle App" community_project referral (post 1502): all 5 sibling drafts (1503-1507) were correctly created and individually language-assigned via pll_set_post_language, but the Polylang post translation group came out as {en: 1502} only — orphaning IT/ES/FR/PT/DE. Cascade: PR #200's auto- link-on-publish hook bailed on the orphans (because cdcf_get_source_post_id couldn't walk back to EN), PR #201's tag-propagation hook bailed for the same reason, and the non-EN Projects pages ended up missing the new siblings. Root cause: cdcf_enqueue_translations_for_submission called pll_save_post_translations 5 times — once per iteration, with a progressively-larger map. Polylang stores translation groups as a single term in a custom taxonomy, edited in-place. While the loop was still running, the Redis worker auto-published the first sibling, which fired Polylang's own post-update plumbing that re-evaluated the group at a point when only a partial map was committed — and the in-flight loop's subsequent saves either lost-updated or were lost-updated against that. Same shape as the lost-update race that PR #156 (issue #155) introduced the /translate-all endpoint to fix for the meta-box Translate-All fan-out. The fix mirrors /translate-all's atomic shape: Phase 1: create all sibling drafts + pll_set_post_language each (per-post language is independent of group state) Phase 2: ONE atomic pll_save_post_translations() with the full map Phase 3: enqueue translation jobs for the newly-created siblings only If the atomic save returns false, every just-created draft is force-deleted so a failed call leaves no orphan rows. 3 new tests guard the new shape: - pll_save_post_translations called exactly once with the full 6-language map (regression guard against re-introducing per-iter saves) - No save and no enqueue when all langs are already linked from a prior partial run (idempotency) - Rollback: drafts force-deleted and no enqueues fire when the atomic save returns false All 6 existing tests for this function continue to pass unchanged. 547/547 theme suite green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../includes/admin/submission-lifecycle.php | 60 ++++++++--- .../tests/SubmissionLifecycleTest.php | 102 ++++++++++++++++++ 2 files changed, 148 insertions(+), 14 deletions(-) diff --git a/wordpress/themes/cdcf-headless/includes/admin/submission-lifecycle.php b/wordpress/themes/cdcf-headless/includes/admin/submission-lifecycle.php index ba4f676..f30fa90 100644 --- a/wordpress/themes/cdcf-headless/includes/admin/submission-lifecycle.php +++ b/wordpress/themes/cdcf-headless/includes/admin/submission-lifecycle.php @@ -49,13 +49,23 @@ function cdcf_is_public_submission(int $post_id): bool { } /** - * For each target language (it/es/fr/pt/de): - * - Skip if a Polylang translation already exists. - * - Otherwise create a draft sibling post, link it via Polylang, - * and enqueue a background AI translation job. + * Create + Polylang-link the missing it/es/fr/pt/de sibling drafts of an + * EN source post, then enqueue an AI translation job per newly-created + * sibling. The existing worker (cdcf_process_translation) will auto- + * publish each translation once its source post is `publish`. * - * The existing worker (cdcf_process_translation) will auto-publish - * each translation once its source post is `publish`. + * Group save is **atomic** — exactly one pll_save_post_translations() + * call after every sibling has been created. The earlier shape (one + * save per iteration, accumulating the map) lost-updated the Polylang + * translation group on production: Interior Castle App publish on + * 2026-06-08 created all 6 siblings with correct per-post languages but + * the group came out as {en} only, orphaning IT/ES/FR/PT/DE — the same + * lost-update race the /translate-all endpoint was created to fix for + * the meta-box Translate-All fan-out. Mirroring its atomic shape here + * makes the publish-flow race-resistant by construction. + * + * If the atomic save fails, every just-created draft is force-deleted + * so a failed call leaves no orphans behind (same shape as /translate-all). * * @param int $en_post_id The English (source) post ID. MUST be the source, * not a translation — caller should resolve via @@ -80,19 +90,20 @@ function cdcf_enqueue_translations_for_submission(int $en_post_id, string $post_ $target_langs = ['it', 'es', 'fr', 'pt', 'de']; - // Build the Polylang translation map once; accumulate as we create new siblings. - // Pre-seeding from the existing map handles partial re-runs where some langs - // are already linked. + // Pre-seed the map from any existing group (handles partial re-runs + // where some langs are already linked from a previous attempt). $translations = pll_get_post_translations($en_post_id); $translations['en'] = $en_post_id; + // Phase 1: create draft siblings + per-post language assignment. + // Group save is intentionally NOT called here — see file-level + // docblock for the lost-update race rationale. + $newly_created = []; foreach ($target_langs as $lang) { - // Skip if a translation is already linked for this language. if (!empty($translations[$lang])) { continue; } - // Create a draft sibling post; the worker will fill content and auto-publish. $trans_id = wp_insert_post([ 'post_type' => $post_type, 'post_status' => 'draft', @@ -105,11 +116,32 @@ function cdcf_enqueue_translations_for_submission(int $en_post_id, string $post_ } pll_set_post_language($trans_id, $lang); - $translations[$lang] = $trans_id; - pll_save_post_translations($translations); + $newly_created[$lang] = $trans_id; + } + + if (empty($newly_created)) { + // Nothing new to link or enqueue. + return; + } + + // Phase 2: one atomic group save with the full {lang => post_id} map. + $save_result = pll_save_post_translations($translations); + if ($save_result === false) { + error_log(sprintf( + 'cdcf_enqueue_translations_for_submission: pll_save_post_translations returned false for post %d; rolling back %d draft(s).', + $en_post_id, + count($newly_created) + )); + foreach ($newly_created as $trans_id) { + wp_delete_post($trans_id, true); + } + return; + } - // Enqueue background translation: Redis Queue if available, WP-Cron fallback. + // Phase 3: enqueue translation jobs for the newly-created siblings only. + // (Existing siblings from the pre-seed already had their content done.) + foreach ($newly_created as $lang => $trans_id) { if (function_exists('cdcf_enqueue_translation')) { cdcf_enqueue_translation($trans_id, $en_post_id, $lang); } else { diff --git a/wordpress/themes/cdcf-headless/tests/SubmissionLifecycleTest.php b/wordpress/themes/cdcf-headless/tests/SubmissionLifecycleTest.php index a0b6f30..6fb6554 100644 --- a/wordpress/themes/cdcf-headless/tests/SubmissionLifecycleTest.php +++ b/wordpress/themes/cdcf-headless/tests/SubmissionLifecycleTest.php @@ -321,6 +321,108 @@ function (int $trans_id, int $en_id, string $lang) use (&$enqueued): string { $this->assertSame(['it', 'es', 'pt', 'de'], $enqueued); } + public function test_enqueue_translations_calls_pll_save_exactly_once_with_full_map(): void + { + // Regression guard for the lost-update race observed on + // production 2026-06-08 (Interior Castle App publish): the old + // shape called pll_save_post_translations 5x with progressively + // larger maps, racing against Polylang's post-update hooks that + // fire when the worker auto-publishes a sibling mid-loop. The + // atomic shape saves exactly once with the complete map. + $this->stubCommonFunctions(); + Functions\when('get_post')->justReturn($this->fakePost([ + 'ID' => 42, + 'post_title' => 'New Project', + ])); + + $counter = 199; + Functions\when('wp_insert_post')->alias( + function () use (&$counter): int { + $counter++; + return $counter; + } + ); + + $saves = []; + Functions\when('pll_save_post_translations')->alias( + function (array $map) use (&$saves): bool { + $saves[] = $map; + return true; + } + ); + $this->allowAllFunctionsToExist(); + + cdcf_enqueue_translations_for_submission(42, 'project'); + + $this->assertCount(1, $saves, 'pll_save_post_translations must be called exactly once'); + $this->assertSame( + ['en' => 42, 'it' => 200, 'es' => 201, 'fr' => 202, 'pt' => 203, 'de' => 204], + $saves[0], + 'the single save must carry the complete 6-language map' + ); + } + + public function test_enqueue_translations_skips_save_and_enqueue_when_all_langs_already_linked(): void + { + // Pre-seed already covers every target lang — no new drafts to + // create, so nothing new to atomically save and nothing to enqueue. + $this->stubCommonFunctions(); + Functions\when('get_post')->justReturn($this->fakePost(['ID' => 42])); + Functions\when('pll_get_post_translations')->justReturn([ + 'it' => 50, 'es' => 51, 'fr' => 52, 'pt' => 53, 'de' => 54, + ]); + Functions\expect('wp_insert_post')->never(); + Functions\expect('pll_save_post_translations')->never(); + Functions\expect('cdcf_enqueue_translation')->never(); + $this->allowAllFunctionsToExist(); + + cdcf_enqueue_translations_for_submission(42, 'project'); + + $this->assertTrue(true); + } + + public function test_enqueue_translations_rolls_back_drafts_when_atomic_save_fails(): void + { + // pll_save_post_translations can return false (Polylang inactive + // post-creation, term-save error, etc.) — drafts that were just + // created must be force-deleted to avoid orphan rows in the DB, + // and no translation jobs should be enqueued for posts that no + // longer have a Polylang group. + $this->stubCommonFunctions(); + Functions\when('get_post')->justReturn($this->fakePost([ + 'ID' => 42, + 'post_title' => 'New Project', + ])); + + $counter = 199; + Functions\when('wp_insert_post')->alias( + function () use (&$counter): int { + $counter++; + return $counter; + } + ); + Functions\when('pll_save_post_translations')->justReturn(false); + + $deleted = []; + Functions\when('wp_delete_post')->alias( + function (int $id, bool $force) use (&$deleted): array { + $deleted[] = [$id, $force]; + return []; + } + ); + Functions\expect('cdcf_enqueue_translation')->never(); + $this->allowAllFunctionsToExist(); + + cdcf_enqueue_translations_for_submission(42, 'project'); + + // All 5 just-created drafts (it, es, fr, pt, de = ids 200-204) + // force-deleted (second arg = true). + $this->assertSame( + [[200, true], [201, true], [202, true], [203, true], [204, true]], + $deleted + ); + } + // ─── cdcf_repend_submission_on_untrash ──────────────────────────── public function test_repend_ignores_status_transitions_that_arent_trash_to_draft(): void