fix: allow multiple tags in dashboards list_by_tag API #1374
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe changes remove the single-tag dashboard listing route and handler, replacing them with an enhanced dashboard listing handler that supports filtering by multiple tags via a query parameter. The underlying method was renamed and updated to accept a vector of tags, returning dashboards matching any of the provided tags. Changes
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/users/dashboards.rs (1)
372-376: Consider optimizing tag lookup performance for large datasets.The filtering logic correctly implements OR semantics (matching any tag), but the current
Vec::contains()operation is O(n) for each tag comparison. For better performance with many tags, consider using aHashSetfor the input tags.-pub async fn list_dashboards_by_tags(&self, tags: Vec<String>) -> Vec<Dashboard> { +pub async fn list_dashboards_by_tags(&self, tags: Vec<String>) -> Vec<Dashboard> { + let tag_set: std::collections::HashSet<&String> = tags.iter().collect(); let dashboards = self.0.read().await; dashboards .iter() .filter(|d| { if let Some(dashboard_tags) = &d.tags { - !tags.is_empty() && dashboard_tags.iter().any(|tag| tags.contains(tag)) + !tag_set.is_empty() && dashboard_tags.iter().any(|tag| tag_set.contains(tag)) } else { false } }) .cloned() .collect() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/handlers/http/modal/server.rs(1 hunks)src/handlers/http/users/dashboards.rs(1 hunks)src/users/dashboards.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:63-71
Timestamp: 2025-05-01T10:22:45.695Z
Learning: Dashboard design in Parseable follows a sharing model where any user can read (list/get) any dashboard, enabling dashboard sharing functionality, but only the dashboard author can modify (update/delete) their own dashboards.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/users/dashboards.rs:154-165
Timestamp: 2025-05-01T12:22:42.363Z
Learning: Title validation for dashboards is performed in the `create_dashboard` HTTP handler function rather than in the `DASHBOARDS.create` method, avoiding redundant validation.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.
src/users/dashboards.rs (1)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.
src/handlers/http/modal/server.rs (2)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/users/dashboards.rs:154-165
Timestamp: 2025-05-01T12:22:42.363Z
Learning: Title validation for dashboards is performed in the `create_dashboard` HTTP handler function rather than in the `DASHBOARDS.create` method, avoiding redundant validation.
src/handlers/http/users/dashboards.rs (2)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/users/dashboards.rs:154-165
Timestamp: 2025-05-01T12:22:42.363Z
Learning: Title validation for dashboards is performed in the `create_dashboard` HTTP handler function rather than in the `DASHBOARDS.create` method, avoiding redundant validation.
🧬 Code Graph Analysis (2)
src/users/dashboards.rs (1)
src/handlers/http/users/dashboards.rs (4)
list_dashboards_by_tags(206-227)tags(212-216)dashboards(37-40)dashboards(221-224)
src/handlers/http/users/dashboards.rs (1)
src/users/dashboards.rs (2)
list_dashboards_by_tags(367-380)dashboards(355-359)
⏰ 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: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: coverage
🔇 Additional comments (3)
src/users/dashboards.rs (1)
367-367: Good: Function name accurately reflects the new multi-tag functionality.The renaming from
list_dashboards_by_tagtolist_dashboards_by_tags(singular to plural) clearly indicates the API now supports multiple tags.src/handlers/http/modal/server.rs (1)
311-315: LGTM: Route definition correctly updated for multi-tag support.The route parameter name change from
{tag}to{tags}and handler function update tolist_dashboards_by_tagsare consistent with the API evolution. The authorization remains appropriately set toListDashboard.src/handlers/http/users/dashboards.rs (1)
206-220: Well-implemented tag parsing with thorough validation.The handler correctly processes comma-separated tags by:
- Splitting on commas
- Trimming whitespace from each tag
- Filtering out empty strings
- Validating the result is not empty
The dual validation (before and after processing) ensures robust error handling for edge cases like input containing only commas or whitespace.
nikhilsinhaparseable
force-pushed
the
list-by-tags
branch
from
f9e18a7 to
e30f9dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/handlers/http/users/dashboards.rs (1)
48-63: LGTM! Clean implementation of multi-tag filtering.The tag processing logic is well-implemented with proper validation and error handling. The early return pattern keeps the code readable and maintains the existing function structure.
Consider improving the error message to be more descriptive:
- return Err(DashboardError::Metadata("Tags cannot be empty")); + return Err(DashboardError::Metadata("No valid tags provided after processing"));This would better reflect that the error occurs when all tags are empty after trimming, not when the parameter itself is empty.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/handlers/http/modal/server.rs(0 hunks)src/handlers/http/users/dashboards.rs(1 hunks)src/users/dashboards.rs(1 hunks)
💤 Files with no reviewable changes (1)
- src/handlers/http/modal/server.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/users/dashboards.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:63-71
Timestamp: 2025-05-01T10:22:45.695Z
Learning: Dashboard design in Parseable follows a sharing model where any user can read (list/get) any dashboard, enabling dashboard sharing functionality, but only the dashboard author can modify (update/delete) their own dashboards.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/users/dashboards.rs:154-165
Timestamp: 2025-05-01T12:22:42.363Z
Learning: Title validation for dashboards is performed in the `create_dashboard` HTTP handler function rather than in the `DASHBOARDS.create` method, avoiding redundant validation.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.
src/handlers/http/users/dashboards.rs (2)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1305
File: src/users/dashboards.rs:154-165
Timestamp: 2025-05-01T12:22:42.363Z
Learning: Title validation for dashboards is performed in the `create_dashboard` HTTP handler function rather than in the `DASHBOARDS.create` method, avoiding redundant validation.
🧬 Code Graph Analysis (1)
src/handlers/http/users/dashboards.rs (1)
src/users/dashboards.rs (2)
dashboards(351-355)dashboards(367-371)
⏰ 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 aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: coverage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes