Make version indicator source and version indicator source fallback configurable.

I see this issue as well. Thanks for reporting, I was just about to report it myself.

Hi All, I followed the steps. Required and installed the module. Cleared all cache, ran drush sdel environment_indicator.current_release command and testing after saving the setting form with toolbar and show favicon checkbox. Still i am not seeing any random string here. Can you elaborate little bit more when this one occurring or i have done something wrong with debugging.
I am using D9.4.8 and php 8.1.
Module V4.0.11.

public function getCurrentRelease() {
    $current_release = $this->state->get('environment_indicator.current_release');
    if ($current_release !== NULL) {
      return (string) $current_release;
    }

    $deployment_identifier = $this->settings->get('deployment_identifier');
    if ($deployment_identifier !== NULL) {
      return (string) $deployment_identifier;
    }

    return NULL;
  }

This code is returning NULL in starting as there is no state var set for current_release and uncommented deployment_identifier line in settings.php file.

@ankitjhakal

What was apparently a random string is actually the text content of my project's deployment_identifier file (in my current docker container Lando appserver environment this is "G %9[+R=").

I'll do more research on this and how the file gets created in my local development environment.

It looks like a build artifact from Acquia BLT setup script commands.

Confirmed: the deployment_identifier file is created as a random string during Acquia BLT setup during drupal:install, and during artifact generation during blt deploy, where it can be specified as a tag for the deploy release.

Looks like this is not a bug.

Hi utcwebdev, so can you please move this issue in closed status.

@utcwebdev It is in fact a bug, because the module should be taking the BLT commands in consideration, I'm having the same issue.

I created a patch to get rid of the identifier version in the title. Maybe we could have some kind of settings asking us if we want to have the version indentifier before the title, that could fix the issue in these cases

@DarkteK Thank you for the patch. I agree, an option to disable/enable the identifier would be helpful.

I think the module is behaving as designed, but not as expected, when used with the BLT workflow. Unless there is a tag specified during a deployment, BLT generates the random string as deployment_identifier. I still see a seemingly random string after CI/CD develop-build branch deployment to our hosted Acquia dev environment, though it looks suspiciously like a failed timestamp. Also, the favicon icon overlay text is borked.

Perhaps this is more of a BLT issue where the deployment identifier could be improved. The BLT developer justification for it to be a random string is because "Drupal uses the deployment identifier to seed the APCu cache", and generating a random string is the most efficient/fast method. The identifier helps to clear the Twig cache when theme updates are deployed, and class autoloader cache when a module is updated.

+1 to making it optional, I guess that was an oversight when I initially added it.

Having the option is really nice when using, for example, tag based deployments where you use ENV to set the deployment_identifier like so in settings:

if (getenv('GIT_BRANCH')) {
  $settings['deployment_identifier'] = getenv('GIT_BRANCH');
}

It's useful when you are already using deployment identifier in a unique way, so you don't have to set the state as well on top of it. Not everyone is using deployment identifier (you should), or does but it doesn't provide anything meaningful in the case of BLT, so making it optional is nice.

While on the topic of deployment identifier, but somewhat off-topic, I am using the following in my settings.local.php to set it to the current branch, this works out really well!

// Set deployment_identifier to current branch
$git_path = __DIR__ . '/'.'../../../.git/'; //adjust path based on your project structure
if (file_exists($git_path)) {
  $settings['deployment_identifier'] = str_replace('refs/heads/', '', trim(substr(file_get_contents($git_path . 'HEAD'), 4)));
}

Thanks for reporting this! Just chiming in to add that this also shows up on Pantheon, where the latest commit hash appears before the environment name when on the development environment, and the Pantheon deployment ID (e.g. "test_35") appears in test and live.

I personally agree with the suggestion of this being a configurable option that could be enabled or disabled. Even if not, I wonder about the possibility of moving it to display after the environment name, rather than before [e.g. Local (rbVZt`Z= )], which would prevent the favicon overlay from being affected.

Thanks!

Configurable would be nice but thankfully there is a straightforward code path to changing this behavior if you need to:

/**
 * Implements hook_toolbar_alter().
 */
function hook_toolbar_alter(&$items) {
  if (!empty($items['environment_indicator'])) {
    $env_name = \Drupal::service('config.factory')
      ->get('environment_indicator.indicator')
      ->get('name');
    $items['environment_indicator']['tab']['#title'] = $env_name;
    $items['environment_indicator']['tab']['#attached']['drupalSettings']['environmentIndicator']['name'] = $env_name;
  }
}

Also seeing the "random" string on a platform.sh site. Wasn't displayed on local environment but appeared once I deployed code to a platform.sh environment.

+1 for this being configurable. It can be confusing to editors who don't know what the (seemingly) random string is.

Until there is a patch, literally the easiest thing to do is set the state for the current release per the README. You can see in the code https://git.drupalcode.org/project/environment_indicator/-/blob/4.x/src/... that the deployment identifier is only used as a fallback.

Needs IS summary update for suggested new configuration setting. Affects all 4.x-dev (and all 4.x releases since 4.0.11).

Issue was introduced with the changes committed for #2610208: Move procedural functions to a manager due to the deployment identifier now being used (if available) as a fallback for current_release value that is prepended to the environment title when the current_release value is not set in the state.

Closed #3325538: Remove extra strings added to the environment title after upgraded to 4.0.11 as duplicate of this issue. Credit should be included for the contributors to that issue when this fixed.

We have also been experiencing the random characters in parentheses prepended to the environment label. We also observed that the characters come from a deployment_identifier file located in our project's root directory.

I'm submitting patch #20 which is similar to #9, but it keeps the environment_indicator.current_release state variable check. It only replaces the deployment_identifier fallback with a \Drupal::VERSION fallback.

I was for disabling the fallback but I really like @ericjenkins' idea of showing the Drupal version. I think that would be useful info to highlight, whereas the deploy ID is really really not.

Adding static patch for CI tools with all the changes in the merge request as of 17c96bd7b7b4d916f43bd30dd58806e21fc956bf against the 4.x branch.

I've added a merge request with the fallback option.
https://git.drupalcode.org/project/environment_indicator/-/merge_request...

You should be able to view a site with this merge request installed here: https://mr44-ujeql2avin1q2wcv5myxo47tlli6m5z1.tugboatqa.com/
username:admin
password: admin

Configuration page here: https://mr44-ujeql2avin1q2wcv5myxo47tlli6m5z1.tugboatqa.com/admin/config...

Adding static patch of https://git.drupalcode.org/project/environment_indicator/-/merge_request...

Suggestion: when the none option is selected for the Source of version identifier to display field, a fall back option should not be allowed.

I've updated the merge request to disallow setting a fallback if a primary version indicator source is set to none.

The changes in @trackleft2's MR work great for me in my testing. I really like that it adds additional configuration options for what is used as the version indicator and the fallback.

Uploading static patch with latest changes from MR (usable with 4.0.19).

Excelent work. I'll release it in the 4.0.20 version.