Great module!.. Well written and easy to work with code, and delivers very simple and seamless functionality.

Here is a patch to integrate environment indicator with the environment module.

Skip to #4 and use this patch environment_integration_1.1_0.patch

If the environment module is installed and enabled, this patch adds a third radio button to the Enabled/Disabled radios: 'Integrate with environment module'

When in integration mode, if the environment module sets its environment as 'production' environment indicator functions as though disabled.

When environment is set to 'development' the $environment variable is put in the display text, in place of the default 'ENVIRONMENT INDICATOR' text. The user can override this with their own text and include an %env% token in which to insert the environment variable.

Other modules may also set the environment variable and these integrate seemlessly as it just inserts the value of the environment variable into the text string.

In this way, in our deployment environment, where we make heavy use of the environment variable. All we have to do is install environment indicator and set its radio to 'Integrate with environment module'. And the rest 'just works'.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Grayside’s picture

Hi Bevan, I was just in here checking on something for a query in Environment module about integration here #1347078: How to use Environment

I originally wrote #986902: Context Integration to facilitate integration with Environment. It does require Context module to be installed. Just an FYI, I leave it to the EI maintainer to decide what he cares to do ;)

thursday_bw’s picture

I have noticed to minor issues with the environment_integration.patch

* If environment module integration is enabled and the environment variable is not set, in settings.php or in drupal variables then the indicator text is empty
* the patch makes use of variable_get a couple of times, and doesn't supply the default value as the second argument thus generating php warnings.

I'll see if I can get around to create a better patch

mrfelton’s picture

Status: Active » Needs work
thursday_bw’s picture

patch including fixes to issues in comment #2

mrfelton’s picture

Status: Needs work » Needs review
garphy’s picture

Isn't the proposed patch in #4 missing the hook_environment_switch() ?

The enabled state and the displayed text could be updated only on environment switches, leaving the original hook_init() logic untouched.

garphy’s picture

BTW, to clarify : patch works for me. Just wondering if the extra bit in hook_init() is more elegant that integrate through environment hooks.

mrfelton’s picture

Version: 6.x-1.0 » 7.x-1.x-dev
Status: Needs review » Needs work

Conceptually, I really like the idea behind this, although there are some issues with the implementation:

Firstly, it should be written for Drupal 7 first ;)

+      $output .= '<p>'. t('The Environment Indicator can integrate with the <a href="http://drupal.org/project/environment">environment module</a> and then be \'aware\' of which environment it is in and change its behaviour accordingly. ie. When in production it functions essentially as though it is disabled and displays no indicator. When in any other environment it displays either the environment value (eg development) or the user defined display text, replacing the token %exp% with the environment.', array('@permissions' => url('admin/user/permissions', array('fragment' => 'module-environment_indicator')))) .'</p>';
+

This references admin/user/permissions which does not exist in Drupal 7 (should be admin/people/permissions).

   $form['environment_indicator_enabled'] = array(
     '#type' => 'radios',
     '#title' => t('Status'),
-    '#default_value' => variable_get('environment_indicator_enabled', 1)? '1' : '0',
+    '#default_value' => variable_get('environment_indicator_enabled', 1),
     '#options' => array(t('Disabled'), t('Enabled')),
     '#description' => t('Should the Environment Indicator display?'),
   );
+
+  //If the environment module is installed and enabled, add a new environment module button 
+  if (module_exists('environment')) {
+    $form['environment_indicator_enabled']['#options'][] =  t('Itegrate with environment module');
+  }

When there were just 2 settings, 1 and 0 was ok. The reason I used ? '1' : '0', was to allow people to also use a value of TRUE in settings.php to enable. However, this introduces a 3rd option. With three options (there could be more in the future), I would rather this used defined values, eg:

define('ENVIRONMENT_INDICATOR_STATUS_DISABLED', 'enabled');
define('ENVIRONMENT_INDICATOR_STATUS_ENABLED', 'enabled');
define('ENVIRONMENT_INDICATOR_STATUS_ENVIRONMENT', 'environment');

However, thinking about this more, your third status is really not a status at all, it is a mode that controls the status. We still really have only 2 states - enabled or disabled. So how about we leave the states as they are, and add another variable called environment_indicator_mode with values of 'default' and 'environment', and this would then be used to override the status based on the environment when set to 'environment'.

$settings['text'] = strtoupper(variable_get('environment', 'ENVIRONMENT NOT SET')); 

Might be better to use environment_current() to find out the current environment rather than a direct call to variable_get.

   if ($setting = variable_get('environment_indicator_text', 'ENVIRONMENT INDICATOR')) {
-    $settings['text'] = check_plain($setting);
+    //Use the environment modules' environment variable for display text if enabled
+    if (variable_get('environment_indicator_enabled', 0) == 2) {
+      if ($setting == 'ENVIRONMENT INDICATOR' ||!$setting) {
+        //No text set, use the environment variable
+        $settings['text'] = strtoupper(variable_get('environment', 'ENVIRONMENT NOT SET')); 
+      } else {
+        //User defined text, do a token replacement
+        $settings['text'] = str_replace ( '%env%' , variable_get('environment', 'ENVIRONMENT NOT SET') , check_plain($setting));
+      }
+    } else {
+      $settings['text'] = check_plain($setting);
+    }

How come you are only performing the string replacement if the text has not been set in settings.php? What about the case where someone wants to integrate with environment, but also override the text in settings.php? For example, on my local host I might want the text to read "Tom's Local (%env%)", and not just the name of the environment so I should be able to set that in settings.php. Also, we should probably show the label of the environment rather than the machine name. This can be found out with a call to environment_environment()

mrfelton’s picture

Issue summary: View changes

Fixes to patch

thursday_bw’s picture

I figure, it's been 3 years and I have clearly dropped the ball on this one.
So here is a comment that is nothing more than a first step to getting around to pulling this patch up to speed.