feat: deferred voting #129
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces significant enhancements to the application's voting functionality, particularly focusing on scheduled votes. Key changes include the addition of new methods and fields in various structs, updates to enums for better state management, and the introduction of a new database module for handling scheduled votes. The user interface is also modified to accommodate these features, allowing users to schedule votes and manage them effectively. Overall, the changes expand the application's capabilities in managing and displaying voting tasks. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (8)
src/backend_task/contested_names/mod.rs (3)
27-33: Add documentation comments toScheduledDPNSVotestruct and its fields.Adding
///comments to theScheduledDPNSVotestruct and its fields will improve code readability and help other developers understand its purpose and usage.
54-55: Simplify error formatting by removing unnecessaryto_string()calls.When formatting error messages, you can use
{}directly with the error, as it implements theDisplaytrait. Theto_string()call is redundant.Apply this diff to simplify the code:
.map_err(|e| format!("Error inserting scheduled votes: {}", e.to_string())) + .map_err(|e| format!("Error inserting scheduled votes: {}", e)) .map_err(|e| format!("Error clearing all scheduled votes: {}", e.to_string())) + .map_err(|e| format!("Error clearing all scheduled votes: {}", e)) .map_err(|e| format!("Error clearing executed scheduled votes: {}", e.to_string())) + .map_err(|e| format!("Error clearing executed scheduled votes: {}", e)) .map_err(|e| format!("Error clearing scheduled vote: {}", e.to_string())) + .map_err(|e| format!("Error clearing scheduled vote: {}", e))Also applies to: 70-71, 74-75, 78-78
56-66: Avoid unnecessary cloning and allocation inCastScheduledVotevariant.Instead of creating a new vector with a cloned
voter, you can pass a slice reference tovote_on_dpns_name, reducing memory allocation and cloning overhead.Apply this diff to optimize the code:
self.vote_on_dpns_name( &scheduled_vote.contested_name, scheduled_vote.choice, - &vec![voter.clone()], + std::slice::from_ref(voter), sdk, sender, )src/ui/dpns_vote_scheduling_screen.rs (1)
181-183: Prevent action when no votes are selected.The function returns
AppAction::Nonewhen no votes are selected, but it might be more user-friendly to disable the "Schedule Votes" button instead.Consider disabling the button when
scheduled_votes.is_empty()to provide immediate feedback to the user.src/app.rs (1)
515-582: Optimize scheduled vote checks to reduce database load.Currently, the application checks for scheduled votes every 60 seconds, which may not be efficient. Consider implementing a more efficient scheduling mechanism, such as using a prioritized task queue or only querying when necessary.
src/backend_task/contested_names/vote_on_dpns_name.rs (1)
91-91: TODO: Implement previous vote count removal.The comment indicates a missing feature for handling previous votes. This should be implemented to ensure accurate vote counting.
Would you like me to help implement the logic for removing previous vote counts or create a GitHub issue to track this task?
src/context.rs (1)
202-226: Consider adding documentation for the scheduled votes management functions.The implementation is clean and follows good practices. Consider adding documentation to describe the purpose and behavior of each function.
Add documentation like this:
+/// Inserts a vector of scheduled votes into the database. +/// Returns a Result indicating success or failure. pub fn insert_scheduled_votes(&self, scheduled_votes: &Vec<ScheduledDPNSVote>) -> Result<()> { self.db.insert_scheduled_votes(self, &scheduled_votes) } +/// Retrieves all scheduled votes from the database. +/// Returns a Result containing a vector of ScheduledDPNSVote. pub fn get_scheduled_votes(&self) -> Result<Vec<ScheduledDPNSVote>> { self.db.get_scheduled_votes(&self) }src/ui/identities/identities_screen.rs (1)
Line range hint
91-98: Improve error handling for alias updates.While the implementation correctly uses the new
app_context.set_alias, the error handling could be enhanced to provide better user feedback.Consider updating the error handling to show the error to the user:
- match self.app_context.set_alias( - &identity_to_update.identity.id(), - identity_to_update.alias.as_ref().map(|s| s.as_str()), - ) { - Ok(_) => {} - Err(e) => { - eprintln!("{}", e) - } - } + if let Err(e) = self.app_context.set_alias( + &identity_to_update.identity.id(), + identity_to_update.alias.as_ref().map(|s| s.as_str()), + ) { + eprintln!("{}", e); + // Show error to user using egui's toast or popup + ui.show_error("Failed to update alias", &e.to_string()); + // Revert the alias change in the UI + identity_to_update.alias = qualified_identity.alias.clone(); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/app.rs(15 hunks)src/backend_task/contested_names/mod.rs(2 hunks)src/backend_task/contested_names/vote_on_dpns_name.rs(2 hunks)src/backend_task/mod.rs(2 hunks)src/context.rs(5 hunks)src/database/initialization.rs(3 hunks)src/database/mod.rs(1 hunks)src/database/scheduled_votes.rs(1 hunks)src/ui/components/dpns_subscreen_chooser_panel.rs(2 hunks)src/ui/components/top_panel.rs(1 hunks)src/ui/dpns_contested_names_screen.rs(21 hunks)src/ui/dpns_vote_scheduling_screen.rs(1 hunks)src/ui/identities/identities_screen.rs(1 hunks)src/ui/mod.rs(18 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/database/mod.rs
🔇 Additional comments (18)
src/ui/dpns_vote_scheduling_screen.rs (1)
122-134: Maintain consistency by resetting scheduling fields when changing selection.
When a user switches from "Scheduled" to "None" and back to "Scheduled", ensure that the time fields reset to default values to avoid unintended scheduling.
Ensure that the days, hours, and minutes values are reset appropriately when the selection changes.
src/app.rs (1)
Line range hint 428-453: Consider handling all possible BackendTaskSuccessResult variants.
In the match statement, ensure that all variants of BackendTaskSuccessResult are handled to prevent future issues if new variants are added.
Review the match arms and consider adding a wildcard arm or handling all possible variants.
src/ui/mod.rs (2)
148-149: Verify all new ScreenType and Screen variants are handled appropriately
The new variants ScheduleVoteScreen and ScheduledVotes have been added to ScreenType and Screen. Please ensure that all match statements and methods handling these enums are updated accordingly throughout the codebase to prevent any potential runtime panics due to unhandled cases.
Also applies to: 258-258
219-231: Correct implementation of create_screen method for new screen types
The create_screen method has been properly updated to handle the new ScheduleVoteScreen and ScheduledVotes types, ensuring that the application can create and manage these screens effectively.
src/ui/dpns_contested_names_screen.rs (3)
49-57: Proper addition of new enums for scheduled voting functionality
The introduction of ScheduledVotes to DPNSSubscreen and the definition of IndividualVoteCastingStatus enhance the screen's capability to manage scheduled votes.
83-84: Initialization of new fields for vote scheduling state management
Adding popup_pending_vote_action and vote_cast_in_progress fields improves the handling of state transitions during vote scheduling and casting processes.
738-930:
Potential concurrency issues with scheduled_votes access
The render_table_scheduled_votes method accesses self.scheduled_votes, which is an Arc<Mutex<...>>, within the UI thread. To prevent data races or deadlocks when accessing shared data across threads, ensure that locking is handled carefully and consider using try_lock with proper error handling or other synchronization mechanisms.
Would you like assistance in reviewing the thread safety of scheduled_votes access?
src/backend_task/mod.rs (1)
Line range hint 10-43: Extending BackendTaskSuccessResult with scheduled vote handling
The addition of Refresh and CastScheduledVote(ScheduledDPNSVote) variants appropriately extends the backend task results to support the new scheduled voting functionalities without disrupting existing logic.
src/ui/components/dpns_subscreen_chooser_panel.rs (1)
Line range hint 14-75: Seamless integration of 'Scheduled Votes' into subscreen navigation
Including DPNSSubscreen::ScheduledVotes in the subscreen chooser allows users to navigate to the scheduled votes screen efficiently, enhancing the application's usability.
src/backend_task/contested_names/vote_on_dpns_name.rs (4)
26-33: LGTM: Proper UI feedback implementation.
The refresh task implementation provides good user feedback during vote casting, particularly useful for the Scheduled Votes Screen.
42-42: LGTM: Consistent error message formatting.
The error message now follows the consistent "Error voting:" prefix pattern used throughout the function.
85-88: LGTM: Improved error message clarity.
The error message now includes both the consistent prefix and the identity ID, which will help in debugging issues.
99-99: LGTM: Consistent error message formatting.
The error message now follows the consistent "Error voting:" prefix pattern.
src/ui/components/top_panel.rs (1)
155-158: LGTM: Improved button spacing.
The adjusted spacing values provide better visual balance in the top panel layout.
src/database/initialization.rs (2)
7-7: LGTM: Proper database versioning.
The version bump and migration logic are correctly implemented for the new scheduled votes table.
Also applies to: 37-39
354-354: LGTM: Proper table initialization order.
The scheduled votes table initialization is correctly placed after other core table initializations.
src/context.rs (2)
166-168: LGTM: Clean implementation of alias management.
The function properly delegates to the database layer and maintains error propagation.
190-192: LGTM: Clean implementation of voting identity retrieval.
The function correctly retrieves voting identities from the database.
Allow users to schedule masternode votes to be executed later. The app must be running in order for them to be executed automatically.
After clicking on a vote choice on the DPNS Active Contests screen and selecting a node (or all), there is an option to vote now or later. If later is selected, they are given a form to select a time.
There is also a new DPNS subscreen for Scheduled Votes where you can view all scheduled votes and have the option to remove them or cast an individual one immediately instead of at the scheduled time (may be useful if the app is closed when it was supposed to be executed).
Summary by CodeRabbit
Release Notes
New Features
DPNSContestedNamesScreento display and manage scheduled votes.Bug Fixes
Documentation
Chores
These enhancements significantly improve the user experience with voting tasks and scheduled votes management.