feat(theme): enqueue AI translations from the meta box instead of translating inline (#156) - #157
Conversation
…nslating inline (#156) The "Translate All" / per-language buttons in the AI Translation meta box ran synchronously: one blocking OpenAI call per button, fired concurrently (Promise.all). On a long article five concurrent calls rate-limited/timed out, so most languages showed "failed". Make the meta box ENQUEUE, exactly like /cdcf/v1/translate, so the worker (cdcf_process_translation) does the OpenAI work async + sequentially — which is reliable at any size and already carries the footnote-anchor protection, auto-publish, ACF copy, and language-aware featured image. - Extract the create/resolve + link + enqueue core into the shared cdcf_enqueue_post_translation() (handlers/translate.php), used by both cdcf_rest_translate() and cdcf_ajax_ai_translate(). The linking now goes through the GET_LOCK helper (moved here as cdcf_translate_link_under_lock) with orphan-cleanup on link failure — so /cdcf/v1/translate gains the same race-safety the meta box had (#150/#152), and concurrent "Translate All" enqueues stay safe. - cdcf_ajax_ai_translate() becomes a thin auth + delegate + JSON wrapper; the ~200 lines of synchronous OpenAI/ACF/attachment/auto-publish code are gone (that behaviour lives in the worker, covered by ProcessTranslationTest). - Meta-box JS: buttons now report "Queued" (DOM-built Edit link, no innerHTML) and "Translate All" says translations will appear shortly. Tests: AjaxAiTranslateTest rewritten for the thin delegate; TranslateHandlerTest gains lock + link-failure-cleanup coverage. Theme suite 363 green; refactored files fully covered (ai-translate 15/15; translate.php's only gaps are the WP-Cron fallback, exercised by its separate-process test). Implements #156. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 39 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR refactors AI translation from synchronous in-request processing to asynchronous enqueueing. A new shared ChangesAsync AI Translation Enqueueing
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related Issues
Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | -49 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wordpress/themes/cdcf-headless/includes/handlers/translate.php (2)
82-149:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate caller-supplied
post_idbefore bypassing resolution.When
post_idis non-zero, this helper skips both source existence checks and any verification that the target post actually belongs to$source_id/$target_lang, then queues work immediately. A stale or arbitrary ID can send the worker at the wrong post. Reject mismatches up front or resolve the expected translation ID from Polylang before enqueueing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wordpress/themes/cdcf-headless/includes/handlers/translate.php` around lines 82 - 149, The function currently trusts a non-zero caller-supplied post_id and skips validating the source/translation relationship; before enqueueing (where cdcf_enqueue_translation/wp_schedule_single_event are called) fetch and validate the provided post_id: ensure get_post($post_id) exists and, if Polylang is available, verify pll_get_post($source_id, $target_lang) either matches $post_id or use the pll_get_post result as the canonical translation id; if there is no matching translation return a WP_Error('invalid_post', ...) (or override $post_id with the resolved ID) so we never queue work for a stale/arbitrary post.
141-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t report
wp-cronifwp_schedule_single_event()fails (wordpress/themes/cdcf-headless/includes/handlers/translate.php:141-147)
wp_schedule_single_event()return value is ignored, so this REST path can still respond202withqueue => wp-croneven when the cron event wasn’t actually scheduled. Treat scheduling failure as a server error before callingspawn_cron().🔧 Proposed fix
if (function_exists('cdcf_enqueue_translation')) { $queue = cdcf_enqueue_translation($post_id, $source_id, $target_lang); } else { - wp_schedule_single_event(time(), 'cdcf_async_translate', [$post_id, $source_id, $target_lang]); + $scheduled = wp_schedule_single_event(time(), 'cdcf_async_translate', [$post_id, $source_id, $target_lang], true); + if (is_wp_error($scheduled) || !$scheduled) { + return new WP_Error('enqueue_failed', 'Failed to schedule translation job.', ['status' => 500]); + } spawn_cron(); $queue = 'wp-cron'; }
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wordpress/themes/cdcf-headless/functions.php`:
- Around line 2147-2152: The current code calls Promise.all with
translateOne(btn).catch(()=>{}) which hides failures and always sets
allBtn.textContent to "All queued…"; update this to detect per-button failures
(use Promise.allSettled or have translateOne return success/failure) by mapping
buttons to promises that resolve with a status object or using
allSettled(buttonPromises), then inspect the results: if every result is
fulfilled/success set allBtn.textContent to the success message, otherwise set a
failure/partial message and re-enable allBtn as appropriate; reference the
translateOne function, the allBtn element and the '.cdcf-ai-translate-btn'
button list when locating the change.
In `@wordpress/themes/cdcf-headless/includes/handlers/translate.php`:
- Around line 157-160: The handler is re-sanitizing request values which
violates the cdcf/v1 REST contract; update the call in translate.php so
cdcf_enqueue_post_translation receives the raw, already-sanitized $request
values instead of wrapping them in intval/absint/sanitize_text_field (i.e. pass
$request['source_id'], $request['target_lang'], $request['post_id'] directly),
and ensure any sanitization remains declared in the register_rest_route() args
block for these fields.
---
Outside diff comments:
In `@wordpress/themes/cdcf-headless/includes/handlers/translate.php`:
- Around line 82-149: The function currently trusts a non-zero caller-supplied
post_id and skips validating the source/translation relationship; before
enqueueing (where cdcf_enqueue_translation/wp_schedule_single_event are called)
fetch and validate the provided post_id: ensure get_post($post_id) exists and,
if Polylang is available, verify pll_get_post($source_id, $target_lang) either
matches $post_id or use the pll_get_post result as the canonical translation id;
if there is no matching translation return a WP_Error('invalid_post', ...) (or
override $post_id with the resolved ID) so we never queue work for a
stale/arbitrary post.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec49b714-71b1-45da-a17a-e3149d6e69d3
📒 Files selected for processing (5)
wordpress/themes/cdcf-headless/functions.phpwordpress/themes/cdcf-headless/includes/admin/ai-translate.phpwordpress/themes/cdcf-headless/includes/handlers/translate.phpwordpress/themes/cdcf-headless/tests/AjaxAiTranslateTest.phpwordpress/themes/cdcf-headless/tests/TranslateHandlerTest.php
- functions.php "Translate All": use Promise.allSettled and report the real outcome — count rejected enqueues and show a partial-failure message (and re-enable the button) instead of swallowing failures into a blanket "all queued". - translate.php cdcf_rest_translate: stop re-sanitizing — the route's args-block already declares absint / sanitize_text_field, so pass the request values through per the cdcf/v1 contract (#111). The admin-ajax caller still sanitizes its raw $_POST on its own side. - translate.php cdcf_enqueue_post_translation: validate a caller-supplied post_id (must exist AND be the source's Polylang translation for the target language) before enqueuing, so the worker never writes a translation into a stale or arbitrary post. Returns invalid_post (404/400) otherwise. Tests: provided-post_id test now asserts validation; added missing-post and not-the-translation rejection tests. Theme suite 365 green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed the review findings (all three valid against current code):
Tests updated (provided-post_id now asserts validation; added missing-post and not-the-translation rejection tests). Theme suite 365 green. |
2 participants
Implements #156.
Problem
The Languages → AI Translation meta box ("Translate All" + per-language buttons) translated synchronously — one blocking OpenAI call per button, fired concurrently (
Promise.all). On a long article (e.g. post 1326, ~33 KB) five concurrent OpenAI calls rate-limit/time-out, so typically only one language succeeds and the rest show "failed".Change
Make the meta box enqueue, exactly like
/cdcf/v1/translate— the worker (cdcf_process_translation) does the OpenAI work asynchronously and sequentially, which is reliable at any size and already carries footnote-anchor protection (#152), auto-publish, ACF copy, and language-aware featured-image handling.cdcf_enqueue_post_translation()(handlers/translate.php), now used by bothcdcf_rest_translate()andcdcf_ajax_ai_translate(). Linking goes through theGET_LOCKhelper (moved here ascdcf_translate_link_under_lock) with orphan-cleanup on link failure — so/cdcf/v1/translategains the same race-safety the meta box had (fix(theme): serialize Polylang group linking in AI media translation #150/fix(cdcf-mcp): preserve colon footnote anchors through wp_kses_post #152), and concurrent "Translate All" enqueues stay safe.cdcf_ajax_ai_translate()is now auth + delegate + JSON. The ~200 lines of synchronous OpenAI / ACF / attachment / auto-publish code are removed — that behaviour lives in the worker (covered byProcessTranslationTest).innerHTML); "Translate All" → "All queued — translations will appear shortly."Reconciliation (per the issue's checklist)
Verified the worker (
cdcf_process_translation) is a superset of the old synchronous handler: title/content/excerpt (+ footnote protection), attachment alt-text + the "no translatable text" no-op, translatable + non-translatable ACF copy, language-aware featured image, and auto-publish-when-source-published. So enqueuing loses nothing.Tests
AjaxAiTranslateTestrewritten for the thin delegate (auth, param forwarding, queued payload, WP_Error mapping).TranslateHandlerTestgains lock (GET_LOCK/RELEASE_LOCKkeyed on source) and link-failure-cleanup coverage; its existing create/reuse/attachment/parent/enqueue tests still pass (behaviour preserved).ai-translate.php15/15;translate.php's only uncovered lines are the WP-Cron fallback, exercised by itsRunInSeparateProcesstest).Behaviour note
"Translate All" stays concurrent (fast enqueue), but each request now returns immediately and the worker drains the queue one at a time — so no more OpenAI pile-up, and the group can't corrupt (locked linking). This is the same path the manual sequential re-translations used successfully.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests