Repository navigation
Upgrade/Install: Show and select translation updates on the Updates screen - #11610
s-a-s-k-i-a wants to merge 7 commits into
Conversation
|
Hi @s-a-s-k-i-a! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
Playground test passed. Manually tested and reviewed. Works. |
a43e354 to
7955a63
Compare
c8dd62d to
d9cbf3e
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
d9cbf3e to
c507717
Compare
tfrommen
left a comment
There was a problem hiding this comment.
👋 Hey there!
Since wp_is_translation_update_deferred is performing one or more DB queries under the hood, you might want to call it only if needed. If $update is already false, there's no point in possibly setting it to the same value again. 😉
c507717 to
460f52f
Compare
|
Quick note on the remaining failing PHPUnit jobs in the current run:
These both appear to be CI/infrastructure issues rather than failures caused by the translation update changeset. The other workflows for the current head are passing, and the targeted syntax, PHPCS, smoke, and E2E validation for this patch remain green. Happy to take another look if either failure reproduces consistently on a rerun. |
460f52f to
8ce1f44
Compare
8ce1f44 to
528b252
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the WordPress Updates screen translation section to mirror the plugin/theme update UX: it lists individual pending translation updates in a checkbox table, allows deferring specific translation updates, and ensures deferred translation updates are skipped by background/async translation updates until re-selected.
Changes:
- Added stable identifiers and site-option persistence helpers to defer/restore translation updates across requests.
- Replaced the translations section on
update-core.phpwith a checkbox table and updated the translation upgrade handler to install only selected translation updates while persisting unchecked ones as deferred. - Updated background translation update logic to skip deferred translation updates, and added PHPUnit coverage for the new helpers and display data.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phpunit/tests/admin/includesUpdate.php | Adds PHPUnit tests for translation update display data, stable IDs, and deferred-state behavior. |
| src/wp-includes/update.php | Introduces translation update stable IDs and deferred update get/set helpers stored in a site option. |
| src/wp-admin/update-core.php | Renders translation updates in a checkbox table and processes selected vs deferred updates during translation upgrades; adds an admin notice for deferred selections. |
| src/wp-admin/includes/update.php | Adds display helpers to resolve translation update names/languages and build display-ready translation update rows. |
| src/wp-admin/includes/class-wp-automatic-updater.php | Skips deferred translation updates when deciding whether to auto-update translations. |
| src/wp-admin/includes/class-language-pack-upgrader.php | Skips deferred translation updates during async translation upgrades. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@tfrommen The branch is rebased to current Current-head validation is complete: Coding Standards, PHP Compatibility, PHPStan, build, both E2E variants, performance, and upgrade tests pass. In the PHPUnit matrix, 176 jobs pass and 3 are skipped by matrix conditions. One PHP 7.4/MySQL 8.0 multisite job reaches the workflow's 20-minute limit after PHPUnit, AJAX, If you have bandwidth for another look, I would appreciate it. Thank you! |
ac83634 to
c4019f6
Compare
c4019f6 to
67819ce
Compare
tfrommen
left a comment
There was a problem hiding this comment.
As mentioned on Trac, I gave this another review and left some inline feedback. Overall, I still think this is good.
The @since annotations need a refresh.
Some code could do with (more) tests, which also requires some minor refactoring.
| /** | ||
| * Gets plugin names for translation updates. | ||
| * | ||
| * @since 7.1.0 |
There was a problem hiding this comment.
issue:
All new functionality is currently marked with 7.1.0. This needs to be refreshed.
There was a problem hiding this comment.
Updated the new @since annotations to 7.2.0 in d9930c0. That's our target release, not a confirmed inclusion. The final version will need to match the release this lands in.
|
|
||
| check_admin_referer( 'upgrade-translations' ); | ||
|
|
||
| if ( empty( $_POST['translations'] ) ) { |
There was a problem hiding this comment.
note:
There is a behavior change in handling requests. Previously, any POST request upgraded all translations. Now, plugins, scripts, or cached forms that post without translations[] do nothing, with no notice. I believe this is the intended behavior, but I wanted to point out the change nevertheless.
There was a problem hiding this comment.
Yes, this is intentional: a request without the form's update IDs installs nothing and leaves saved preferences unchanged. Unchecking everything in the current form is different: those updates are saved for later, and a confirmation is shown. Tests cover both cases and outdated forms.
| $selected_translation_updates = array(); | ||
|
|
||
| if ( ! empty( $_POST['checked'] ) ) { | ||
| $selected_translation_update_ids = array_unique( | ||
| array_map( | ||
| 'sanitize_text_field', | ||
| wp_unslash( (array) $_POST['checked'] ) | ||
| ) | ||
| ); | ||
|
|
||
| $selected_translation_updates = array_intersect_key( $translation_updates, array_flip( $selected_translation_update_ids ) ); | ||
| } | ||
|
|
||
| $deferred_translation_updates = array_intersect_key( | ||
| $current_translation_updates, | ||
| wp_get_deferred_translation_updates( $current_translation_updates ) | ||
| ); | ||
| $deferred_translation_updates = array_diff_key( $deferred_translation_updates, $translation_updates ); | ||
| $deferred_translation_updates += array_diff_key( $translation_updates, $selected_translation_updates ); |
There was a problem hiding this comment.
suggestion:
Since this core logic lives directly in the file scope, it cannot be (and thus is not) unit-tested. I'd recommend extracting it into a function (e.g., in wp-admin/includes/update.php) and add unit tests for stale entries, items that are not in the form, and all-unchecked.
There was a problem hiding this comment.
Moved the selection logic into wp_process_translation_update_selection() in d9930c0. Permission and nonce checks stay in the controller. Tests cover outdated entries, updates outside the form and unchecking everything, and verify which updates are selected or saved for later.
| /** | ||
| * @ticket 42281 | ||
| */ | ||
| public function test_wp_get_translation_update_data_loads_plugin_names_once() { |
There was a problem hiding this comment.
suggestion:
This test is brittle. It checks hard-coded 2 transient reads. That number breaks when any code adds a read. The property that matters is that reads do not grow with the number of plugin rows.
A better test could look like this (I did not actually run this):
public function test_wp_get_translation_update_data_reads_plugin_updates_independent_of_row_count() {
$count_reads = function ( $row_count ) {
$translations = array();
for ( $i = 1; $i <= $row_count; $i++ ) {
$translations[] = array(
'type' => 'plugin',
'slug' => "plugin-$i",
'language' => 'de_DE',
'version' => '1.0.0',
);
}
set_site_transient( 'update_plugins', (object) array( 'translations' => $translations ) );
$reads = 0;
$filter = static function ( $value ) use ( &$reads ) {
++$reads;
return $value;
};
add_filter( 'pre_site_transient_update_plugins', $filter );
wp_get_translation_update_data();
remove_filter( 'pre_site_transient_update_plugins', $filter );
return $reads;
};
$this->assertSame(
$count_reads( 1 ),
$count_reads( 10 ),
'Plugin update reads must not grow with the number of translation rows.'
);
}You may also want to add a test with zero plugin updates to confirm that the plugin-name lookup does not run at all.
There was a problem hiding this comment.
The test now compares one plugin translation row with ten instead of requiring exactly two reads. Separate tests check that no plugin-name lookup is added when there are no updates or only Core/theme updates. All these tests pass in d9930c0.
Extract and test translation selection processing, avoid redundant update-map work, and refresh the intended release annotations. Document explicit-update scope and clear matching deferrals after successful language-pack installation. See https://core.trac.wordpress.org/ticket/42281.
|
I've pushed d9930c0 and replied to each of your comments in its own thread. Explicit updates, including WP-CLI, can still install translations saved for later. The shared upgrader now clears the saved deferral only after a successful installation; failed updates remain deferred. The tests passed for manual selection, background updates and WP-CLI, including real WordPress.org downloads. All 10 GitHub workflows completed successfully: 121 checks passed and 20 skipped. |
Trac ticket: https://core.trac.wordpress.org/ticket/42281
Summary
list_translation_updates()currently renders only a generic translations notice and anUpdate Translationsbutton. Unlike the WordPress core, plugin, and theme update sections, it does not resolve or display the specific translation updates that are pending, and it does not allow individual translation updates to be deferred for later installation.This PR brings the translations section closer to the existing plugin and theme update UX by:
Changes
wp-admin/includes/update.phpwp-includes/update.phpupdate-core.phpwith a checkbox table styled like the existing plugin/theme update tablesLanguage_Pack_Upgrader::bulk_upgrade()WP_Automatic_Updater::should_update()andLanguage_Pack_Upgrader::async_upgrade()Test plan
php -l src/wp-admin/includes/update.phpphp -l src/wp-admin/update-core.phpphp -l src/wp-includes/update.phpphp -l src/wp-admin/includes/class-language-pack-upgrader.phpphp -l src/wp-admin/includes/class-wp-automatic-updater.phpphp -l tests/phpunit/tests/admin/includesUpdate.phpphp -l tests/phpunit/tests/admin/languagePackUpgrader.phpphp -l tests/phpunit/tests/admin/wpAutomaticUpdater.phpvendor/bin/phpcs --standard=phpcs.xml.dist src/wp-admin/includes/update.php src/wp-admin/update-core.php src/wp-includes/update.php src/wp-admin/includes/class-language-pack-upgrader.php src/wp-admin/includes/class-wp-automatic-updater.php tests/phpunit/tests/admin/includesUpdate.php tests/phpunit/tests/admin/languagePackUpgrader.php tests/phpunit/tests/admin/wpAutomaticUpdater.phpgit diff --checknpm run build:devhttps://wpcore.wp-studio.dev/wp-admin/update-core.phpusing a temporary MU-plugin fixture and local language-pack ZIP fixtures:3rows forWordPress,Hello Dolly, andTwenty Twenty-Four3translation rows were selected by default2successful translation updatesdeferred_translation_updatespersisted only the unchecked plugin translation updateWP_Automatic_Updater::should_update( 'translation', ... )returnedfalsefor the deferred plugin translation update andtruefor a non-deferred translation updateLanguage_Pack_Upgrader::async_upgrade()left the deferred translation update pendingms-files, and Xdebug test commands had all passed.Local PHPUnit execution was unavailable because this environment does not have the required WordPress test database/Docker runtime; the official GitHub Actions matrix provides the PHPUnit validation above.
Use of AI Tools
AI assistance: Yes
Tool(s): Codex
Model(s): GPT-5-based Codex
Used for: implementation, test scaffolding, server-side E2E validation orchestration, and PR drafting; outputs were reviewed and validated with syntax checks, PHPCS,
git diff --check, and authenticated admin-page testing.This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.