Skip to content

Agents Manager: load UI translations from widgets.wp.com#50069

Open
t-wright wants to merge 1 commit into
trunkfrom
add/agents-manager-translations
Open

Agents Manager: load UI translations from widgets.wp.com#50069
t-wright wants to merge 1 commit into
trunkfrom
add/agents-manager-translations

Conversation

@t-wright

@t-wright t-wright commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Relates to FORNO-373

Proposed changes

  • Enqueue Agents Manager translations

Related product discussion/links

  • FORNO-373

Does this pull request change what data or activity we track or use?

No

Testing instructions

@t-wright
Agents Manager: load UI translations from widgets.wp.com
513a90c 
The package enqueued its JS/CSS bundles but never the matching translation
files, so the UI always rendered in English. For connected variants with a
non-English locale, enqueue the locale-specific translation script from
widgets.wp.com/agents-manager/languages/{locale}-v1.js (with wp-i18n as a
dependency of the main bundle), mirroring Help Center and Image Studio.

Adds determine_iso_639_locale() to map the WordPress locale to the CDN file
naming, plus tests for the enqueue behaviour and locale normalisation.
@github-actions

github-actions Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the add/agents-manager-translations branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack add/agents-manager-translations

bin/jetpack-downloader test jetpack-mu-wpcom-plugin add/agents-manager-translations

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions

github-actions Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@github-actions github-actions Bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jun 30, 2026
@jp-launch-control

jp-launch-control Bot commented Jun 30, 2026

Copy link
Copy Markdown

Code Coverage Summary

Coverage changed in 1 file.

File Coverage Δ% Δ Uncovered
projects/packages/agents-manager/src/class-agents-manager.php 284/409 (69.44%) 1.49% 0 💚

Full summary · PHP report · JS report

@t-wright t-wright marked this pull request as ready for review June 30, 2026 10:42
@t-wright

t-wright commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

@claude please review

@claude

claude Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Claude finished @t-wright's task in 1m 20s —— View job

Code Review: Agents Manager translations

  • Read the changed files and surrounding context
  • Review enqueue_script() translation loading
  • Review determine_iso_639_locale() normalization
  • Review test coverage
  • Post review

Overall this is a clean, well-scoped change that mirrors the established Help Center / Image Studio pattern, and the test coverage is genuinely thorough (enqueue + dependency ordering, English skip, disconnected skip, and a data-provider for locale normalization). Nice work. A few observations, none blocking.

Correctness — looks good ✅

  • Dependency ordering is right. Adding agents-manager-translations to $script_dependencies (and making the translation script itself depend on wp-i18n) guarantees wp.i18n.setLocaleData() runs before the main bundle reads strings. The test at Agents_Manager_Test.php:2527 asserts this dependency, which is the important invariant.
  • No dangling enqueue. The translation script is enqueued just before the main script with no early-return between them, so there's no path where translations load without the app.
  • Disconnected variants correctly skipped via str_contains( $variant, 'disconnected' ), matching the AGENTS.md contract.

Minor notes

  1. Dead entries in the locale allowlist (class-agents-manager.php:579). get_user_locale() returns underscore-form WP locales (pt_BR, zh_TW), and strtolower() doesn't convert _→-, so the hyphenated entries 'pt-br', 'zh-tw', 'zh-cn' in the in_array() list can never match. They're harmless (defensive), but worth a one-line comment or trimming to avoid implying get_user_locale() might return hyphens.

  2. Silent 404 for missing translation files. If widgets.wp.com/.../languages/{locale}-v1.js doesn't exist for a given locale, the <script> 404s and the UI silently stays English. This is acceptable degradation (and matches Help Center), but it does mean a locale WP supports but the Calypso app hasn't shipped translations for will fail quietly. No action needed — just flagging the behavior.

  3. English variants get no translations. en_GB, en_AU, etc. all normalize to en and skip the translation file (class-agents-manager.php:582). That's intended here since strings ship in the bundle, just confirming it's deliberate.

  4. Test gap (very minor): no assertion that the translation script declares wp-i18n as a dependency. The current tests cover the main→translations edge; you could add one line asserting wp-i18n is in the translation script's deps to lock in that the locale data binds before it's used. Optional.

Security

URL construction uses $locale, which is derived from get_user_locale() and stripped to the segment before the first -/_, so there's no meaningful injection surface. 👍

Nothing here blocks merge from my perspective — the notes are nits and confirmations rather than required changes.
· add/agents-manager-translations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zaguiini zaguiini Awaiting requested review from zaguiini
@wellyshen wellyshen Awaiting requested review from wellyshen
@saroshaga saroshaga Awaiting requested review from saroshaga
@kat3samsin kat3samsin Awaiting requested review from kat3samsin

Assignees

No one assigned

Labels

Docs [Package] Agents Manager [Status] In Progress [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Tests] Includes Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@t-wright