Skip to content

Conversation

@AltamashShaikh
Copy link
Contributor

@AltamashShaikh AltamashShaikh commented Oct 16, 2025

Description

Update CustomAlerts to include Matomo domain with idSite

Issue No

#PG-4676

Steps to Replicate the Issue

  1. Create a new alert and you should see Slack enabled in "Send alerts via" section.
  2. Create a new alert, which can be triggered like unique_visitors < 2
  3. Execute this command to run alerts ./console core:run-scheduled-tasks --force "Piwik\Plugins\CustomAlerts\Tasks.runAlertsDaily_{alertID}
  4. You will see the alert has no link to Matomo and idSite
  5. Checkout this PR and redo step 1-4 and you will see an alert with link to Matomo and idsite.

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [✔] New test case added/updated?
  • [NA] Are all newly added texts included via translation?
  • [NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [✔] Version bumped?

@AltamashShaikh AltamashShaikh added the Needs Review For pull requests that need a code review. label Oct 16, 2025
Comment on lines +471 to +473
if (stripos($settingURL, 'index.php') === false) {
$settingURL .= 'index.php';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I spent way too long thinking about this bit. I think the upstream code could be improved!

I'm not 100% sure getPiwikUrl will always include the trailing / either, but I couldn't figure it out for sure!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@james-hill-matomo We have this code for GA and SEKP plugin and its running fine without any errors so I didn't bother adding / check.

Copy link
Contributor

Choose a reason for hiding this comment

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

We really need a central location for this stuff!

@AltamashShaikh AltamashShaikh merged commit 11d8974 into 5.x-dev Oct 17, 2025
8 checks passed
@AltamashShaikh AltamashShaikh deleted the PG-4676-update-alert-msg branch October 17, 2025 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review For pull requests that need a code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants