Language Aware Destination Rule

The patch looks good on first sight.
Let's give a week to the community to test it with sites using i18n and sites w/o i18n.
I'll test myself on two test sites.
Also I will try and find out if we need to write update hooks for existing module installs.

Thanks for the patch, @alienzed! After a bit of testing and review, it will be committed.

The patch does add some coding standard violations. https://www.drupal.org/pift-ci-job/1225168 says that there are nine more.

1. +++ b/src/Entity/LoginDestination.php
   @@ -179,6 +186,13 @@ class LoginDestination extends ConfigEntityBase implements LoginDestinationInter
   +   * @inheritdoc
   

   Should be {@inheritdoc}

2. +++ b/src/Entity/LoginDestination.php
   @@ -179,6 +186,13 @@ class LoginDestination extends ConfigEntityBase implements LoginDestinationInter
   +  ¶
   

   Extra whitespace that should be removed.

3. +++ b/src/Form/LoginDestinationRuleForm.php
   @@ -122,6 +123,17 @@ class LoginDestinationRuleForm extends EntityForm {
   +    foreach(\Drupal::languageManager()->getLanguages() as $key => $value){
   
   +++ b/src/LoginDestinationManager.php
   @@ -120,6 +120,12 @@ class LoginDestinationManager implements LoginDestinationManagerInterface {
   +      if ($destination_language != '' && $destination_language != $lang_code ){
   

   On space is needed after closing parenthesis. 1 space after 'foreach' keyword needed.

4. +++ b/src/Form/LoginDestinationRuleForm.php
   @@ -146,7 +158,6 @@ class LoginDestinationRuleForm extends EntityForm {
   -
   

   Removement of this line seems to not be necessary.

+++ b/src/Controller/LoginDestinationListBuilder.php
@@ -67,6 +68,9 @@ class LoginDestinationListBuilder extends DraggableListBuilder {
+      '#markup' => $entity->getLanguage() != "" ? $entity->getLanguage() : $this->t("All languages"),

+++ b/src/Form/LoginDestinationRuleForm.php
@@ -122,6 +123,17 @@ class LoginDestinationRuleForm extends EntityForm {
+    $languages[''] = $this->t("All languages");

For consistency, use single quotes here (double quotes should only be used to avoid escaping the string).

regarding the update hooks:
- these are functions that do procedures like cleaning the cache, or changing (altering) drupal's DB schema (fields, tables) - most patches don't need to migrate/alter data/db fields structure but some do. A common trick indeed is when adding a new config to have good default like NULL, '', false, that will keep things working retro-actively.

- in general your logic seems right, that having a good default namely empty string "", should work ok. But I need to check a few things and then we'll be sure. Thing 1 to check is: how your patch works on an already installed module and 2. I need to go through the code reading it slowly and trying to figure out the implications.

@MegaChriz: thanks for the coding standards work and I'd like to ask you this:
Do you have an idea if adding
+ language:
+ type: string
+ label: 'Language'

to config/schema/login_destination.schema.yml would mean we need an update hook?

I hope you know enough about the D8 api to answer that.

@rsvelko
I don't know if adding config keys require update hooks in D8. I haven't done much with update hooks in D8 yet. So that would need to be investigated. I know the Webform module added lots of update hooks (and probably also new config keys?).

ok, more and more I'm leaning towards that if the new config vars have sane defaults, that should cover all existing installs.
We just need to be carefull when checking the variable value and treat null, not defined vars / keys properly, as well as empty strings.

1.
> $language = '' is in fact supplying the default value, all should be well.
= I agree

2. lang codes instead of names in LD rules list - I like lang codes there as they are short.

3. I've tried the patch, tested with one-time login links + language filter/rule/condition - works. Tested login trigger + language - works (tried 2 langauges and 2 destination pages - so lang 1 -> page 1, lang 2 -> page 2).

No regressions, works well on existing site w/o need for update hook.

I'm committing this one now. Will be part of release 8.x-alpha2.