We noticed after upgrading several sites from 8.x-1.x to 2.0.x that the user role condition settings for existing Google Tag containers had been effectively reversed. Containers that were previously configured to only include GTM for the anonymous user role were changed to exclude only the anonymous user role after upgrading to 2.0.x and applying DB updates.
This seems to be tied to the way that the 8.x-1.x config settings are mapped to 2.0.x config settings in the GoogleTagUpgradeManager class used by google_tag_update_8201().
The GoogleTagUpgradeManager::convertConditions() method currently maps any 8.x-1.x xxxx_toggle config setting values to 2.0.x negate values as TRUE because the string value of the $negate_toggle variable used to compare to 8.x-1.x values against ('exclude_listed') does not match the value of GOOGLE_TAG_EXCLUDE_LISTED constant in 8.x-1.x ('exclude listed'). See https://git.drupalcode.org/project/google_tag/-/blob/8.x-1.x/google_tag....
Furthermore, enabling one of the "negate" toggles in 2.0.x is functionally equivalent to setting the 8.x-1.x toggles to 'exclude listed' yet the GoogleTagUpgradeManager::convertConditions() method is currently setting the migrated negate values to TRUE when the 8.x-1.x settings do not match the value of GOOGLE_TAG_EXCLUDE_LISTED and it should actually be doing the opposite. It should be setting the values to TRUE only if they do match the 8.x-1.x 'exclude listed' value.
xxxx_toggle configuration settings wereUpdate GoogleTagUpgradeManager::convertConditions() method to map negate values correctly:
xxxx_toggle: 'exclude listed' should be mapped to negate: truexxxx_toggle values should be mapped to negate: false| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 3382757-5.patch | 2.78 KB | joegraduate |
| #5 | 3382757-3-5-interdiff.txt | 728 bytes | joegraduate |
Comments
Comment #2
joegraduateComment #3
joegraduateThe attached patch updates
GoogleTagUpgradeManager::convertConditions()so that 8.x-1.x "xxxx_toggle" config setting values are mapped to 2.0.x negate values as follows:xxxx_toggle: 'include_listed'is mapped tonegate: falsexxxx_toggle: 'exclude_listed'is mapped tonegate: trueComment #4
joegraduateComment #5
joegraduateAfter further testing, I realized that value of the
$negate_togglevariable ('exclude_listed') thatGoogleTagUpgradeManager::convertConditions()uses to compare to values it's migrating does not actually match the value of theGOOGLE_TAG_EXCLUDE_LISTEDconstant used in 8.x-1.x ('exclude listed'). See https://git.drupalcode.org/project/google_tag/-/blob/8.x-1.x/google_tag....This updated patch addresses that problem as well.
Comment #6
joegraduateUpdated issue summary with additional findings from #5.
Comment #7
joegraduateComment #8
joegraduateComment #9
joegraduateComment #11
japerryYah, that looks right. Not really wanting to write upgrade tests, but per slack you said this is working better so I've committed it to head. Thanks for the help!
Comment #12
joegraduateThanks @japerry! Made some minor clarification updates to issue summary.
Comment #13
codebymikey commentedCan this please be tagged and released so that people doing the upgrade to 2.0.x don't unintentionally break their integrations?
Comment #15
bramvandenbulcke commentedThis regression really breaks the proper functioning of the module!
If you have anonymous users checked under user role and this condition is reversed after the 2.x update, then you are basically tracking no users at all.
I'm using config management and I didn't see it.
Comment #16
jhuebsch commentedI agree with codebymikey and bramvandenbulcke, this is a regression and will cause data collection loss for those who upgrade.
The fix has been in dev for 5 months, can we get a new release?