Create a Language check Condition

Snagging this after clearing with EclipseGc on irc

First cut, needs more tests - will tackle those tomorrow.

So this adds more unit tests and is ready for a serious look

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,140 @@
+        '#default_value' => !empty($this->configuration['langcodes']) ? $this->configuration['langcodes'] : array(),
+        '#options' => $langcodes_options,
...

$langcodes_options looks like not previously defined
possibly we should...

/**
 * description...
 */
private $langcodes_options;

all other looks good for me
+1RTBC

looking at the test

+++ b/core/modules/language/lib/Drupal/language/Tests/Condition/LanguageConditionTest.phpundefined
@@ -0,0 +1,131 @@
+    // Setup English.
+    $default_language = language_save(language_default());

$default_language never used here
Looks like it should be removed or used in tests )

langcode_options is defined here

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,140 @@
+      foreach ($languages as $language) {
+        // @todo $language->name is not wrapped with t(), it should be replaced
+        //   by CMI translation implementation.
+        $langcodes_options[$language->langcode] = $language->name;

bump

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,125 @@
+    if (language_multilingual()) {
...
+    else {
+      $form['language']['langcodes'] = array(
+        '#type' => 'value',
+        '#value' => !empty($this->configuration['langcodes']) ? $this->configuration['langcodes'] : array()
+      );

Could this be handled cleaner with '#access' => language_multilingual()?

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,125 @@
+    $language_list = language_list(LANGUAGE_ALL);
+    $selected = $this->configuration['langcodes'];
+    // Reduce the language list to an array of language names.
+    $language_names = array_reduce($language_list, function(&$result, $item) use ($selected) {
+      if (!empty($selected[$item->langcode])) {
+        $result[$item->langcode] = $item->name;
+      }
+      return $result;
+    }, array());
+
+    // Collate language type names.
+    $language_types = language_types_info();
+    $language_type_options = array();
+    foreach ($language_types as $type_key => $info) {
+      if (!empty($info['name'])) {
+        $language_type_options[$type_key] = $info['name'];
+      }

Why is building an options list done 2 different ways here? I love me some closures, but a foreach would be simpler.

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,125 @@
+    $language = $this->getContextValue('language');
+    // Language visibility settings.
+    if (!empty($this->configuration['langcodes'])) {
+      if (empty($this->configuration['langcodes'][$language->langcode])) {
+        return FALSE;
+      }

I have to look at the condition implementation, but this seems to suggest that this plugin will always evaluate, even if it was never configured. Is that correct?

+++ b/core/modules/language/lib/Drupal/language/Tests/Condition/LanguageConditionTest.phpundefined
@@ -0,0 +1,131 @@
+    $this->assertTRUE($condition->execute(), 'Language condition passes as expected.');

This should have been the name :)

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,109 @@
+      $form['langcodes'] = array(
+        '#type' => 'checkboxes',
+        '#title' => t('Show this block only for specific languages'),
+        '#default_value' => !empty($this->configuration['langcodes']) ? $this->configuration['langcodes'] : array(),
+        '#options' => $langcodes_options,
+        '#description' => t('Show this block only for the selected language(s). If you select no languages, the block will be visibile in all languages.'),
+      );
+    }

Is this only going to be used for blocks?

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,109 @@
+    // Reduce the language list to an array of language names.
+    $language_names = array_reduce($language_list, function(&$result, $item) use ($selected) {
+      if (!empty($selected[$item->langcode])) {
+        $result[$item->langcode] = $item->name;
+      }
+      return $result;
+    }, array());

Is it commonplace / accepted now to use inline functions like this? I have not seen this much in Drupal core :)

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,109 @@
+    if (!empty($this->configuration['negate'])) {
+      return t('The language is not @languages.', array('@languages' => $languages,));
+    }
+    return t('The language is @languages.', array('@languages' => $languages,));

The commas at the ends do not fit our code style AFAIS, we don't include them if the array is all in one line.

+++ b/core/modules/language/lib/Drupal/language/Tests/Condition/LanguageConditionTest.phpundefined
@@ -0,0 +1,129 @@
+    variable_set('language_default', array(
+      'langcode' => 'it',
+      'name' => 'Italian',
+      'direction' => 0,
+      'weight' => 0,
+      'locked' => 0,
+    ));

You can/should language_load('it') instead to avoid repeating properties from above.

+++ b/core/modules/language/lib/Drupal/language/Tests/Condition/LanguageConditionTest.phpundefined
@@ -0,0 +1,129 @@
+    // Use the language interface now.
+    $condition = $this->manager->createInstance('language')
+      ->setConfig('langcodes', array('en' => 'en', 'it' => 'it'))
+      ->setContextValue('language', language(LANGUAGE_TYPE_INTERFACE));
+

interface language (vs language interface :D)

Also I don't see how is this different from checking the content language, since they are negotiated for the same value in this test. Merely changing the default language and reiniting the language manager will change both. So this is not really testing what happens if the interface language and the content language are different. That would need separate setup for them in the test instead of changing the default language and re-initing the manager.

Looks good. Only one note then.

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,109 @@
+        '#description' => t('Return TRUE only for the selected language(s). If no language is selected, this will return TRUE.'),

Is this not shown to regular users on the UI? "Returns TRUE" sounds like the least useful text if this ever ends up on a front facing UI.

Looks good to me now. I cannot vouch for the correctness of the condition API use, but the language stuff looks good.

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,109 @@
+   * Overrides \Drupal\Core\Condition\ConditionPluginBase::buildForm().
...
+   * Overrides \Drupal\Core\Condition\ConditionPluginBase::submitForm().
...
+   * Implements \Drupal\Core\Executable\ExecutableInterface::summary().
...
+   * Implements \Drupal\condition\ConditionInterface::evaluate().

Should be {@inheritdoc} now

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,109 @@
+    // Reduce the language list to an array of language names.
+    $language_names = array_reduce($language_list, function(&$result, $item) use ($selected) {
+      if (!empty($selected[$item->langcode])) {
+        $result[$item->langcode] = $item->name;
+      }
+      return $result;
+    }, array());
+
+    if (count($this->configuration['langcodes']) > 1) {
+      $languages = implode(', ', $language_names);
+    }
+    else {
+      $languages = array_pop($language_names);

I think this could use some more inline docs. The internal part of array_reduce, and both the if and else.

+++ b/core/modules/language/lib/Drupal/language/Plugin/Core/Condition/Language.phpundefined
@@ -0,0 +1,109 @@
+    if (!empty($this->configuration['langcodes'])) {
+      if (empty($this->configuration['langcodes'][$language->langcode])) {
+        return FALSE;
+      }
+    }
+    return TRUE;

I'm pretty sure this is redundant, since both are wrapped in empty, you only need one.
Could just be:
return !empty($this->configuration['langcodes'][$language->langcode]);

Those comments really help, thanks!

Committed 4e833c9 and pushed to 8.x. Thanks!

I think the conditions change notice might need an update...

Updated change notice here http://drupal.org/node/1961370.

Removing tag.