-
-
Notifications
You must be signed in to change notification settings - Fork 150
update: add tags, created to alerts #1389
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
update: add tags, created to alerts #1389
Conversation
similar to dashboards, server supports adding tags while creating alerts list alerts by tags by providing query param tags=<list of tags> list_tags endpoint provides list of unique tags from all alerts
WalkthroughThe changes introduce alert tagging functionality throughout the alert management system. Alerts can now have optional tags, which are stored, filtered, and listed via new and updated methods. The HTTP API is extended to support tag-based alert filtering and to provide an endpoint for listing all tags. Supporting changes include error handling for invalid query parameters and updates to alert summaries. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP_Handler as HTTP Handler
participant Alerts
participant DB as Database
Client->>HTTP_Handler: GET /alerts?tags=foo,bar
HTTP_Handler->>Alerts: list_alerts_for_user(session, tags=["foo", "bar"])
Alerts->>DB: Query alerts for user with tags filter
DB-->>Alerts: Matching alerts
Alerts-->>HTTP_Handler: Filtered alerts
HTTP_Handler-->>Client: JSON summaries of alerts
Client->>HTTP_Handler: GET /alerts/list_tags
HTTP_Handler->>Alerts: list_tags()
Alerts->>DB: Query all alerts
DB-->>Alerts: All alerts
Alerts-->>HTTP_Handler: Unique, sorted tag list
HTTP_Handler-->>Client: JSON tag list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/handlers/http/modal/server.rs (1)
272-278
: Consider using a more appropriate authorization action.The new route implementation follows existing patterns correctly. However, using
Action::ListDashboard
for authorization seems semantically incorrect for listing alert tags. Consider usingAction::GetAlert
or a similar alert-related action for better semantic alignment.web::resource("/list_tags").route( web::get() .to(alerts::list_tags) - .authorize(Action::ListDashboard), + .authorize(Action::GetAlert), ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/alerts/mod.rs
(12 hunks)src/handlers/http/alerts.rs
(3 hunks)src/handlers/http/modal/server.rs
(1 hunks)src/prism/home/mod.rs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: coverage
🔇 Additional comments (19)
src/alerts/mod.rs (14)
22-22
: LGTM!The additional chrono imports are necessary for the new
created
timestamp functionality.
200-207
: LGTM!The Display trait implementation for AlertType is correct and follows standard Rust patterns.
539-539
: LGTM!The optional tags field is appropriately typed as
Option<Vec<String>>
and follows the existing pattern for optional fields.
560-560
: LGTM!The tags field is correctly transferred from the request to the config during conversion.
583-583
: LGTM!The tags field is consistently added to AlertConfig with the same optional type.
559-559
: LGTM!Setting the created timestamp to the current UTC time during alert creation is appropriate.
582-582
: LGTM!The created field with DateTime type is appropriate for tracking alert creation time.
613-614
: LGTM!Setting created timestamp and tags to appropriate defaults during migration is correct.
1202-1239
: LGTM!The to_summary method implementation is well-structured and handles optional fields correctly. The JSON map creation follows appropriate patterns for alert summaries.
1278-1279
: LGTM!The InvalidQueryParameter error variant is appropriately added for handling query parameter validation errors.
1302-1302
: LGTM!The HTTP status code mapping to BAD_REQUEST is correct for invalid query parameter errors.
1410-1410
: LGTM!The tags parameter addition to list_alerts_for_user method signature is appropriate for tag-based filtering.
1419-1429
: LGTM!The tag filtering logic is correctly implemented. It properly handles the empty tags case and filters alerts based on tag intersection.
1529-1541
: LGTM!The list_tags method implementation is well-designed. It correctly handles optional tags, flattens the structure, and ensures uniqueness through sorting and deduplication.
src/prism/home/mod.rs (1)
359-359
: LGTM!The addition of an empty tags vector correctly aligns with the updated
list_alerts_for_user
method signature. Since no tag filtering is needed in this context, passing an empty vector is appropriate.src/handlers/http/alerts.rs (4)
19-19
: LGTM!The HashMap import is necessary for the new query parameter parsing functionality.
41-55
: LGTM!The query parameter parsing logic is well-implemented. It properly handles tag splitting, trimming, validation, and error cases. The validation to prevent empty tags after filtering is a good defensive programming practice.
57-62
: LGTM!The updated list function correctly passes the parsed tags and uses the new to_summary method for consistent alert summary serialization.
177-180
: LGTM!The list_tags handler function is simple, effective, and follows the established patterns in the codebase.
Summary by CodeRabbit
New Features
Bug Fixes
Other Improvements