Skip to content

Conversation

@AltamashShaikh
Copy link
Contributor

Description

Adds changes to send CustomAlerts via Slack

Issue No

#PG-4530

Steps to Replicate the Issue

  1. Requires Changes to enable send alerts via Slack, #PG-4530 plugin-CustomAlerts#209
  2. Create a new alert and you should see Slack enabled in "Send alerts via" section.
  3. Create a new alert, which can be triggered like unique_visitors < 2
  4. Execute this command to run alerts ./console core:run-scheduled-tasks --force "Piwik\Plugins\CustomAlerts\Tasks.runAlertsDaily_{alertID}

Checklist

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

@AltamashShaikh AltamashShaikh added the Needs Review For pull requests that need a code review. label Sep 22, 2025
@AltamashShaikh AltamashShaikh requested a review from a team September 22, 2025 03:55
snake14
snake14 previously approved these changes Sep 22, 2025
Copy link
Contributor

@snake14 snake14 left a comment

Choose a reason for hiding this comment

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

Looks good and posted to the Slack channel as expected 👍

I had a couple nitpick/optional comments. My only other thoughts were:

  1. It's a bit screenshot heavy. Could any of those screenshots use DOM instead of image comparison?
  2. It would be good to document the methods with comment blocks, especially the public ones.

Copy link
Contributor

@snake14 snake14 left a comment

Choose a reason for hiding this comment

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

Thank you for the changes. Adding the comment blocks did highlight how many method parameters aren't typed, but that's probably out of scope.

@AltamashShaikh AltamashShaikh merged commit 6a6602c into 5.x-dev Sep 23, 2025
8 checks passed
@AltamashShaikh AltamashShaikh deleted the PG-4530-alert-via-slack branch September 23, 2025 05:03
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