Adding/updating interface translations should invalidate page & render caches

I believe this is now blocked on #2428703: Add a 'user.permissions' cache context (was: "Should cache contexts be able to associate a cache tag?").

#2432837: Make cache contexts hierarchical (e.g. 'user' is more specific than 'user.roles') added the languages:<type> cache contexts. #2453059: Set default render cache contexts: 'theme' + 'languages:' . LanguageInterface::TYPE_INTERFACE is applying it everywhere. That means that we now know which render arrays should be invalidated by a per-language interface translation cache tag.

Once #2428703: Add a 'user.permissions' cache context (was: "Should cache contexts be able to associate a cache tag?") lands, we'll be able to associate such a cache tag with 'languages:' . LanguageInterface::TYPE_INTERFACE, which would (finally) allow us to solve this issue.

#2428703: Add a 'user.permissions' cache context (was: "Should cache contexts be able to associate a cache tag?") landed and settled on a different direction. Therefore this issue needs a slightly new direction too: instead of associating a cache tag with the cache context, we'll need to something analogous to #2453059: Set default render cache contexts: 'theme' + 'languages:' . LanguageInterface::TYPE_INTERFACE: we'll need to set default render cache tags.

1. +++ b/core/modules/locale/src/EventSubscriber/LocaleTranslationCacheTag.php
   @@ -0,0 +1,51 @@
   +class LocaleTranslationCacheTag implements EventSubscriberInterface {
   ...
   +   * Constructs a ThemeSettingsCacheTag object.
   

   Mismatch :) C/p remnant.

2. +++ b/core/modules/locale/src/Form/TranslateEditForm.php
   @@ -233,6 +234,9 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +      $this->eventDispatcher->dispatch(LocaleEvents::SAVE_TRANSLATION);
   

   Shouldn't we also dispatch this event for the "import translation" UI?

3. +++ b/core/modules/locale/src/Form/TranslateEditForm.php
   @@ -233,6 +234,9 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +
   

   Extraneous newline.

4. +++ b/core/modules/locale/src/LocaleEvents.php
   @@ -0,0 +1,32 @@
   + * Contains \Drupal\language\Config\LanguageConfigOverrideEvents.
   ...
   +final class LocaleEvents {
   

   Another remnant.

Addressed issues, now also has a test - it's a bit longer then removing the 3 lines that we saw :)

Regarding 3: yes, moved that to _locale_refresh_translations().

1. +++ b/core/modules/locale/locale.module
   @@ -1037,6 +1038,7 @@ function locale_string_is_safe($string) {
   + * - Rendered cache.
   

   Nit: we always say "render cache", not "rendered cache" :)

2. +++ b/core/modules/locale/src/LocaleEvents.php
   @@ -0,0 +1,32 @@
   +   * The name of the event fired when saving the configuration override.
   +   *
   +   * This event allows you to perform custom actions whenever a language config
   +   * override is saved. The event listener method receives a
   +   * \Drupal\language\Config\LanguageConfigOverrideCrudEvent instance.
   +   *
   +   * @Event
   +   *
   +   * @see \Drupal\language\Config\LanguageConfigOverrideCrudEvent
   +   * @see \Drupal\language\Config\LanguageConfigOverride::save()
   +   * @see \Drupal\locale\LocaleConfigSubscriber
   

   These docs are still c/p remnants :P

3. +++ b/core/modules/locale/src/Tests/LocaleTranslationUiTest.php
   @@ -152,7 +147,32 @@ public function testStringTranslation() {
   +    $this->config('system.performance')
   +      ->set('cache.page.use_internal', 1)
   +      ->set('cache.page.max_age', 300)
   +      ->save();
   

   Nit: This should not be necessary anymore; page cache is already enabled by default.

Also pinged Fabianx for a review.

First this needs a IS update :) (will review later)

Changed issues from #12

+++ b/core/modules/locale/src/LocaleEvents.php
@@ -0,0 +1,30 @@
+   * This event allows you to perform custom actions whenever a translated string
+   * is saved. The event listener method receives a
+   * \Symfony\Component\EventDispatcher\Event instance.
+   *

Actually, not that I think of it, this is wrong, it doesn't receive any event at all, so this comment can go away completely. Do we want to pass one for consistency and pass on the language that is being invalidated so that anyone else listening to this can enhance this in say contrib ?

For example, we tag a lot of stuff based on the roles a user has. invalidating all that just because you translated a role label would be a bit stupid, are there cases where it makes sense, e.g. views?

Now that permission hash is used as the cache context rather than roles, shouldn't we stop doing that? I failed to notice this on the issue that added the permission hash despite that being the whole idea of switching to it...

Addressed the comment in #15. I have no idea if I should do anything with the comment catch posted. Is there any action I should do now.

Marking as NR for #17.

IS updated.

Another, unrelated thing that I was just thinking of. Should "Flush all changes" invalidate certain cache tags, like rendered, explicitly, so that we can use that to notify external things as well?

Let's do this in another issue.

For example, we tag a lot of stuff based on the roles a user has. invalidating all that just because you translated a role label would be a bit stupid, are there cases where it makes sense, e.g. views?

Now that permission hash is used as the cache context rather than roles, shouldn't we stop doing that? I failed to notice this on the issue that added the permission hash despite that being the whole idea of switching to it...

That would indeed be stupid. And we no longer do it. Everything has migrated from "per role" to "per permissions".

Two silly, silly nitpicks:

1. +++ b/core/modules/locale/locale.module
   @@ -1054,6 +1056,9 @@ function _locale_refresh_translations($langcodes, $lids = array()) {
   +  Drupal::service('event_dispatcher')->dispatch(LocaleEvents::SAVE_TRANSLATION);
   

   Missing leading backslash.

2. +++ b/core/modules/locale/src/LocaleEvents.php
   @@ -0,0 +1,29 @@
   +   * This event allows you to perform custom actions whenever a translated string
   +   * is saved.
   

   80 cols.

Great!

+++ b/core/modules/locale/locale.module
@@ -1054,6 +1056,9 @@ function _locale_refresh_translations($langcodes, $lids = array()) {
   Cache::invalidateTags(array('locale'));
...
+  // Throw locale.save_translation event.
+  \Drupal::service('event_dispatcher')->dispatch(LocaleEvents::SAVE_TRANSLATION);

I think we should do the invalidation of the locale tag in this event too. Also I think we should pass the variables $langcodes and $lids so that this event can eventually replace _locale_refresh_translations.

Sorry, a few tiny nitpicks. Once that's done, this is RTBC again.

1. +++ b/core/modules/locale/src/LocaleEvent.php
   @@ -0,0 +1,62 @@
   +   * @var array
   ...
   +   * @return array $langCodes
   

   s/array/string[]/

2. +++ b/core/modules/locale/src/LocaleEvent.php
   @@ -0,0 +1,62 @@
   +   * @param array $langCodes
   ...
   +  public function __construct(array $langCodes, array $lids = array()) {
   

   s/$langcodes/$lang_codes/

3. +++ b/core/modules/locale/src/LocaleEvent.php
   @@ -0,0 +1,62 @@
   +   * The language codes.
   ...
   +   * The string identifiers.
   

   Returns the […]

+++ b/core/modules/locale/src/LocaleEvent.php
@@ -17,14 +17,14 @@ class LocaleEvent extends Event {
-  protected $langCodes;
+  protected $lang_codes;

@@ -37,21 +37,21 @@ class LocaleEvent extends Event {
   public function __construct(array $langCodes, array $lids = array()) {
-    $this->langCodes = $langCodes;
+    $this->lang_codes = $langCodes;

Gah… you changed it the wrong way around :(

We camelCase class properties.

We under_score parameters.

Sorry for the confusion :(

Yay!

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed f347360 and pushed to 8.0.x. Thanks!

diff --git a/core/modules/locale/locale.module b/core/modules/locale/locale.module
index 5056328..8e4a6c2 100644
--- a/core/modules/locale/locale.module
+++ b/core/modules/locale/locale.module
@@ -27,7 +27,6 @@
 use Drupal\locale\Locale;
 use Drupal\locale\LocaleEvent;
 use Drupal\locale\LocaleEvents;
-use Symfony\Component\EventDispatcher\Event;
 
 /**
  * Regular expression pattern used to localize JavaScript strings.

Removed unused use.

Continued in #3060007: Adding/updating interface translations should invalidate JavaScript library cache