Adds ability to send alerts to MicrosoftTeams Channel, #PG-4671 #220
Conversation
james-hill-matomo
left a comment
james-hill-matomo
left a comment
There was a problem hiding this comment.
Code looks good, haven't managed to test it yet. Can we tag in Jeff for that?
| * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later | ||
| * | ||
| */ | ||
|
|
There was a problem hiding this comment.
Nitpick, don't fix. Use strict.
| return array( | ||
| $this->migration->db->addColumn('alert', 'ms_teams_webhook_url', 'VARCHAR(500) NULL', 'slack_channel_id'), | ||
| $this->migration->db->addColumn('alert_triggered', 'ms_teams_webhook_url', 'VARCHAR(500) NULL', 'slack_channel_id'), | ||
| ); |
There was a problem hiding this comment.
I believe code can be migrated before db is updated, for up to an hour. Codex impact assessment below.
I'm not sure what our policy for this stuff is currently - I suspect previously we wouldn't have thought about it, and wouldn't have seen any of the errors.
Maybe a good middle ground is to is to fix Model.php only, so that alerts continue working until the schema is updated. A fix would be to check for existence of the column first.
- Updates/5.2.0.php:42-45 is what adds ms_teams_webhook_url to both alert and alert_triggered. If that migration isn’t run, those columns simply don’t exist.
- Model.php:264-289 and Model.php:349-371 always include ms_teams_webhook_url in the insert/update payload. Without the column, every call to the addAlert/editAlert API fails with an
“Unknown column” SQL error, so no alert can be saved (not just Teams alerts).
- When an alert fires, Model.php:392-417 copies the same field into the alert_triggered insert. With the old schema this insert also errors, so the scheduler can’t persist triggered
alerts and all downstream notifications (email/SMS/Slack) abort for that run.
- Housekeeping code such as CustomAlerts.php:166-183 reads $alert['ms_teams_webhook_url'] while updating alerts. If the column is missing, those arrays never have that key, leading to
PHP notices and preventing clean saves even when editing unrelated fields.
- Because the UI and API contract now expose a ms_teams_webhook_url property (see vue/src/EditAlert/EditAlert.vue:114-118 and API.php:136-254), users will see the Teams options but
any submission will error until the schema is updated.
There was a problem hiding this comment.
I believe this is okay to pass, as we have done similar thing in past and have received very few complaints
There was a problem hiding this comment.
Only add//update will be broken, till the time we run the update, existing alerts will keep working.
There was a problem hiding this comment.
You are right - triggerAlert could potentially insert the wrong column, but it's reading from the db in the first place, so never will.
|
I am merging this in-order to fix test cases. |
Description
Adds ability to send alerts to MicrosoftTeams Channel, #PG-4671
Requires: matomo-org/plugin-MicrosoftTeams#1
Issue No
#PG-4671
Steps to Replicate the Issue
./console scheduled-tasks:run --force "Piwik\Plugins\CustomAlerts\Tasks.runAlertsDaily_{idsite}"Checklist