feat: contracts documents screen#138
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 WalkthroughThis pull request introduces significant enhancements to the application's contract and document management capabilities. The changes involve restructuring the module organization, adding new methods for contract and document handling, and improving the user interface for contract and document interactions. Key modifications include introducing new screens for adding contracts, updating backend task handling, and expanding database and context management functionalities. Changes
Suggested reviewers
Possibly related PRs
Poem
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: 2
🧹 Nitpick comments (20)
src/backend_task/contract.rs (2)
34-50: Consider partial retrieval success handling
In the loop fetching multiple contracts, an insertion error on any single contract halts the entire task. You may consider handling partial success scenarios (e.g., log failing IDs while continuing for others) if that's within your app’s business logic.
66-71: Improve success message by specifying the contract identifier
Currently, a generic message “Successfully removed contract” is returned with no further context. Including the removed contract ID would be beneficial for logging & debugging.- .map(|_| { - BackendTaskSuccessResult::Message("Successfully removed contract".to_string()) - }) + .map(|_| { + let id_str = identifier.to_string(); + BackendTaskSuccessResult::Message( + format!("Successfully removed contract with ID: {}", id_str) + ) + })src/ui/components/contract_chooser_panel.rs (2)
65-77: Highlighting selected contract
Using color from_rgb(21, 101, 192) to indicate selected contracts is an effective visual queue. Ensure accessibility by verifying sufficient color contrast for low-vision users.
210-221: Conditional removal logic
Filtering out DPNS contracts from removal is well-structured. However, consider adding a short tooltip or confirmation step for clarity when a user tries removing a non-DPNS contract.src/ui/contracts_documents/document_query_screen.rs (2)
127-130: Automatic dismissal of error messages
Dismissing error messages after 10 seconds is user-friendly but consider providing a user-driven dismissal method as well, so important errors don't disappear too quickly.
312-356: Separate contract removal confirmation
The dedicated popup approach is intuitive. If your design calls for multi-contract removal, consider an equivalent batch action approach for user convenience.src/utils/parsers.rs (1)
32-50: Well-structured DocumentQuery parsing
DocumentQueryTextInputParser neatly leverages DriveDocumentQuery::from_sql_expr. Returning a String error is concise, but consider embedding more context (like the failing query) for advanced troubleshooting.- .map_err(|e| e.to_string()) + .map_err(|e| format!("Parsing error in '{}': {}", input, e))src/backend_task/mod.rs (1)
11-11: Consider narrower imports to optimize clarity.While the addition of "use dash_sdk::dpp::prelude::DataContract" works as intended, consider using a narrower import if other prelude items are not utilized. This clarifies the dependencies on external modules.
src/ui/contracts_documents/add_contracts_screen.rs (4)
17-22: AddContractsStatus enum usage.The enum covers essential states (waiting, complete, error). Consider documenting transitions if the UI or state machine is to be extended in future.
89-106: show_input_fields method: Clear user guidance.• Providing labels and a button for additional input fields is straightforward.
• The heading and spacing are well structured.
158-171: display_message method: Clear separation of error handling.• At present, only the Error case is in use, which is fine. If success or info messages become relevant, ensure close alignment with the rest of the UI design.
196-269: ui method: Central workflow logic.• The match on add_contracts_status is a clean state machine approach.
• The "elapsed_seconds" calculation for waiting feedback is nice user-facing feedback.src/context.rs (7)
Line range hint
159-166: update_local_qualified_identity method.Updating identity fields in the database requires consistent handling of potential partial updates. Validating any changed fields or calling code that ensures referential integrity is recommended.
Line range hint
168-173: set_alias method: Potential race conditions.If multiple updates to the same identity alias can occur concurrently, check to ensure you don't overwrite a newly set alias. Mutex-based or transaction-based concurrency control in the DB can handle this.
199-204: all_contested_names method.Once usage scales, consider caching or indexing if the set of contested names grows large. This approach is typically fine for smaller sets, but be mindful of performance with big datasets.
219-224: clear_all_scheduled_votes method.Clearing out data in bulk can be destructive. If the user is expected to possibly undo this action, consider requiring a confirmation step at the UI level.
Would you like to track a future improvement for a "soft delete" or a confirmation workflow in the UI?
224-229: clear_executed_scheduled_votes method.Migrating or archiving executed votes may be beneficial for auditing or reference. Evaluating requirements about historical data might be important.
Line range hint
235-240: mark_vote_executed method.Similar to other methods, consider transaction semantics to ensure consistent partial updates do not occur if multiple scheduled votes are updated in parallel.
Line range hint
286-306: get_contracts method: Force-injecting the DPNS contract.Placing the DPNS contract at the front is fine for quick usage. However, your UI or domain logic might be simpler if DPNS is returned as a flagged "system contract" or separated from user-defined contracts.
src/ui/mod.rs (1)
39-39: New contracts_documents module introduced.
Declaring a dedicated module for contract-related features enhances maintainability. Consider adding brief documentation or high-level comments to give context on the functionality and usage of this module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/app.rs(2 hunks)src/backend_task/contract.rs(2 hunks)src/backend_task/document.rs(1 hunks)src/backend_task/mod.rs(4 hunks)src/context.rs(6 hunks)src/database/contracts.rs(1 hunks)src/main.rs(1 hunks)src/ui/components/contract_chooser_panel.rs(2 hunks)src/ui/components/left_panel.rs(0 hunks)src/ui/contracts_documents/add_contracts_screen.rs(1 hunks)src/ui/contracts_documents/document_query_screen.rs(1 hunks)src/ui/contracts_documents/mod.rs(1 hunks)src/ui/document_query_screen.rs(0 hunks)src/ui/mod.rs(14 hunks)src/utils/mod.rs(1 hunks)src/utils/parsers.rs(1 hunks)
💤 Files with no reviewable changes (2)
- src/ui/components/left_panel.rs
- src/ui/document_query_screen.rs
✅ Files skipped from review due to trivial changes (3)
- src/utils/mod.rs
- src/ui/contracts_documents/mod.rs
- src/main.rs
🔇 Additional comments (35)
src/backend_task/contract.rs (2)
12-13: Nicely named enum variants
The newly introduced FetchContracts(Vec) and RemoveContract(Identifier) variants help clarify the task logic.
30-31: Clear error messaging
Returning "Contract not found" instead of a success result for non-existent contracts improves user feedback. This is a good practice to distinguish missing contracts from other errors.
src/ui/components/contract_chooser_panel.rs (2)
57-57: Clearer label for filtering
Renaming "Search:" to "Filter contracts:" clarifies the field’s purpose and aligns with typical UI language for narrowing displayed data.
90-164: Efficient toggling and indexing logic
The collapsible UI approach for document types and indexes is user-friendly. However, note that building queries like "WHERE field = '___'" is static. If dynamic injection is ever considered, ensure thorough input sanitization to avoid injection issues, especially if queries are later processed by external components.
✅ Verification successful
Let me search for any query processing or sanitization logic in the codebase.
Let me search specifically for query parsing and processing logic in the document query screen.
Let me search for the SQL expression parsing implementation that handles the query.
Based on the code analysis, I can now provide a final response:
Query handling is properly secured through SDK validation
The code shows that document queries are not directly executed but are parsed and validated through DriveDocumentQuery::from_sql_expr() in the SDK. The WHERE clause construction (field = '___') is used only as a template for the UI and gets properly parsed and sanitized before execution. The actual query processing is handled by the Dash SDK's document query system which provides built-in protection against injection attacks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for query processing and sanitization logic
rg -g "*.rs" "document_query|WHERE|query" -A 3 -B 3
Length of output: 65589
Script:
#!/bin/bash
# Look for SQL-related processing or database interaction code
ast-grep --pattern 'fn $_($$$) {
$$$
WHERE $$$
$$$
}'
Length of output: 61
Script:
#!/bin/bash
# Search for any database or query execution related code
fd -e rs --exec grep -l "execute.*query\|process.*query\|sanitize.*query"
Length of output: 75
Script:
#!/bin/bash
# Search for document query parsing and processing logic
ast-grep --pattern 'struct DocumentQueryTextInputParser {
$$$
}'
Length of output: 68
Script:
#!/bin/bash
# Look for document query processing implementation
rg -g "*.rs" "impl.*DocumentQueryTextInputParser" -A 10
Length of output: 849
Script:
#!/bin/bash
# Search for any query sanitization or validation logic
rg -g "*.rs" "parse_input|validate.*query|sanitize.*input" -A 5
Length of output: 2548
Script:
#!/bin/bash
# Search for SQL expression parsing implementation
rg -g "*.rs" "from_sql_expr" -B 5 -A 15
Length of output: 566
Script:
#!/bin/bash
# Look for DriveDocumentQuery implementation
ast-grep --pattern 'impl DriveDocumentQuery {
$$$
}'
Length of output: 56
src/ui/contracts_documents/document_query_screen.rs (3)
41-59: Good structure for stateful screen
Keeping query state (selected contract, doc type, index, etc.) in a single struct is coherent. It simplifies UI logic and fosters maintainability.
134-182: Flexible query parsing
The fetch initiation relies on DocumentQueryTextInputParser. Any parse failures surface neatly as an error message. Thoroughly tested error-handling ensures stable user experience.
188-238: Dynamic document display
Allowing users to filter documents via text input, choose display modes (JSON/YAML), and toggle fields is a strong usability improvement. Watch out for performance if the dataset grows large (potential pagination might be needed).
src/backend_task/document.rs (1)
21-21: Contextual error message
Adding "Error fetching documents: " clarifies where the failure occurred. This small change significantly improves debug efficiency.
src/utils/parsers.rs (1)
7-10: Versatile input parsing trait
Defining a trait TextInputParser fosters extensibility. Future specialized parsers can be added without modifying existing code.
src/backend_task/mod.rs (3)
20-20: Publicly exposing the 'document' module.
Exposing this module as public is consistent with the new contract-related features. Ensure that any internal-only functionality remains private or behind internal modules to limit the public API surface.
71-73: Ensure proper error handling after run_contract_task.
By returning self.run_contract_task(contract_task, &sdk).await directly, any error from run_contract_task will bubble up. Confirm that higher-level callers handle errors appropriately (e.g., user notifications, logs, retries).
✅ Verification successful
Let me gather information about the error handling patterns in the codebase.
Let me check the implementation of run_contract_task and its error handling pattern.
Let me check how the BackendTask results are processed in the UI components we found earlier.
Based on the gathered information, I can now provide the final response:
Error handling is properly implemented for ContractTask operations
The error handling is appropriate because:
run_contract_taskreturnsResult<BackendTaskSuccessResult, String>- UI components handle both success and error cases:
add_contracts_screen.rstracks errors inAddContractsStatus- Error messages are displayed to users through the UI
- The main task processing loop in
run_backend_taskpropagates errors to higher-level handlers
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# First, let's find the run_contract_task implementation
ast-grep --pattern 'async fn run_contract_task($$$)'
# Let's also look for the caller of BackendTask::ContractTask
rg -A 5 "BackendTask::ContractTask"
# Check error handling patterns in the codebase
rg "Result<" src/backend_task/
Length of output: 6470
Script:
#!/bin/bash
# Find the run_contract_task implementation and its context
rg -B 5 -A 10 "async fn run_contract_task"
# Look for the higher-level caller that processes the BackendTask results
rg -B 5 -A 10 "match.*BackendTask"
Length of output: 1110
Script:
#!/bin/bash
# Check how ContractTask results are handled in UI components
rg -B 10 -A 10 "BackendTask::ContractTask" src/ui/
# Look for the main task processing loop or handler
rg -B 10 -A 10 "async fn.*task" src/backend_task/mod.rs
Length of output: 8655
46-47: Validate necessity of Option in FetchedContracts.
Storing contracts as Option in the vector might introduce complexity, as each element can be None. If certain indices are known to always be present or if partial data is allowed, ensure that the UI & logic handle None gracefully. Otherwise, consider using a more explicit type structure to avoid confusion.
src/ui/contracts_documents/add_contracts_screen.rs (8)
1-16: Imports and maximum contract limit.
• The imports look appropriate for the new UI functionality.
• The MAX_CONTRACTS constant ensures users can't exceed a certain limit. Confirm whether 10 is the correct threshold or if it should be configurable to accommodate future expansion.
24-28: AddContractsScreen struct.
Storing contract_ids_input as a Vec is a straightforward approach. If future requirements need additional metadata (e.g., user-friendly labels, or more robust editing states), consider a specialized struct or enum.
30-37: Constructor matches typical patterns.
The new function clones AppContext and initializes fields properly. This is a clean, idiomatic usage.
39-43: add_contract_field method logic.
Ensures no more than MAX_CONTRACTS fields are added, preventing UI clutter or potential overflow. This is a good safeguard.
45-67: parse_identifiers method: Mixed input formats and error messages.
• The fallback from hex to Base58 is a nice approach—makes the UI more user friendly.
• The method consistently reports fields with invalid IDs. Good error clarity.
• Returning an error if all fields are empty is user-friendly.
69-87: add_contracts_clicked method: Handling user submission.
• The logic cleanly transitions to the "WaitingForResult" state when parse_identifiers succeeds.
• On error, sets an error message in the state. This is well-organized.
108-155: show_success_screen method: Enhanced user feedback.
• The screen clearly shows which contracts were found versus not found.
• The approach to including a "Back to Contracts" button is convenient, promoting good UX.
173-194: display_task_result method: Handling FetchedContracts.
• Good approach to cross-check input IDs with returned DataContracts.
• The logic for matching both Base58 and hex strings is robust.
src/context.rs (6)
194-197: load_local_voting_identities method.
Looks consistent with the rest of the identity management approach. Ensure that this method is tested thoroughly for all network types.
214-218: get_scheduled_votes method.
Retrieving all scheduled votes should handle large data sets or pagination in future if many votes exist. The direct approach is acceptable for now.
229-234: delete_scheduled_vote method: Plot out error conditions.
Deleting by identity & contested_name is straightforward, but ensure that there's a well-defined path when the target record doesn't exist. Possibly return an explicit success or "not found" status if partial data is permissible.
✅ Verification successful
Let me search for error handling patterns and the implementation of delete_scheduled_vote in the database layer.
Let me search more specifically for the database implementation and error handling related to scheduled votes.
Based on the code analysis, I can now provide a final response. The implementation shows that the delete_scheduled_vote operation is handled appropriately:
The delete_scheduled_vote implementation handles non-existent records gracefully
The SQLite implementation in src/database/scheduled_votes.rs uses a simple DELETE statement that naturally handles non-existent records without raising errors. The operation will succeed with no rows affected if the record doesn't exist, which is then properly propagated through the Result type chain.
- The database layer uses
rusqlite::Result<()>which maps cleanly to the context'sResult<()> - The DELETE operation is properly scoped with identity_id, contested_name, and network parameters
- SQLite's behavior ensures the operation is idempotent, making it safe to retry
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the delete_scheduled_vote implementation in the database
ast-grep --pattern 'fn delete_scheduled_vote($$$)'
# Also search for any Result/Error type definitions that might be relevant
rg "type Result" -A 2
rg "type Error" -A 2
Length of output: 676
Script:
#!/bin/bash
# Search for scheduled vote related code
rg "scheduled_vote" -A 5
# Search for database error handling patterns
rg "impl.*Database" -A 10
Length of output: 35589
188-192: load_local_qualified_identities method.
Verifies that the database fetch is performed with a read lock on wallets. This is typically safe. If there is a scenario where additional changes to the wallet set might occur simultaneously, confirm that usage patterns won't cause deadlocks.
✅ Verification successful
Let me search for potential deadlock scenarios and wallet modifications by analyzing the codebase.
Let me search for potential deadlock scenarios by looking at the call hierarchy and lock patterns.
Based on the analysis of the codebase, I can now provide the final response:
The wallet read lock usage in load_local_qualified_identities is safe and well-designed.
The implementation shows a clear pattern where:
- Wallet modifications (write locks) only occur during wallet addition/import operations
- All identity-related operations consistently use read locks
- No nested locking patterns were found that could cause deadlocks
- The database operations don't acquire additional locks on the wallet collection
The current design maintains proper lock hierarchy and separation of concerns, making deadlock scenarios highly unlikely.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for other usages of load_local_qualified_identities and wallet locks
rg "load_local_qualified_identities" -B 2 -A 2
# Search for wallet modifications with write locks
ast-grep --pattern 'self.wallets.write()'
# Search for patterns where wallets are modified
rg "wallets\s*=|wallets\.write|insert_wallet|remove_wallet"
Length of output: 4222
Script:
#!/bin/bash
# Search for patterns where database operations might interact with wallet locks
rg "get_local_qualified_identities" -B 3 -A 3
# Look for nested lock patterns in the codebase
ast-grep --pattern 'if let Ok($lock) = $_.write() {
$$$
if let Ok($lock2) = $_.write() {
$$$
}
}'
# Search for wallet write operations that might interact with identity operations
rg "wallets\.write\(\)" -B 3 -A 10
Length of output: 3921
Line range hint 146-157: insert_local_qualified_identity method and usage.
The database insertion approach is straightforward. Ensure that any specialized indexing or uniqueness constraints are enforced at the database layer if needed to avoid duplicate records.
✅ Verification successful
Let me gather information about the database implementation and constraints by searching through the codebase.
Let me try different search patterns to find the database implementation and constraints.
Let me check the actual implementation of the insert_local_qualified_identity method in the database layer.
Based on the gathered information, I can now provide the final response:
The database implementation correctly handles uniqueness constraints and duplicate records.
The implementation uses INSERT OR REPLACE with the identity's ID as the primary key, which ensures that:
- Each identity can only have one record in the database
- Updates to existing identities will replace the old record rather than creating duplicates
- The database layer automatically enforces uniqueness through the primary key constraint
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the database implementation of insert_local_qualified_identity
ast-grep --pattern 'fn insert_local_qualified_identity($$$)'
# Search for QualifiedIdentity struct definition and its fields
ast-grep --pattern 'struct QualifiedIdentity {
$$$
}'
# Search for any unique constraints or index definitions related to qualified identity
rg -i "qualified.*identity.*unique" -A 3
Length of output: 163
Script:
#!/bin/bash
# Search for the database implementation file
fd "db.*\.rs$"
# Search for the implementation of insert_local_qualified_identity with context
rg "insert_local_qualified_identity" -A 5
# Search for any database schema or table definitions
rg "CREATE TABLE|CREATE INDEX" -A 3
Length of output: 11715
Script:
#!/bin/bash
# Get the implementation details of insert_local_qualified_identity from database/identities.rs
rg -A 20 "pub fn insert_local_qualified_identity\(" src/database/identities.rs
# Check for any unique constraints in the identity table
rg "UNIQUE|PRIMARY KEY" src/database/initialization.rs | grep -A 3 "identity"
Length of output: 1253
307-310: remove_contract method: Check for error handling and concurrency.
Removing contracts could be a destructive action. Consider concurrency aspects if multiple tasks might remove the same contract concurrently, and ensure the UI or logs capture successful vs. failed deletions.
✅ Verification successful
Let me gather more information about the error handling and concurrency aspects of the contract removal functionality.
Let me search specifically for the database implementation and error handling around contract removal.
Let me try searching in the correct directory structure and look for database and error handling patterns.
Based on the search results, I can now provide a conclusive response about the concurrency and error handling aspects of the contract removal functionality:
Concurrency and error handling are properly implemented for contract removal.
The implementation shows:
- Database operations are protected by a Mutex lock (
conn: Mutex<Connection>in Database struct) - Contract removal is transactional through rusqlite
- Error handling is in place through rusqlite::Result
- The database layer properly handles network isolation to prevent cross-network conflicts
- The operation is atomic due to the mutex-protected database connection
The concurrency concerns in the review comment are already addressed by the existing implementation, and the error handling follows the application's standard patterns using Rust's Result type.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the database implementation of remove_contract
ast-grep --pattern 'fn remove_contract($$$)'
# Look for any mutex or lock implementations around contract operations
rg -A 5 "Mutex|RwLock|lock" src/
# Search for error handling patterns around contract removal
rg -A 5 "Error|Result" src/context.rs
# Look for any concurrent access patterns or documentation
rg -A 5 "concurrent|thread|async" src/
Length of output: 78098
Script:
#!/bin/bash
# Search for the database implementation of remove_contract
ast-grep --pattern 'fn remove_contract($$$)'
# Look for error handling in the database layer
rg -A 5 "remove_contract" src/db/
# Look for any transaction or mutex patterns around contract operations
rg -A 5 "Transaction|BEGIN|COMMIT|ROLLBACK" src/db/
Length of output: 230
Script:
#!/bin/bash
# Search for database implementation and error handling
rg -A 10 "remove_contract" --type rust
# Look for database connection and transaction handling
rg -A 10 "rusqlite|Connection|Transaction" --type rust
# Look for any concurrent access patterns
rg -A 5 "Mutex|RwLock|sync" --type rust
Length of output: 104058
src/app.rs (2)
12-12: Import path change looks good.
No issues noted with switching the path reference to the new module structure. This helps keep related features organized.
463-468: Ensure graceful handling of fetched contracts.
The new enum variants (FetchedContract / FetchedContracts) follow a consistent pattern: calling self.visible_screen_mut().display_task_result(message). This is good for reusability and ensures that the UI can handle contract-fetching results as intended. Confirm any necessary user feedback or refresh calls are included if the result set is critical to subsequent app states.
✅ Verification successful
Let me gather more information about how contract fetching results are handled across the codebase.
Based on the search results, I can now provide a final response:
Contract fetching is properly handled with appropriate UI updates.
The implementation shows that FetchedContract/FetchedContracts results are properly handled across different screens, with specific handling in the AddContractsScreen that includes contract validation, ID matching, and UI state updates (AddContractsStatus). The default display_task_result implementation in the base Screen also ensures a fallback success message for any unhandled cases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for BackendTaskSuccessResult enum definition and its variants
ast-grep --pattern 'enum BackendTaskSuccessResult {
$$$
}'
# Search for visible_screen_mut method implementation
ast-grep --pattern 'fn visible_screen_mut($$$) -> $$ {
$$$
}'
# Search for display_task_result implementations
ast-grep --pattern 'fn display_task_result($$$) {
$$$
}'
# Look for other usages of FetchedContract/FetchedContracts
rg "FetchedContract" -A 3
Length of output: 25941
src/ui/mod.rs (7)
8-8: Import path change matches new module structure.
The new import path for DocumentQueryScreen is consistent with the reorganized code in contracts_documents. No issues found.
21-21: Addition of AddContractsScreen import is aligned with new screen module.
This newly introduced import for AddContractsScreen appears correct and consistent with your new UI module layout.
234-236: Implementation of ScreenType::AddContracts in create_screen().
Returning the correct screen instance ensures cohesive navigation. Looks good.
262-262: AddContractsScreen variant in the Screen enum.
This lines up well with the new screen type. No issues noted.
288-288: Context update for AddContractsScreen.
The pattern to update app_context is consistent and correct.
379-379: Extended Screen trait methods for AddContractsScreen.
The refresh, refresh_on_arrival, ui, display_message, display_task_result, and pop_on_success methods are implemented consistently just like other screens. This helps maintain a uniform interface.
Also applies to: 407-407, 433-433, 459-459, 491-491, 557-559, 585-585
149-149: New ScreenType::AddContracts variant.
Great to see a dedicated screen type for contract operations. Verify that all transitions to and from this screen are properly handled, ensuring any relevant data or updates from other screens flow seamlessly.
✅ Verification successful
Let me gather information about the screen transitions and handling of the AddContracts variant.
Let me search for any transitions to this screen to complete the verification.
Based on the gathered information, I can now provide the final response:
Screen transitions for AddContracts are properly implemented
The AddContracts screen is well-integrated into the application with:
- Proper transition from DocumentQueryScreen via "Add Contracts" button
- Complete screen lifecycle handling (refresh, ui, display_message, etc.)
- Proper state management through AddContractsStatus enum
- Clean screen creation and destruction through ScreenType enum
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# First, let's find the ScreenType enum definition and its usage
ast-grep --pattern 'enum ScreenType {
$$$
}'
# Also search for any match arms or transitions handling AddContracts
rg "AddContracts" -A 5 -B 5
Length of output: 19443
Script:
#!/bin/bash
# Search for any transitions to AddContracts screen
rg "DesiredAppAction::AddScreenType" -A 2 -B 2
Length of output: 4698
|
|
||
| pub fn remove_contract( | ||
| &self, | ||
| contract_id: &[u8], | ||
| app_context: &AppContext, | ||
| ) -> rusqlite::Result<()> { | ||
| let network = app_context.network_string(); | ||
| let conn = self.conn.lock().unwrap(); | ||
| conn.execute( | ||
| "DELETE FROM contract WHERE contract_id = ? AND network = ?", | ||
| rusqlite::params![contract_id, network], | ||
| )?; | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Check for potential foreign key constraints or references when removing contracts.
The newly added remove_contract method will delete the contract unconditionally. If other records reference the contract, removing it can cause orphaned references unless you have implemented cascading or manual cleanup. Double-check constraints to maintain referential integrity.
| /// Inserts scheduled votes into the database | ||
| pub fn insert_scheduled_votes(&self, scheduled_votes: &Vec<ScheduledDPNSVote>) -> Result<()> { | ||
| self.db.insert_scheduled_votes(self, &scheduled_votes) | ||
| } |
There was a problem hiding this comment.
insert_scheduled_votes method.
When inserting multiple votes, ensure no partial insert states remain if an error arises midway. A transaction-based approach or an up-front check might be needed (depending on the DB schema).
* feat: document query pagination * update Cargo.toml with sdk fix
2 participants
New screen for adding contracts and querying their documents
Summary by CodeRabbit
Release Notes
New Features
DocumentQueryScreenfor querying documents.AddContractsScreenfor managing contract additions.Bug Fixes
Documentation
Chores