Cannot modify non-default containers or enable use_collection setting when multiple google_tag_container entities already exist

Created on 9 September 2023, 12 months ago
Updated 23 February 2024, 7 months ago

Problem/Motivation

When multiple google_tag_container configuration entities already exist in a site's active configuration it is not possible to enable the google_tag.settings use_collection configuration setting or modify the non-default container settings via the admin UI.

This seems to be related to code existing in a few places in this module:

  • Code in the SettingsForm always disables the form element for the google_tag.settings use_collection setting if more than 1 entity is returned by $this->entityTypeManager->getStorage('google_tag_container')->loadMultiple()
  • Code in the GoogleTagController prevents access to the controller methods for some of the entity.google_tag_container routes if the google_tag.settings use_collection setting is disabled

Steps to reproduce

I encountered this issue when installing a custom module that depends on this module and includes an exported google_tag_container config entity (in config/install) on a site that had the Google Analytics module installed. This resulted in 2 google_tag_container config entities that had been created programmatically:

  • 1 imported from the custom module's config/install directory
  • 1 that was created by this module's GoogleAnalyticsMigrator service

There are probably other ways that multiple google_tag_container config entities could be created outside of the normal admin UI workflow.

Proposed resolution

  • Change the code in the module SettingsForm so that the use_collection form element isn't disabled when multiple containers exist and the setting is not yet enabled (this change is definitely needed)
  • Consider changing the code in GoogleTagController to not restrict access to the entity.google_tag_container routes if multiple containers exist

Remaining tasks

User interface changes

API changes

Data model changes

πŸ› Bug report
Status

Fixed

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States joegraduate Arizona, USA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @joegraduate
  • πŸ‡ΊπŸ‡ΈUnited States joegraduate Arizona, USA
  • Status changed to Needs review 12 months ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update 12 months ago
    54 pass
  • πŸ‡ΊπŸ‡ΈUnited States joegraduate Arizona, USA

    The attached patch changes the code that disables the form element for the use_collection setting in the SettingsForm to only disable the form element if the setting is already enabled and multiple containers already exist.

    It occurred to me that another way this could potentially be addressed would be to add validation to the SettingsForm instead of dynamically disabling the form element but this patch should fix the problem of not being able to enable the setting if it isn't already enabled and multiple container already exist.

  • Status changed to Fixed 7 months ago
  • πŸ‡ΊπŸ‡ΈUnited States japerry KVUO

    At first I was confused why you'd want to do this, but yes you're right -- in case somehow you ended up with this box unchecked, you should be able to check it even if other containers are there.

    Fixed.

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

Production build 0.71.5 2024