Optionally enable disabled languages for entity translation.

Just applied the patch - works like a charm. Thanks.

Thanks for the patch, actually my plan was to inherit i18n settings where possible.

+++ b/entity_translation.admin.inc
@@ -85,6 +92,13 @@ function entity_translation_overview($entity_type, $entity) {
   // @todo: Do we want only enabled languages here?

Yes, only enabled languages makes sense in my case, hopefully there is not too many edge cases where this is needed?

Oh, I just noticed you have a nice little hidden gem in the latest dev:)

variable_get('entity_translation_languages_enabled', FALSE)

drush vset entity_translation_languages_enabled TRUE

Thank you @plach!

This is a little an old issue but I'd like to go ahead on this.

as joelpittet told, the function to toggle the availability of disabled language already exists. So adding a form field in the setting page is enough, maybe. Here is a simple patch for that.

(Disabled language is included by default and it may be better to revise the issue title...)

Thanks for this @hgoto, tests are now clean.

The FAPI element addition is perfectly fine as provided in the patch. There is one condition though that should be taken into account, and this has to do with the @todo comment in the first if block:

function entity_translation_languages($entity_type = NULL, $entity = NULL) {
  if (isset($entity) && $entity_type == 'node' && module_exists('i18n_node')) {
    // @todo Inherit i18n language settings.
  }
  elseif (variable_get('entity_translation_languages_enabled', FALSE)) {
    $languages = language_list('enabled');
    return $languages[1];
  }
  return language_list();
}

In case i18n_node is enabled entity_translation_languages_enabled will be ignored. In that case, I would hide the FAPI element as well, to avoid confusion.
N.B.1: This @todo is what started the whole issue, but implementing the i18n settings inheritance should be handled in a different issue.
N.B.2: We should think about having some tests for this, no matter how trivial the functionality, would be great to have them in the patch.

@hgoto: Short update:

1. The @todo now has its own issue #2872194: entity_translation_languages() should inherit i18n language settings
2. The now obsolete #2339495: English language appears in list even if it is disabled has a patch that provides some tests already

@stefanos.petrakis, thank you for your review and updates.

In case i18n_node is enabled entity_translation_languages_enabled will be ignored. In that case, I would hide the FAPI element as well, to avoid confusion.

I see. I agree with you that it's better to avoid confusion. How about disabling the field keeping it visible? It may be nicer for easier understanding. Please see the attached image (took after the patch is applied).

The now obsolete #2339495: English language appears in list even if it is disabled has a patch that provides some tests already

Thank you! I adjusted the test proposed in #2339495: English language appears in list even if it is disabled and merged into this patch.

Thanks @hgoto, good stuff! We are close:

+++ b/tests/entity_translation.test
@@ -134,6 +134,19 @@ class EntityTranslationTestCase extends DrupalWebTestCase {
   }
 
   /**
+   * Disable a language which is in the language list

Missing a full stop at the end of this line.

+++ b/tests/entity_translation.test
@@ -270,11 +283,32 @@ class EntityTranslationTranslationTestCase extends EntityTranslationTestCase {
+    $this->assertRaw('value="en"', 'English is enabled and available');
+    $this->assertRaw('value="es"', 'Spanish is enabled and available');
+    $this->assertRaw('value="fr"', 'French is available even if the language is disabled');
+
+    variable_set('entity_translation_languages_enabled', TRUE);
+    $this->drupalGet('node/add/page');
+    $this->assertRaw('value="en"', 'English is available because the language is enabled');
+    $this->assertRaw('value="es"', 'Spanish is available because the language is enabled');
+    $this->assertNoRaw('value="fr"', 'French is not available when the language is disabled and the option entity_translation_languages_enabled is enabled.');

After reading this code some, I am not sure we need to check for 'en' and 'es' at all, enabled languages will always be part of the list. And the function's name hints to testing disabled languages only.

Additionally, I would change the wording of the messages slightly, how about:

• French is disabled and available since entity_translation_languages_enabled is FALSE or not set.
• French is disabled and unavailable since entity_translation_languages_enabled is TRUE.

+++ b/entity_translation.admin.inc
@@ -18,6 +18,18 @@ function entity_translation_admin_form($form, $form_state) {
+  $description = t('If checked, disabled languages will not show up as available languages.');
+  if (module_exists('i18n_node')) {
+    $description .= ' <strong>' . t('This restriction is not effective when Multilingual content (i18n_node) module is enabled.') . '</strong>';
+  }
+  $form['entity_translation_languages_enabled'] = array(
+    '#type' => 'checkbox',
+    '#title' => t('Use only enabled languages'),
+    '#description' => $description,
+    '#default_value' => variable_get('entity_translation_languages_enabled', FALSE),
+    '#disabled' => module_exists('i18n_node'),
+  );

My previous comment about conditionally hiding this has a flaw. Looking at the code, entity_translation_languages() gets called in various places, so, even if i18n_node is enabled, this setting still makes sense e.g. for entities that are not nodes. So, on second thought, I would leave this FAPI element always visible and simply add a note along the lines of what you wrote. Here is a little draft/example:

  $form['entity_translation_languages_enabled'] = array(
    '#type' => 'checkbox',
    '#title' => t('Use only enabled languages'),
    '#description' => t('If checked, disabled languages will not show up as available languages. This setting does not apply to content types that are configured to use the Multilingual content (i18n_node) module as this module provides its own configuration.'),
    '#default_value' => variable_get('entity_translation_languages_enabled', FALSE),
  );

Thanks!

@stefanos.petrakis thanks! I agree with you on all the points you mentioned. I revised the patch.

In addition, I added the type string in the doc comment in the added method.

-   * @param $langcode
+   * @param string $langcode

Committed, thanks!

Great. Thank you!