Problem/Motivation

In a use case where there are multiple frontend enviroments where some of them are inactive (status=0) it would be nice to only show active ones in the toolbar.
Inactivity of some frontend environments can be, in example, due to different deployment environments.

Steps to reproduce

Add and configure multiple frontend environments.
Set the status of one of them to 0 (via config of settings.php)
The environment is still shown in the toolbar.

Proposed resolution

In the hook_toolbar, check if the environment status is 1 before adding it to toolbar.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 3219055-9.patch2.19 KBpetar_basic
#8 3219055-8.patch2.04 KBlarowlan
#8 3219055-interdiff-8.txt533 byteslarowlan
#5 dont_show_inactive_front_envs.patch1.7 KBpetar_basic
#3 dont_show_inactive_front_envs.patch4.98 KBpetar_basic
#2 build_hooks_LDP-754_dont_show_inactive_front_envs.patch4.98 KBpetar_basic

Comments

petar_basic created an issue. See original summary.

petar_basic’s picture

petar_basic commented
StatusFileSize
new build_hooks_LDP-754_dont_show_inactive_front_envs.patch4.98 KB

Here is a patch that adds checks for environment status before adding it to toolbar.

petar_basic’s picture

petar_basic commented
StatusFileSize
new dont_show_inactive_front_envs.patch4.98 KB

Changing the name of the patch.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 3.1.4 » 3.x-dev
Status: Active » Needs work
Issue tags: +Needs tests, +Novice 
+++ b/build_hooks.module
@@ -97,57 +97,62 @@ function build_hooks_toolbar() {
+        $items['build_hooks_deployments'] = [
+          '#cache' => $cache,
+          '#type' => 'toolbar_item',

We need to return the #cache here so that the item can be invalidated properly.

So perhaps we just return the empty cache item.

Can you add a new test for this, see \Drupal\Tests\build_hooks\Functional\UiTest::assertToolbarIntegration

For an example.

The test should disable the env, make sure the button isn't there, then re-enable it.

petar_basic’s picture

petar_basic commented
StatusFileSize
new dont_show_inactive_front_envs.patch1.7 KB

I improved the patch so it only looks for environments with status true instead of having multiple if-s.

Added a test that checks if an active environment is shown in the toolbar and that an inactive one is not shown.

petar_basic’s picture

petar_basic commented
Status: Needs work » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Needs work 
+++ b/tests/src/Functional/UiTest.php
@@ -284,4 +284,22 @@ class UiTest extends BrowserTestBase {
+  private function assertInactiveEnvironmentToolbarIntegration(FrontendEnvironmentInterface $environment) {

We need to call this method from one of the `test{Something}` methods in order for it to be run

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 3219055-interdiff-8.txt533 bytes
new 3219055-8.patch2.04 KB

Status: Needs review » Needs work

The last submitted patch, 8: 3219055-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

petar_basic’s picture

petar_basic commented
StatusFileSize
new 3219055-9.patch2.19 KB

Seems like the entity has to be saved after change. And the test should be run before the last one.

petar_basic’s picture

petar_basic commented
Status: Needs work » Needs review

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Fixed

Thanks, cutting 3.2.1 with this in it

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

https://www.drupal.org/project/build_hooks/releases/3.2.1

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.