dashboard changes #1362
dashboard changes #1362
Conversation
|
""" WalkthroughTwo new GET endpoints Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Dashboards
Client->>Server: GET /dashboards/list_tags (with auth)
Server->>Dashboards: list_tags()
Dashboards-->>Server: Sorted, deduplicated tag list
Server-->>Client: 200 OK + JSON(tag list)
sequenceDiagram
participant Client
participant Server
participant Dashboards
Client->>Server: GET /dashboards/list_by_tag/{tag} (with auth)
Server->>Dashboards: list_dashboards_by_tag(tag)
Dashboards-->>Server: Filtered dashboard summaries
Server-->>Client: 200 OK + JSON(dashboard summaries)
Suggested reviewers
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 (2)
src/users/dashboards.rs (2)
85-88: Remove redundant comment.The comment on line 87 duplicates the information already provided on line 85.
// if is_favorite is None, set it to false, else set it to the current value self.is_favorite = Some( - self.is_favorite.unwrap_or(false), // default to false if not set + self.is_favorite.unwrap_or(false), );
336-348: Consider future optimization for tag listing.While the implementation is correct and consistent with the current in-memory architecture, loading all dashboards to extract tags may become inefficient as the system scales. Consider maintaining a separate index of tags in the future for better performance.
📜 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(9 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#1348
File: src/prism/home/mod.rs:366-368
Timestamp: 2025-06-15T18:18:14.590Z
Learning: In the Parseable dashboard system, dashboard_id is guaranteed to always be present (never None) because IDs are generated at dashboard creation time, and there are no pre-v1 dashboards that could lack IDs.
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.
src/handlers/http/modal/server.rs (3)
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.
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.
src/users/dashboards.rs (4)
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/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#1348
File: src/prism/home/mod.rs:366-368
Timestamp: 2025-06-15T18:18:14.590Z
Learning: In the Parseable dashboard system, dashboard_id is guaranteed to always be present (never None) because IDs are generated at dashboard creation time, and there are no pre-v1 dashboards that could lack IDs.
🧬 Code Graph Analysis (1)
src/handlers/http/users/dashboards.rs (1)
src/users/dashboards.rs (1)
list_tags(338-348)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
🔇 Additional comments (5)
src/users/dashboards.rs (3)
211-219: LGTM! Title uniqueness check is well implemented.The implementation correctly enforces title uniqueness while properly handling updates by excluding the current dashboard from the duplicate check.
239-239: Excellent timestamp handling!The implementation correctly sets the
createdtimestamp during creation and preserves it during updates, maintaining proper audit trail integrity.Also applies to: 257-262
55-55: Verify API consumers & docs for camelCase serialization changeAdding
#[serde(rename_all = "camelCase")]on theDashboardstruct insrc/users/dashboards.rschanges all JSON fields from snake_case to camelCase (e.g.dashboard_id→dashboardId,is_favorite→isFavorite). We didn’t find any in-repo tests or JSON fixtures expecting the old names, but downstream clients, SDKs, OpenAPI specs or README examples may still rely on snake_case. Please ensure:
src/users/dashboards.rs– theDashboardstruct now uses camelCase.- Any OpenAPI/Swagger definitions or generated clients are updated to use camelCase property names.
- Frontend/mobile apps and SDKs that consume the dashboard API handle
dashboardId,isFavorite, etc.- README.md examples and JSON test fixtures reflect the new naming.
- New tests are added (or existing ones updated) to assert camelCase serialization.
src/handlers/http/users/dashboards.rs (1)
148-151: LGTM! Clean handler implementation.The handler correctly delegates to the business logic layer and returns the appropriate response.
src/handlers/http/modal/server.rs (1)
282-288: Route properly configured with appropriate authorization.The
/list_tagsendpoint is correctly added withAction::ListDashboardauthorization, which aligns with the dashboard sharing model where any user can read dashboard information.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/handlers/http/users/dashboards.rs (1)
97-123: Consider optimizing the dashboard retrieval pattern.The current implementation retrieves the existing dashboard even when performing full updates via request body. This could be optimized by only fetching the existing dashboard when query parameters are used for partial updates.
Additionally, consider adding validation to ensure the
rename_toparameter respects the same uniqueness constraints as the title validation in the create function.- let mut existing_dashboard = DASHBOARDS - .get_dashboard_by_user(dashboard_id, &user_id) - .await - .ok_or(DashboardError::Metadata( - "Dashboard does not exist or user is not authorized", - ))?; - - let query_map = web::Query::<HashMap<String, String>>::from_query(req.query_string()) - .map_err(|_| DashboardError::InvalidQueryParameter)?; - - // Validate: either query params OR body, not both - let has_query_params = !query_map.is_empty(); - let has_body_update = dashboard.title != existing_dashboard.title || dashboard.tiles.is_some(); + let query_map = web::Query::<HashMap<String, String>>::from_query(req.query_string()) + .map_err(|_| DashboardError::InvalidQueryParameter)?; + + let has_query_params = !query_map.is_empty(); + + // Only fetch existing dashboard if we need it for partial updates + let existing_dashboard = if has_query_params { + Some(DASHBOARDS + .get_dashboard_by_user(dashboard_id, &user_id) + .await + .ok_or(DashboardError::Metadata( + "Dashboard does not exist or user is not authorized", + ))?) + } else { + None + }; + + // Validate: either query params OR body, not both + let has_body_update = has_query_params || dashboard.title.is_empty() || dashboard.tiles.is_some();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/handlers/http/users/dashboards.rs(5 hunks)src/users/dashboards.rs(9 hunks)
🚧 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#1348
File: src/prism/home/mod.rs:366-368
Timestamp: 2025-06-15T18:18:14.590Z
Learning: In the Parseable dashboard system, dashboard_id is guaranteed to always be present (never None) because IDs are generated at dashboard creation time, and there are no pre-v1 dashboards that could lack IDs.
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 (3)
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.
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.
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Build Default 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: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: coverage
🔇 Additional comments (6)
src/handlers/http/users/dashboards.rs (6)
19-19: LGTM!The HashMap import is necessary for the new query parameter handling functionality.
77-82: Authorization approach follows established patterns.The use of
get_dashboard_by_userfor authorization checks aligns with the established pattern for dashboard operations, ensuring only the dashboard author can modify their dashboards.
84-95: Robust validation prevents conflicting update approaches.The logic to prevent simultaneous use of query parameters and request body for updates is well-implemented and prevents potential conflicts or confusion in the API behavior.
124-140: Excellent tile validation logic.The validation for tile IDs (checking for nil values and uniqueness) is comprehensive and prevents common data integrity issues. The use of HashSet for uniqueness checking is efficient.
196-199: Simple and effective tag listing implementation.The function correctly delegates to the DASHBOARDS service and returns the expected JSON response. Based on the dashboard sharing model, this endpoint should be accessible to all users for reading dashboard metadata.
215-216: Appropriate error handling for query parameter validation.The new
InvalidQueryParametererror variant with BAD_REQUEST status code correctly handles malformed query parameters and provides clear error responses to clients.Also applies to: 228-228
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/users/dashboards.rs(9 hunks)
🧰 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#1348
File: src/prism/home/mod.rs:366-368
Timestamp: 2025-06-15T18:18:14.590Z
Learning: In the Parseable dashboard system, dashboard_id is guaranteed to always be present (never None) because IDs are generated at dashboard creation time, and there are no pre-v1 dashboards that could lack IDs.
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 (4)
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/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#1348
File: src/prism/home/mod.rs:366-368
Timestamp: 2025-06-15T18:18:14.590Z
Learning: In the Parseable dashboard system, dashboard_id is guaranteed to always be present (never None) because IDs are generated at dashboard creation time, and there are no pre-v1 dashboards that could lack IDs.
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: coverage
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (9)
src/users/dashboards.rs (9)
55-55: LGTM: CamelCase serialization added for API consistency.The camelCase serialization aligns with JavaScript/JSON naming conventions commonly used in web APIs.
61-64: LGTM: New metadata fields enhance dashboard functionality.The new fields
created,tags, andis_favoriteprovide valuable metadata tracking capabilities. The optional nature maintains backward compatibility with existing dashboards.
85-86: LGTM: Default value initialization for is_favorite field.The logic correctly initializes
is_favoriteto false when unset, matching the documented default behavior.
106-111: LGTM: Summary method updated to include new metadata fields.The new fields are properly included in the dashboard summary with appropriate camelCase naming for API consistency.
Also applies to: 127-141
122-122: LGTM: Field name updated to camelCase for consistency.The
dashboardIdnaming matches the camelCase serialization pattern applied to the struct.
237-237: LGTM: Created timestamp set during dashboard creation.The
createdfield is properly initialized to the current time when creating new dashboards.
255-257: LGTM: Improved ownership verification with timestamp preservation.The refactored approach efficiently combines ownership verification with retrieving the existing dashboard, allowing the
createdtimestamp to be preserved during updates.Also applies to: 260-260
334-346: LGTM: Efficient tag aggregation implementation.The
list_tagsmethod correctly:
- Flattens tags from all dashboards
- Removes duplicates with
dedup()- Returns a sorted list
- Handles the case where dashboards may not have tags
The implementation is efficient and follows Rust best practices.
356-356: LGTM: Enhanced ownership verification with improved error messaging.The method now returns the
Dashboardinstead of just success/failure, enabling efficient data reuse. The error message is more descriptive and user-friendly compared to the previousUnauthorizedvariant.This change aligns with the usage pattern in the
updatemethod where the existing dashboard data is needed.Also applies to: 359-363
nikhilsinhaparseable
force-pushed
the
update-dashboards
branch
from
f6e625f to
3fef31e
Compare
1. add below fields to dashboard creation -
a. tags - list of strings
b. created - created datetime
c. is favorite - true/false, default false
2. ensure title is unique
3. add API to get all tags - `GET /api/v1/dashboards/list_tags`
1. is_favorite=true/false -- to set dashboard to favorite 2. rename_to=<updated title> -- to update the title of the dashboard 3. tags=<comma separated tags> -- to update tags add validation - body and query params both cannot co-exist in PUT request
nikhilsinhaparseable
force-pushed
the
update-dashboards
branch
from
ab2a9d8 to
155e18e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/handlers/http/oidc.rs (2)
80-86: Consider if this formatting change is necessary.This is a purely cosmetic change that converts a single-line return statement to a multi-line block. While it may improve readability, it doesn't add functional value and could be considered unnecessary churn in a PR focused on dashboard functionality.
122-127: Consider if this formatting change is necessary.Similar to the previous change, this is purely cosmetic formatting that doesn't add functional value to a PR focused on dashboard enhancements.
src/users/dashboards.rs (1)
351-363: Consider performance implications of tag collection.The
list_tagsmethod usesflat_map,collect,sort, anddedupoperations which create intermediate collections. For large numbers of dashboards, this could be optimized.Consider using a more efficient approach:
pub async fn list_tags(&self) -> Vec<String> { let dashboards = self.0.read().await; - let mut tags = dashboards - .iter() - .filter_map(|d| d.tags.as_ref()) - .flat_map(|t| t.iter().cloned()) - .collect::<Vec<String>>(); - tags.sort(); - tags.dedup(); - tags + let mut tags: std::collections::BTreeSet<String> = dashboards + .iter() + .filter_map(|d| d.tags.as_ref()) + .flat_map(|t| t.iter().cloned()) + .collect(); + tags.into_iter().collect() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/cli.rs(1 hunks)src/handlers/http/modal/server.rs(1 hunks)src/handlers/http/oidc.rs(2 hunks)src/handlers/http/users/dashboards.rs(5 hunks)src/users/dashboards.rs(9 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/cli.rs
- src/handlers/http/modal/server.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/handlers/http/users/dashboards.rs
🧰 Additional context used
🧠 Learnings (3)
📓 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#1348
File: src/prism/home/mod.rs:366-368
Timestamp: 2025-06-15T18:18:14.590Z
Learning: In the Parseable dashboard system, dashboard_id is guaranteed to always be present (never None) because IDs are generated at dashboard creation time, and there are no pre-v1 dashboards that could lack IDs.
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/oidc.rs (1)
Learnt from: nikhilsinhaparseable
PR: parseablehq/parseable#1340
File: src/query/mod.rs:64-66
Timestamp: 2025-06-18T06:39:04.775Z
Learning: In src/query/mod.rs, QUERY_SESSION_STATE and QUERY_SESSION serve different architectural purposes: QUERY_SESSION_STATE is used for stats calculation and allows dynamic registration of individual parquet files from the staging path (files created every minute), while QUERY_SESSION is used for object store queries with the global schema provider. Session contexts with schema providers don't support registering individual tables/parquets, so both session objects are necessary for their respective use cases.
src/users/dashboards.rs (4)
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/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#1348
File: src/prism/home/mod.rs:366-368
Timestamp: 2025-06-15T18:18:14.590Z
Learning: In the Parseable dashboard system, dashboard_id is guaranteed to always be present (never None) because IDs are generated at dashboard creation time, and there are no pre-v1 dashboards that could lack IDs.
⏰ 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-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
🔇 Additional comments (7)
src/users/dashboards.rs (7)
61-64: New dashboard metadata fields look well-designed.The addition of
created,tags, andis_favoritefields enhances dashboard functionality appropriately. The optional nature allows backward compatibility.
85-86: Default value initialization is correct.Setting
is_favoritetofalseby default whenNoneis appropriate and consistent with typical boolean field handling.
227-243: Title uniqueness validation is now properly synchronized.The implementation correctly addresses the race condition mentioned in previous reviews by performing the uniqueness check under the same write lock used for insertion. This ensures atomicity and prevents concurrent duplicates.
256-284: Inline ownership verification improves efficiency.The refactored update method efficiently combines ownership verification with the update logic, eliminating the need for a separate method call. The uniqueness validation is also properly synchronized under the write lock.
365-378: Tag filtering implementation is correct.The
list_dashboards_by_tagmethod correctly filters dashboards by checking if the specified tag exists in the dashboard's tags vector.
388-396: Improved ownership verification with better error messaging.The updated
ensure_dashboard_ownershipmethod now returns the dashboard on success and provides more descriptive error messages, which improves the API's usability and debugging experience.
55-55: Verify API consumers for camelCase serialization changes.Our search didn’t reveal any direct references to snake_case field names (e.g.,
dashboard_id,is_favorite) in JSON/JS/TS files, but this doesn’t rule out external clients or documentation still relying on snake_case. Please manually review:
- API documentation or OpenAPI specs
- Client SDKs or integrations
- Existing integration/end-to-end tests
and ensure they’re updated (or that clients are notified) to handle the new camelCase field names (e.g.,
isFavorite).
add below fields to dashboard creation -
ensure title is unique
add API to get all tags -
GET /api/v1/dashboards/list_tagsSummary by CodeRabbit
New Features
Improvements