Problem/motivation

Due to the new Multilingual module group introduced in #1833184: Find a consistent naming scheme for translation-related modules, update module does not treat language, content translation and interface translation modules as part of core. This does not regress any functionality, since the modules are grouped to be under project "" (empty string), and ignored for update module. Since there are other core modules, core itself is checked for updates. The only visible resulting problem is these modules are not listed anymore as part of core on the available updates page:

LanguageModulesUpdate.png

Proposed solution

Do not use the 'Core' package as sole indication of a module being in core. The fix in #1871328: Modules in new Multilingual package are no longer tested by Module\EnableDisableTest for example used new conditions to check for the path. The problematic code is:

function update_get_project_name($file) {
  $project_name = '';
  if (isset($file->info['project'])) {
    $project_name = $file->info['project'];
  }
  elseif (isset($file->info['package']) && (strpos($file->info['package'], 'Core') === 0)) {
    $project_name = 'drupal';
  }
  return $project_name;
}

Also would need a small extension to tests to enable Language module and check it shows up on the available updates page (or if we want to test the pure functionality, to check if language module is part of the core project as decided by update_get_project_name().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Title: Language-related modules not listed among core modules anymore » Language-related modules not listed on available updates page

Fix title.

vijaycs85’s picture

Status: Active » Needs review
FileSize
814 bytes
20.08 KB

Patch for package to path change. update_language

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

RTBC in case it comes back green. (If not the bot will set it to needs work.)

Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs tests, -D8MI, -language-base

The last submitted patch, 1879732-language-available-update-page-1.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1879732-language-available-update-page-1.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +Needs tests, +D8MI, +language-base

The last submitted patch, 1879732-language-available-update-page-1.patch, failed testing.

vijaycs85’s picture

Every time getting failed in different test case and seems they are fine locally. Guessing it is testbot.

YesCT’s picture

Are they all near the end of the tests? This lasted 57 minutes. In think the time out is at 90 min or even higher now so that's probably not it.

Maybe http://drupal.org/node/1859714 or other issues in that queue might have hints.

After reading the "differences between local and test bot" (a link in that issue) I think it would be ok to open a testbot issue.

Gábor Hojtsy’s picture

+++ b/core/modules/update/update.compare.incundefined
@@ -254,7 +254,7 @@ function update_get_project_name($file) {
-  elseif (isset($file->info['package']) && (strpos($file->info['package'], 'Core') === 0)) {
+  elseif (isset($file->info['package']) && (strpos($file->filename, 'core/modules') === 0)) {

What is the role of the isset() anymore?!

Gábor Hojtsy’s picture

Status: Needs work » Needs review
vijaycs85’s picture

Updated patch with review comment in #11

Gábor Hojtsy’s picture

Looks good :) Can we expand on this in the tests to check for the Language module's appearance on this page for example, so we avoid this regression in the future :) Thanks!

vijaycs85’s picture

Updating test case...

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks superb! Thanks for the fix!

YesCT’s picture

Rock on @vijaycs85 ! :)

vijaycs85’s picture

Thanks @Gábor Hojtsy and @YesCT.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Great!

Committed and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Superb, thanks! :)

@vijaycs85: Thanks for your great work! If you are looking for something on a similar scale, #1853720: Hide language selection option is backwards and #1869328: Restore simplicity of language list are such issues. Eg. the later one needs some simple code updates to ensure weight setup is proper and tests.

vijaycs85’s picture

Thank you @Gábor Hojtsy. I will check them.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Add needs tests description.