refactor: how dashboards are handled #1173
Conversation
* generate ids if not provided * set default value * improve error response(404) * improve method signature
WalkthroughThe pull request refactors dashboard handling and data struct definitions. In the HTTP handler, the retrieval method has been altered from Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant H as HTTP Handler
participant D as Dashboard Service
C->>H: Request dashboard (dashboard_id, user_id)
H->>D: Call get(dashboard_id, get_hash(user_id))
alt Dashboard exists
D-->>H: Dashboard Data
H-->>C: Return dashboard info (200 OK)
else Dashboard missing
D-->>H: None
H-->>C: Return error DashboardDoesNotExist (404 NOT FOUND)
end
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
37-46: LGTM! Consider adding a constant for the random string length.The implementation ensures uniqueness through a combination of random string and microsecond timestamp.
Consider extracting the magic number
8into a named constant for better maintainability:+const TILE_ID_RANDOM_LENGTH: usize = 8; + fn gen_tile_id() -> String { get_hash( format!( "{}{}", - rand::distributions::Alphanumeric.sample_string(&mut rand::thread_rng(), 8), + rand::distributions::Alphanumeric.sample_string(&mut rand::thread_rng(), TILE_ID_RANDOM_LENGTH), Utc::now().timestamp_micros() ) .as_str(), ) }
112-118: LGTM! Consider adding documentation for these helper functions.The implementation is clean and straightforward. The functions serve their purpose well as default value generators.
Consider adding documentation to explain the purpose and behavior of these functions:
+/// Returns the current dashboard version string. fn default_version() -> String { CURRENT_DASHBOARD_VERSION.to_owned() } +/// Generates a unique dashboard ID using microsecond timestamp. fn gen_dashboard_id() -> String { get_hash(Utc::now().timestamp_micros().to_string().as_str()) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/handlers/http/users/dashboards.rs(7 hunks)src/users/dashboards.rs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: coverage
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (8)
src/users/dashboards.rs (4)
51-52: LGTM! Good use of serde's default attribute.The change from
Option<String>toStringwith a default generator simplifies the code by eliminating the need forOptionhandling.
122-127: LGTM! Good use of default value generators.The change from
Option<String>toStringwith default generators for bothversionanddashboard_idfields simplifies the code and ensures these essential fields are always present.
211-214: LGTM! Method name and implementation are now more concise.The simplified comparison logic aligns well with the non-optional
dashboard_idfield.
216-223: LGTM! Good use ofis_some_andfor cleaner Option handling.The method name is more concise, and the comparison logic is more elegant using
is_some_and.src/handlers/http/users/dashboards.rs (4)
50-50: LGTM! Improved error handling with specific error variant.The method call aligns with the renamed method, and the error handling is now more specific and descriptive.
Also applies to: 54-54
86-87: LGTM! Consistent error handling.The error handling is consistent with other methods and uses the specific error variant.
111-112: LGTM! Consistent error handling.The error handling is consistent with other methods and uses the specific error variant.
131-132: LGTM! Improved error reporting with specific variant and status code.The new error variant and its status code mapping improve the clarity of error reporting.
Also applies to: 143-143
Fixes #XXXX.
Description
This PR has:
Summary by CodeRabbit
Bug Fixes
Refactor