-
Notifications
You must be signed in to change notification settings - Fork 0
Adds support for Matomo Tag Manager containers #282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Pull Request Test Coverage Report for Build 19515133385Details
💛 - Coveralls |
d76cc3b to
e592e6e
Compare
** Why are these changes being introduced: One of the instances of this application, GeoData, has an existing integration with Matomo using their legacy approach that relies on a site ID. The USE UI project will be using a newer integration, which requires us to support Matomo Tag Manager. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/use-160 ** How does this address that need: This adds a second option for integration with Matomo, using just a single ENV variable - for the container URL. We do this by extending the conditional in the _head partial, allowing the application to support _either_ a tag manager container _or_ the legacy approach, but not both. Once GeoData gets converted over to using a tag manager container, we will be able to drop that half of this conditional. ** Document any side effects to this change: The legacy integration has a bit of bespoke javascript in order to let turbo frames interactions be sent to Matomo. Thanks to whomever left the URL to the github issue about this approach, which led me to https://developer.matomo.org/guides/spa-tracking - which tells me that we don't need to worry about this under a tag manager context. As a result, I've left that code out of the new integration, and we'll handle tracking all changes directly within the Matomo container.
e592e6e to
28511bf
Compare
|
Please note: The existing GeoData integration via the legacy matomo approach should still be operational under this change, but I haven't looked at that specifically - I'm relying on an understanding of how if / elsif / end works in Ruby. |
jazairi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be working as expected. Thanks for setting this up! I assume we overrode the theme gem here because of the Turbo bits, but that made me wonder if we should include this container code in the theme gem. I don't think that change would be in scope of this ticket, though, so from my perspective this is good to go. ![]()
|
I think the plan is to move a lot of these changes up to the theme gem, yeah. Support for both Matomo configurations seems like a natural to move up to the theme gem, even if the other customizations here don't move up. I could see there being a Matomo-specific partial that gets called from |
One of the instances of this application, GeoData, has an existing integration with Matomo using their legacy approach that relies on a site ID.
The USE UI project will be using a newer integration, which requires us to support Matomo Tag Manager.
Ticket:
https://mitlibraries.atlassian.net/browse/use-160
How does this address that need:
This adds a second option for integration with Matomo, using just a single ENV variable - for the container URL. We do this by extending the conditional in the _head partial, allowing the application to support either a tag manager container or the legacy approach, but not both.
Once GeoData gets converted over to using a tag manager container, we will be able to drop that half of this conditional.
Document any side effects to this change:
The legacy integration has a bit of bespoke javascript in order to let turbo frames interactions be sent to Matomo. Thanks to whomever left the URL to the github issue about this approach, which led me to https://developer.matomo.org/guides/spa-tracking - which tells me that we don't need to worry about this under a tag manager context. As a result, I've left that code out of the new integration, and we'll handle tracking all changes directly within the Matomo container.
Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
E.g., if the PR includes updated dependencies and/or data
migration, or how to confirm the feature is working.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing