Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rodrigoaguilera’s picture

Issue tags: +D8MI

I'm curious about this too

cilefen’s picture

Title: Without "Update Manager" module enable, "Available translation updates" not work » Without "Update Manager" module enabled, "Available translation updates" does not work
penyaskito’s picture

This is on purpose. The project data is based on the project list supplied by the Update module, so it degrades gracefully to not checking for translations. Maybe this should be more explicitly remarked on the page?
Pinging Gábor about it.

Gábor Hojtsy’s picture

Title: Without "Update Manager" module enabled, "Available translation updates" does not work » Without "Update Manager" module enabled, "Available translation updates" does not work, should be documented
Version: 8.1.x-dev » 8.0.x-dev
Category: Support request » Bug report
Issue tags: +language-ui, +sprint

Yes, this is by design. The UI should inform people about it. We need the version information and update status is the only module that can tell us about that.

Gábor Hojtsy’s picture

BTW the idea is update manager can update the project information that locale uses, but the project information is cached and available from the last time the update module was still enabled.

Gábor Hojtsy’s picture

Issue tags: -sprint

Apparently nobody is working on this one, so removing from sprint.

Gábor Hojtsy’s picture

Title: Without "Update Manager" module enabled, "Available translation updates" does not work, should be documented » Without "Update Manager" module enabled, "Available translation updates" does not get updated, should be documented

Fix incorrect title.

Jose Reyero’s picture

There's nothing here that degrades gracefully, unless you can call "degrade gracefully" to "not working".

I suggest:

1. Removing all UI mentions to translation updates, included the Reports/Translation updates pages completely when Update Manager module is not enabled. Really if it's not working I'd prefer not seeing it.

2. Adding to Update Manager module description: "Checks for available updates for ... and Translations"

This way, the functionality may look like coming from Update Manager module thought it is built into the Locale module. No one will wonder why they don't get translation updates if they don't have Translation (Locale) enabled.

This may look ugly but at least it won't confuse users that much.

bsztreha’s picture

I've created the patch. Is this the right way?

penyaskito’s picture

Status: Needs review » Needs work
Issue tags: +sprint

@bsztreha Thanks for working on this!

For the text is perfect I think, but for the translations form I think Jose is suggesting to remove the page. We could do that by using _module_dependencies in the routing YAML file providing that form (See docs at https://www.drupal.org/node/2092643).

Gábor Hojtsy’s picture

In that case, let's update the issue title and summary.

bsztreha’s picture

I've attached the new patch, that contains the new hook, because locale.links.menu.yml was break Drupal

bsztreha’s picture

Status: Needs work » Needs review
Gábor Hojtsy’s picture

How was this one not enough?

+    _module_dependencies: 'update'
Gábor Hojtsy’s picture

Status: Needs review » Needs work
+++ b/core/modules/locale/locale.module
@@ -1378,3 +1378,14 @@ function locale_translation_language_table($form_element) {
+
+function locale_menu_links_discovered_alter(&$links) {

Missing "Implements ..." docblock.

bsztreha’s picture

#14
Because of the _module_dependencies, if update module does not exists, locale.links.menu.yml want to create menu, but route does not exists, and you get an error page on clear cache

Gábor Hojtsy’s picture

Ha, good catch!

bsztreha’s picture

I hope i've repaired one of fail, and i marked the places where we use Url with this path.

Gábor Hojtsy’s picture

+++ b/core/modules/locale/locale.pages.inc
@@ -34,6 +34,7 @@ function locale_translation_manual_status() {
   if (batch_get()) {
     return batch_process('admin/reports/translations');
   }
+  // @TODO: check module exists
   return new RedirectResponse(\Drupal::url('locale.translate_status', array(), array('absolute' => TRUE)));
 }

Where is this function even invoked?

drupal gabor$ git grep locale_translation_manual_status
core/modules/locale/locale.pages.inc:function locale_translation_manual_status() {
bsztreha’s picture

Honestly i dont know, i think its not in use.
In my opinion this way is not good, because is anybody create a link like:

\Drupal::url('locale.translate_status')

then Drupal page will be failed.

I dont how to solve this kind of url call, in this case the module exists check is overhead.

return '<p>' . t('Interface translations are automatically imported when a language is added, or when new modules or themes are enabled. The report <a href="!update">Available translation updates</a> shows the status. Interface text can be customized in the <a href="!translate">user interface translation</a> page.', array('!update' => \Drupal::url('locale.translate_status'), '!translate' => \Drupal::url('locale.translate_page'))) . '</p>';
Gábor Hojtsy’s picture

I don't think that is an overhead. The same happens for routes of whatever module that may or may not be enabled.

bsztreha’s picture

I have to separated the hook_help description, where is linked the locale.translate_status Url.
Is this what you thinking about?

Sutharsan’s picture

  1. +++ b/core/modules/locale/locale.routing.yml
    @@ -44,3 +44,4 @@ locale.translate_status:
         _title: 'Available translation updates'
       requirements:
         _permission: 'translate interface'
    +    _module_dependencies: 'update'
    

    We should not fix it here. We should fix locale_translation_build_projects(). The fallback solution there is quick and dirty. A better solution is already available in the l10n_update module.

  2. +++ b/core/modules/update/update.info.yml
    @@ -1,6 +1,6 @@
    -description: 'Checks for available updates, and can securely install or update modules and themes via a web interface.'
    +description: 'Checks for available updates, available translation updates and can securely install or update modules and themes via a web interface.'
    

    It is not the update module that checks for translation updates. It is (currently) a requirement.

Sutharsan’s picture

@bsztreha Please add an interdiff if you make a new patch.

Sutharsan’s picture

I've created #2550137: Improve interface translation fallback if Update module is disabled to fix the problem at the right end. We may still need some additional user notification, but only to tell the admin that better results may be achieved in when dev module translations are missing or not being updated.

Gábor Hojtsy’s picture

Well, its not just dev modules but falling back on older releases too, no?

Sutharsan’s picture

Update module is only used to determine a fallback in case of for dev releases. Since #2550137 only catches dev released after their first release, the content of the status page may be off. We still need some explanation there, especially in this stage of development with a lot of dev releases. I suggest to put a warning on the page like "The translation status of this page may be incomplete. Enable Update module to get accurate information."

bsztreha’s picture

I mentioned that at #9, see patch

Gábor Hojtsy’s picture

Note that after #2113957: Build server side version fallback system for translations, we could even explore not depend on update status at all even for the fallback. We can either repurpose this issue or open a new one if people think this is a good idea :) Although the client side has more info than the server redirects using update status' data, it is only slightly different and may work just as well with the server side info and be simpler.

Jose Reyero’s picture

+1 to get rid of that dependency on update status.

The issue is already there, I think this is the one #2113957: Build server side version fallback system for translations

So I'd ask for the patch here to be put 'on hold' for a few days and I'm giving it a try, will follow up on the other issue. I think we all want to get rid of update manager on our production servers while still being able to use localization update.

That server side fallback is great news, btw.

Gábor Hojtsy’s picture

I don't necessarily agree people will want to have localization update on production sites, it depends on the stability of the strings I guess coming from your translation team. If the team like changing translations of important concepts overnight (eg. user, node, theme, etc), that may be a problem :D Localization update can in fact do more harm if you are concerned about the stability of your strings than update status, given that update status merely informs you update things and does not change anything.

That said, I fully supporting making the two independent if we can get reasonably the same feature set, so people have the option to configure their site however they want.

Gábor Hojtsy’s picture

Status: Needs review » Closed (duplicate)
Issue tags: -sprint

#1832946: Runtime translation download fallback works different from installer translation download fallback landing obsoletes this issue. Sorry @bsztreha, hope this does not discourage you from contributing. It happens that we have different approaches and some win over others.

bsztreha’s picture

It's no problem :)
I hope i will help in another issue soon