Move ConfigurableLanguage hook implementations in language module to the entity

Looks like these were not converted when postSave() and postDelete() methods were introduced.

Please review.

Here is a updated patch. Not sure if it is the right way.

+++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
@@ -178,6 +185,15 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
+    unset($prefixes[$this->id()]);
...
+    unset($domains[$this->id()]);

$this is not accessible in static context.

Let me try.

Here is the updated patch, based on @andypost feedbacks.

@segi you must change the Status to "Need review" for testbot test it.

I have reviewed the last patch, everything seems correct. The only recommendation, after to talk with @tstoeckler about this issue, is to use the object instead of these two functions: language_negotiation_url_domains() and language_negotiation_url_domains_save

- just wanted to remove the hashtag from the issue tag ... yannickoo was faster -

Updated the patch, I removed call of language_negotiation_url_domains() and language_negotiation_url_domains_save() functions.

Hi,
Issue reviewed, everything seems correct :)

I fixed the text indent, but I used this indent because I saw it in the language_negotiation_url_domains_save() and language_negotiation_url_prefixes_save() functions, so the indent wrong there as well.

Looks good to me, and still applies.

1. This issue is a task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary. However making this about fixing config overrides bleeding in also makes this address a bug (partially).

2. +++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
   @@ -144,6 +144,17 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
   +    if ($this->isLocked() || $update) {
   +      return;
   +    }
   

   Lets not do an early return let's wrap the write to config in an if (!$this->locked() && !$update) {.

3. +++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
   @@ -144,6 +144,17 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
   +    $domains = \Drupal::config('language.negotiation')->get('url.domains');
   +    $domains[$this->id()] = '';
   +    \Drupal::configFactory()->getEditable('language.negotiation')
   +      ->set('url.domains', $domains)
   +      ->save();
   
   @@ -178,6 +189,18 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
   +    // Remove language from language prefix list.
   +    $prefixes = \Drupal::config('language.negotiation')->get('url.prefixes');
   +    unset($prefixes[$language_manager->getCurrentLanguage()->getId()]);
   +
   +    // Remove language from language domain list.
   +    $domains = \Drupal::config('language.negotiation')->get('url.domains');
   +    unset($domains[$language_manager->getCurrentLanguage()->getId()]);
   +    \Drupal::configFactory()->getEditable('language.negotiation')
   +      ->set('url.prefixes', $prefixes)
   +      ->set('url.domains', $domains)
   +      ->save();
   

   Let's just do one config getEditable() instead of the get()s in each of the methods here which will also fix some of the config override bleeding into active config issues.

   The other issue can add the test and address the other problems with the procedural functions.

+++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
@@ -144,6 +144,17 @@ public function postSave(EntityStorageInterface $storage, $update = TRUE) {
+    $domains = \Drupal::config('language.negotiation')->get('url.domains');
...
+    \Drupal::configFactory()->getEditable('language.negotiation')

@@ -178,6 +189,18 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
+    $prefixes = \Drupal::config('language.negotiation')->get('url.prefixes');
...
+    $domains = \Drupal::config('language.negotiation')->get('url.domains');
...
+    \Drupal::configFactory()->getEditable('language.negotiation')

Why not just get this config object once (in each method) and manipulate it directly?

I rerolled the patch. My plan is I will continue with the fixes based on feedbacks.

+++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
@@ -178,6 +189,18 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
+    // Remove language from language prefix list.
+    $prefixes = \Drupal::config('language.negotiation')->get('url.prefixes');
+    unset($prefixes[$language_manager->getCurrentLanguage()->getId()]);
...
+    // Remove language from language domain list.
+    $domains = \Drupal::config('language.negotiation')->get('url.domains');
+    unset($domains[$language_manager->getCurrentLanguage()->getId()]);
+    \Drupal::configFactory()->getEditable('language.negotiation')
+      ->set('url.prefixes', $prefixes)
+      ->set('url.domains', $domains)
+      ->save();

Why are we using getCurrentLanguage() here? We are supposed to react to deleting of possibly multiple entities, not just one AFAIS.

I tried to fix the patch base on feedbacks, but the requests for config management weren't clean for me, so I did a solution. I hope I step closer to final solution.

#2403229: language.negotiation configuration can have overrides bleed in already added the test coverage and resolved the bleeding. This one just cleans up the code now.

Trackertest GET http://ec2-52-88-182-40.us-west-2.compute.amazonaws.com/checkout/user/login returned 0 (0 bytes) does not seem related at all.

Yes this change makes sense and has no risk but it does not meet our beta guidelines for Drupal 8 inclusion at this time. In my opinion this can go in after Drupal 8 release. I don't think we need to wait to 8.1.

Tagging with a new tag "D8 patch release target" since I don't think we have a tag for this.

Removing from sprint then.

yeah.. 8.0 beta is over now :)

I reviewed the last patch it looks good and the test went well.

+++ b/core/modules/language/src/Entity/ConfigurableLanguage.php
@@ -173,6 +181,18 @@ public static function postDelete(EntityStorageInterface $storage, array $entiti
+    $prefixes = $config->get('url.prefixes');
+    $domains = $config->get('url.domains');
+    unset($prefixes[$entity->id]);
+    unset($domains[$entity->id]);
...
+    // Save the modified settings.
+    $config->set('url.prefixes', $prefixes)
+      ->set('url.domains', $domains)

This can be written even more succinctly by using $config->clear('url.prefixes.' $entity->id()) (and the same for the domains).

Updated patch as per comment #55 & also added interdiff.

maybe this patch will solve the PHPLint issue.

my bad few things i missed.

Nice. We could save a few more charactes with chained calls, even skip the $config variable completely but not sure if that really is an improvements, lets see what @alexpott thinks.

Adding credit. The lack of chaining is okay. Makes no difference really.

Committed 877e4a8 and pushed to 8.6.x. Thanks!