🇺🇸United States @solotandem

I appreciate that missing snippets is a problem. However, the claim that the module fails to do as intended has been made several times (a search of closed issues confirms this) and in each case the problem was NOT related to the code in this project, but to other causes like deployment steps or lack thereof. The most recent duplicate issue is 🐛 Google Tag 1.7 breaks analytics Needs work . I would draw your attention to comment #5.

In my testing of the module upgrade to release 1.7, there has never been a failure to insert snippets on the page after the upgrade. My testing is done in a simple environment which tests the code in the module. Deployment steps that might cause a problem are outside the scope of this code. From your 'Steps to Reproduce', my testing indicates that all that needs to be done is #1 and #4 would confirm the presence of snippets. Again, my testing is done on a plain vanilla site without interference from deployment steps.

If you can demonstrate this code is the cause of missing snippets, then please elaborate.

The related issue mentioned in #2 has a further related issue 🐛 Adding or editing a block through the UI saves the entity twice Fixed which is marked fixed. Is that fix in the core release you are using?

The community bug reporting guidelines indicate the burden of showing cause and effect is on you. So far you have reported an undesirable outcome, but have not indicated the cause. That makes this a support request.

Have you confirmed that the snippet insertion conditions render true so that a snippet is placed on the page? (You can turn on the debug output in this module to see which conditions return true and false.) Is the google tag js on the page but analytics are not being recorded? If you can provide some details like these, then maybe we can figure out the cause.

Also, needs work only applies if there is a patch or merge request in play.

Done.

More ad hominem.

yes, you did provide a detailed description of what you observed (along with harsh and false accusations) and, by deduction, missed. from that i replied with items intended to help you get the desired outcome.

no, the tone of my response was not dismissive, but simply factual. in the first paragraph, i refute your claim. in the second, i justify the issue status change (probably what you find as dismissive). in the third paragraph, i offer a possible reason for the undesirable outcome (hinting at a solution).

taking a step backward, this is really two issues: update hook and snippet insertion, with the second one being the persistent problem that you need support for. the second issue is clear from the direction change you took after managing to get the update hook to stick.

from what you wrote the update hook did run and was successful. but then something in your deployment workflow (e.g. config override) reverted the changes from the update hook. (the latter you did not notice as you have not documented it and would have omitted it from the issue if you had.) this is evident from your description of steps to revert the module update value to 8103 and rerun the hook, all using drush. as the update hook succeeded here so it did on the initial run of the update hook (as i attest to in first paragraph of my previous comment).

then we get to the problem of container visibility and snippet insertion. in its ContainerAccessControlHandler class, the tag module overrides the core processing of condition plugins. if it did not, then most likely no one would have snippets on a page. it changes the incorrect core behavior of returning false if any of the plugins does not have context. given that block visibility is the only use case of condition plugins in core, this logic, while incorrect, is benign because the default is to deny the block visibility and so not display it. with block placement you need to affirmatively set a condition to display a block, so the default visibility condition being false is acceptable.

with the tag module the default visibility should be true unless you declare a reason to hide the snippets. the condition plugins provided by the tag nodule (that override their core counterparts) do just that; their default out-of-the-box value returns true (for non-admin paths). condition plugins that have no context should not be present to be evaluated and certainly should not render the final verdict.

apparently this is not the case with the rules plugins, some of which will have context on the pages you are wanting to gather analytics.

after noticing that the update hook was overridden (that is not what you noticed but that is what happened), why is it you did not immediately notice the snippets not being added to the page? how is it you dare to blame me for this? rather are you not negligent in this regard?

if you would read the code in this project, you might notice that it implements core hook_plugin_filter_TYPE_alter, to remove some of the condition plugins defined by core and a contributed module (as they do not behave well). this hook is available to you to filter the condition plugins defined by rules. until core properly implements condition plugins, everyone probably needs to implement this hook.

lastly, is it 'particularly constructive or in the spirit of the Drupal community' for you to lash out at me with ad hominem remarks:

• loudly (with an issue status of critical) accusing me of something you can not prove (in disregard of community guidelines)? as i have written elsewhere, if you are not able to provide the necessary evidence, then do not tag an issue as a bug (and certainly not critical).
• and, without justification, blaming me for you having 'lost a month of business-critical analytics data'? rather, are you not negligent for not noticing this much sooner?

I cannot reproduce what you claim. After running update 8104 on a very plain drupal site all is well. My guess is the undesirable outcome you describe has something to do with your site configuration or deployment procedures. The former may include other modules whose code interferes in some way.

While you may have a concern, the issue summary is lacking and fails to comply with the community bug reporting guidelines. You have indicated an undesirable response but not uncovered (or disclosed) the actual cause (in particular you have not shown the response to be the result of the code in this project). If you can please update the summary. Until then let's change the issue type to support request.

@coaston On what do you base your claim?

The same items are shown during automated testing on a patch. The items noted are all false positives.

Disagreement about text in a README does not constitute a bug. Snippet terminology should be clear from general usage.

Category and status changes long overdue.

Issue summary does not satisfy bug reporting guidelines and text suggests a site configuration error that has nothing to do with this module.

solotandem → created an issue.

solotandem → created an issue.

It would seem the upgrade status module is reporting false positives. This module defines the 'field' permissions in a permission callback. Have you inspected the code in this module? Have you tested the field permissions on a drupal 10.x site? In my experience they work just fine.

While you may have a concern, the issue summary is lacking and fails to comply with community bug reporting guidelines. If you can demonstrate a bug due to the code in this project, then please update the issue summary. Until then an issue type of support request seems more appropriate.

solotandem → created an issue.

This request seems quite problematic given the only JS file in the project relies on core drupalSetSummary function which, even in branch 10.3.x, relies on jQuery.

Thanks to everyone with input.

Commit follows the merge request with some minor changes. The primary visual difference is adding the hostname to the advanced tab instead of the general tab.

As mentioned early on this is a bug in the core condition plugin code. This may have been resolved by the commit last October for issue 2815829. Either update to a core release with that commit or patch an earlier core release. Then see if this issue is resolved. Regardless, the fix needs to be in core not this module (as has been consistently indicated on numerous duplicate issues over the years).

This module is relying on the API of the path_alias module which says AliasManagerInterface::getAliasByPath() returns string. If you are getting a NULL then I would suggest you find out why and fix that. What you are suggesting is that module A can not rely on the published API of module B but is required to type check every value returned from a method in module B?

Change status due to lack of response.

Change status due to lack of response.

Thanks for the suggestion and sorry about the extensive delay to complete.

Status change long overdue/

I can not agree with you more that "conditions that don't apply shouldn't kill the page". But that is a core design failure and a core issue.

What happened to conditions from contributed modules in 7x drupal? Conditions should be able to be added one at a time as relevant, able to be grouped, and use operators (OR, AND) instead of always being present because a module is installed that defines one, and always using an AND operator. Pitiful.

If your situation is related to the negate toggle on most condition plugins, did the core commit on issue 2815829 help?

IMHO, the claims in the issue summary are false. Instead, the 1.x branch:

• does support 10.2.x
• does NOT require a patch to run in 10.2.x
• the 'appropriate changes suggested by upgrade_status' are all false positives.

The patch provided on this issue is not needed:

• accessCheck() does not apply to query run on a config entity
• call to file_url_transform_relative() only runs if file_url_generator service is not present (e.g. drupal 8)
• defaultTheme property is set

If you can demonstrate to the contrary, please do so.

@Anybody Yes I will.
Thanks to all.

See related issue 3105221 and commit to 2.x branch on 2020-01-09.

try checking a content type (or all of them)

The console errors are not related to this module. Have you confirmed the snippet directory is writable by the website user? And that it contains the snippet files? You mention the add container form is empty. What about the module settings form?

You have not provided any reason to 'requalify' this issue as a bug. So far you have indicated that something undesirable occurs, but have yet to demonstrate a connection to this module. Are you knowledgeable enough to debug the code?

Thanks for mentioning.

@calebtr the feature you refer to (check to see if the file exists on each page request) was added to 8x branch but not as yet back ported to 7x. No need for you to work on that.

Most people have issue with the container form being empty after a cache rebuild or site deploy. You indicate this error on the add container workflow which seems highly unusual. Do you have any existing containers? If so what happens if you try to edit one?

Review the module settings at admin/config/system/google-tag/settings. Do you have checked Recreate snippets on cache rebuild? If so and you use drush to rebuild cache, does the web site user have ability to overwrite files written by the drush user? The GA module has nothing to do with this.

The console error you pasted Uncaught TypeError: n.toolbar is undefined has nothing to do with this module. Can you investigate that?

Thanks for the suggestion.

This patch file → is the intended change. Whether it "applies" to the core release you are running I do not know (patch is 3 years old).

Other things you might try are reviewing the snippet insertion conditions and enabling debug output from the settings form for this module. If the problem is not the core code, the debug output should give some helpful information.

What code (core, this module) are you running that generated this 'problem'? The code in 8.x-1.6 release of this module checks for the 'file_url_generator' service available in drupal 10. if present it uses that, otherwise it calls file_create_url() which exists prior to 10. How can the 'problem' you mention exist if you are using compatible code?

your suggestion makes sense.
what paths do you have that fall into the admin* category but not admin/*?

try clearing the caches. this issue has been raised before and been determined to be a problem with deployment procedures not this module. thanks for using it.