fix: test alert should open valid link with more info #9896
Conversation
…and added test cases for it
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| funcs := tmpltext.FuncMap{ | ||
| "urlescape": func(value string) string { | ||
| return url.PathEscape(value) | ||
| }, |
There was a problem hiding this comment.
PathEscape used instead of QueryEscape for query parameter
The urlescape function uses url.PathEscape instead of url.QueryEscape for encoding the ruleId query parameter value. PathEscape doesn't encode characters like &, =, and / that have special meaning in query strings. If a ruleId contains these characters (e.g., foo&bar), the generated URL would be malformed (?ruleId=foo&bar instead of ?ruleId=foo%26bar), causing incorrect URL parsing. Since the UUID regex validation was also removed, arbitrary ruleId values are now accepted, increasing the likelihood of this issue occurring.
1 participant
📄 Summary
Updated the link generation logic for test alert notifications. Previously, test alerts for rules currently being created resulted in broken links due to missing Alert IDs (since the alert does not yet exist in the DB).
Key Changes:
Modified the notification link to include a specific query parameter (
isTestAlertindicating a "test" context) for all test alerts.This would allow the Frontend to route the user to a specific preview/test page using the query parameter, bypassing the requirement for a pre-existing Alert ID.
✅ Changes
🏷️ Required: Add Relevant Labels
ex:
frontendbackenddevopsbugenhancementuitest👥 Reviewers
🧪 How to Test
🔍 Related Issues
Closes #
📸 Screenshots / Screen Recording (if applicable / mandatory for UI related changes)
📋 Checklist
👀 Notes for Reviewers
Note
Improves alert link generation, especially for test alerts.
rules/manager.go, setlabels.TestAlertLabelto"true"for test notifications so downstream templates can detect test alertsTestAlertLabeltoutils/labels/labels.goalertmanagertypes/template.go:urlescapetemplate func and use it forruleId__ruleIdPathto: useruleIdif present/non-empty, append&isTestAlert=truewhentestalert=true, otherwise omit the edit pathalertmanagertypes/template_test.gocovering single/multiple alerts, matching/mismatching/blankruleId,testalerthandling, and URL-escapingWritten by Cursor Bugbot for commit cd59742. This will update automatically on new commits. Configure here.