Skip to content

Replace piwik with matomo analytic templates #7693

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

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

fnagel
Copy link

@fnagel fnagel commented Nov 28, 2024

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related issues/PRs #7692
License MIT

What's in this PR?

Updated the Matomo templates.

Why?

Because the current ones are outdated.

@alexander-schranz alexander-schranz changed the base branch from 2.6 to 3.0 November 28, 2024 17:24
@alexander-schranz alexander-schranz added the Feature New functionality not yet included in Sulu label Nov 28, 2024
@alexander-schranz alexander-schranz changed the title Fix matomo analytic templates Replace piwik with matomo analytic templates Nov 28, 2024
@alexander-schranz
Copy link
Member

@fnagel thx for the pull request can you add a short line to the UPGRADE.md

Something similar to:

### Piwik replaced with Matomo script

The script provided by Sulu for the piwik implementation has been updated to use mataomo path so the script is now pointing to matomo.php instead of the piwik.php file.

Based on Matomo version 5.1.2.
See sulu#7692
@fnagel fnagel force-pushed the fix-matomo-analytic-templates branch from 5b44b6f to 138da56 Compare November 28, 2024 17:31
@fnagel
Copy link
Author

fnagel commented Nov 28, 2024

@alexander-schranz Changed the UPGRADE.md.

@alexander-schranz alexander-schranz enabled auto-merge (squash) November 28, 2024 17:33
@alexander-schranz
Copy link
Member

@fnagel awesome. Thank you!

@alexander-schranz alexander-schranz merged commit 84a6d7b into sulu:3.0 Nov 28, 2024
9 checks passed
_paq.push(['trackPageView']);
_paq.push(['enableLinkTracking']);
(function() {
var u="{{ analytics.content.url|trim('/') }}";
_paq.push(['setTrackerUrl', u+'/piwik.php']);
_paq.push(['setTrackerUrl', u+'matomo.php']);
Copy link

@benr77 benr77 Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The forward slash has been removed here, which is causing invalid URLs to be constructed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I copied the latest code from the Matomo docs without checking what the u variable will contain. Thanks!

@@ -1 +1 @@
<noscript><p><img src="{{ analytics.content.url }}/piwik.php?idsite={{ analytics.content.siteId }}" style="border:0;" alt="" /></p></noscript>
<noscript><p><img referrerpolicy="no-referrer-when-downgrade" src="{{ analytics.content.url }}/matomo.php?idsite={{ analytics.content.siteId }}&amp;rec=1" style="border:0;" alt="" /></p></noscript>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs the |trim('/') filter to ensure there is not a duplicate forward slash.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. This seems to be an issue even before my changes.

@fnagel fnagel deleted the fix-matomo-analytic-templates branch January 17, 2025 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality not yet included in Sulu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants