A new module version is not recognized by interface translation update.

1. +++ b/core/modules/locale/locale.batch.inc
   @@ -20,6 +20,43 @@
   +function locale_translation_batch_version_check($project, $langcode, &$context) {
   
   +++ b/core/modules/locale/locale.compare.inc
   @@ -268,7 +268,8 @@ function _locale_translation_batch_status_operations($projects, $langcodes, $opt
   +      $operations[] = ['locale_translation_batch_version_check', [$project, $langcode]];
   

   I bet it needs type-hints for the function arguments

   Moreover I'd better created separate class for new function in context on "Kill includes" tag but I sure there's follow-up already for that

2. +++ b/core/modules/locale/tests/src/Functional/LocaleTranslationChangeProjectVersionTest.php
   @@ -0,0 +1,87 @@
   +    $moduleHandler = $this->container->get('module_handler');
   +    $moduleHandler->loadInclude('locale', 'inc', 'locale.batch');
   ...
   +    \Drupal::state()->set('locale.test_projects_alter', TRUE);
   +    \Drupal::state()->set('locale.remove_core_project', TRUE);
   

   I'm sure $this->container should be replaced with \Drupal::moduleHandler()

Thank you a lot for #27!

The last $project, $langcodeboth needs string type-hint as they available since PHP 7.0

Thanks, looks ready

Removed typehint of $context in locale_translation_batch_version_check.

 diff --git a/core/modules/locale/locale.batch.inc b/core/modules/locale/locale.batch.inc
-index fcc284618f..43b3476f67 100644
+index fcc284618f..94ace67938 100644
 --- a/core/modules/locale/locale.batch.inc
 +++ b/core/modules/locale/locale.batch.inc
 @@ -20,6 +20,43 @@
@@ -20,7 +20,7 @@
 + * @param array|\ArrayAccess $context
 + *   The batch context.
 + */
-+function locale_translation_batch_version_check(string $project, string $langcode, array &$context) {
++function locale_translation_batch_version_check(string $project, string $langcode, &$context) {
 +  $locale_project = \Drupal::service('locale.project')->get($project);
 +
 +  if (empty($locale_project)) {

phpDoc allows \ArrayAccess and drush use this. ;-)

Fixed deprecation.

assertFileNotExists -> assertFileDoesNotExist.

Patch is working fine and applied cleanly ....

Thanks

Rerolled #34 for 9.5

Hi i have the same issue with drush locale:update that doesn't replace contrib module .po file by new version when module is updated even after a locale:update locale:check.

Experiencing it on drupal 9.4.8

Hi patch #38 works against 9.5.2

tested with both config remote_and_local and Local
it works nicely, the only strange thing is that the conts of updated strings slihtly differs on each test i did event while using the same database as start point.

When rexproting the sites po for comparison, everything looks fine, i only have 2 extra string when i'm in remote_and_local mode as those strings are displayed during remote locale:check.

with same database :
remote+local
> [notice] Translations imported: 847 added, 13712 updated, 0 removed.
local (with po commited from first test)
> [notice] Translations imported: 848 added, 13468 updated, 0 removed.

Came here from a related issue in Bug Smash. Have updated the patch in #38 to fix the test fail / deprecation. Also since we can have union types now I've reverted #31 and added a union type.

LocaleTranslationChangeProjectVersionTest - Error : Call to undefined function Drupal\Tests\locale\Functional\locale_translation_batch_version_check()

testUpdateCron -
Queue holds tasks for one project.
Failed asserting that 2 matches expected 3.
Expected :3
Actual :2

Made sure the tests fail without the fix and they do.
Reviewing the code changes look good.

Question for committer if locale_translation_batch_version_check needs a change record.

I would be interested in getting this finished and merged. This might not be the worst bug that Drupal has to offer, but it still breaks a pretty basic core functionality. Right now, people are upgrading to Drupal 10.1 and will be wondering why all kinds of simple and visible Strings are not translated.

I just added the patch #43 to our Drupal 10.1.6 installation. It applied without a problem and cured the problem with the missing translations of newer Strings. The new test LocaleTranslationChangeProjectVersionTest.php runs successfully, as does the changed test LocaleUpdateCronTest.

I'm not sure about Drupal\Tests\ckeditor5\FunctionalJavascript\MediaTest that apparently failed back in May, see #45. Our project still uses CKEditor 4, but I tried it on an unmodified Drupal 10.1 installation. Everything worked there, but all test methods in the MediaTest were skipped.

So what is actually mssing here? I tried to trigger a restest of patch #43. When this hopefully succeeds, can we simply set this back RTBC?

In the meantime, I had a second look and understood that I need to install a Chromium driver so that the JavaScript tests are not simply skipped. With ddev-selenium-standalone-chrome installed, they are now executed. In my local test setup of a fresh Drupal 10.2-dev installation, JSWebAssertTest works with the patch installed as does MediaTest.

Can somebody with a better understanding of how Drupal's test setup works please have a look at what might be missing here or if we can set this back to RTBC? I'm setting this back to "Needs review" for now.

Also, comment #44 raised the question "if locale_translation_batch_version_check needs a change record". I'm not sure about that -- do we need a change record just because there is a new function? But I think fixing the bug might warrant a change record, as people might rely on the old translations or have lots of missing translations added themselves.

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

JavaScript tests tend to have random fails. I've converted to a merge request rather than rerunning the failing job since it's a lot quicker. Let's see if it comes back green.

MR is showing green, believe #43 was a random failure.

Thanks @mstrelan for hiding all patches.

Committed to 11.x

Backported to 10.2.x

Nice work folks, great to get this long-standing issue solved 🐛🔨