Schema and Form ADO #591
Schema and Form ADO #591
Conversation
WalkthroughThe pull request introduces significant changes across multiple files within the Andromeda project. Key modifications include the addition of new dependencies in 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 36
🧹 Outside diff range and nitpick comments (24)
contracts/data-storage/andromeda-form/src/testing/mod.rs (1)
1-3: LGTM! Consider adding module documentation.
The module organization follows good testing practices by separating mock implementations, custom queriers, and tests into distinct modules. This structure promotes better test isolation and maintainability.
Consider adding module-level documentation comments to describe the purpose of each module:
+//! Testing utilities for the Form ADO contract
+
+/// Mock implementations for testing Form ADO functionality
mod mock;
+/// Custom mock querier for handling schema-related requests
mod mock_querier;
+/// Integration and unit tests for the Form ADO contract
mod tests;contracts/modules/andromeda-schema/src/state.rs (1)
1-4: Add documentation for schema format and validation rules.
The implementation looks correct, but could benefit from additional documentation:
- Document the expected JSON schema format
- Specify validation rules and constraints
- Consider adding examples of valid schemas
Add documentation above the SCHEMA constant:
+/// Stores the JSON schema used for validating form submissions.
+/// The schema should follow the JSON Schema specification (https://json-schema.org/).
+///
+/// # Example
+/// ```json
+/// {
+/// "type": "object",
+/// "properties": {
+/// "name": { "type": "string" },
+/// "age": { "type": "integer", "minimum": 0 }
+/// }
+/// }
+/// ```
pub const SCHEMA: Item<JSON> = Item::new("schema");contracts/data-storage/andromeda-form/.cargo/config (1)
1-4: LGTM with suggestions for production improvements.
The cargo aliases are well-structured and follow CosmWasm conventions. Consider these production-ready enhancements:
[alias]
-wasm = "build --release --target wasm32-unknown-unknown"
+wasm = "build --release --target wasm32-unknown-unknown --locked"
-unit-test = "test --lib"
+unit-test = "test --lib --no-fail-fast"
schema = "run --example schema"The changes:
- Added
--lockedto ensure reproducible builds usingCargo.lock - Added
--no-fail-fastto run all tests even if some fail, helpful for CI/CD
contracts/modules/andromeda-schema/src/lib.rs (1)
1-4: Consider adding module-level documentation.
The core module structure looks good and follows best practices. However, adding module-level documentation (//! comments) would improve maintainability by explaining the purpose of each module:
contract: Entry points for contract instantiation and message handlingexecute: Implementation of execute messages for schema operationsquery: Implementation of query messages for schema validationstate: State management and storage logic
+//! Schema ADO implementation for storing and validating JSON schemas
+
pub mod contract;
pub mod execute;
pub mod query;
pub mod state;packages/andromeda-modules/src/schema.rs (1)
10-14: Consider schema update implications
The UpdateSchema operation needs additional considerations:
- How will existing data be validated against the new schema?
- Should there be access control for schema updates?
- Consider adding version tracking for schema changes
contracts/modules/andromeda-schema/src/query.rs (2)
19-21: Consider using borrowed str for better efficiency.
Instead of using data.as_str(), you can directly use &data since String implements AsRef.
- let data_instance = from_str(data.as_str()).map_err(|e| ContractError::CustomError {
+ let data_instance = from_str(&data).map_err(|e| ContractError::CustomError {9-43: Add documentation for public functions.
Both public functions lack documentation. Consider adding doc comments to explain their purpose, parameters, and return values.
Add documentation like this:
/// Retrieves the stored schema from storage.
///
/// # Arguments
///
/// * `storage` - The storage to read from
///
/// # Returns
///
/// * `Result<GetSchemaResponse, ContractError>` - The schema or an error
pub fn get_schema(storage: &dyn Storage) -> Result<GetSchemaResponse, ContractError>
/// Validates the provided JSON data against the stored schema.
///
/// # Arguments
///
/// * `storage` - The storage to read the schema from
/// * `data` - The JSON string to validate
///
/// # Returns
///
/// * `Result<ValidateDataResponse, ContractError>` - Validation result or an error
pub fn validate_data(storage: &dyn Storage, data: String) -> Result<ValidateDataResponse, ContractError>Cargo.toml (1)
61-61: Consider updating jsonschema-valid dependency.
The dependency is functional but relatively old. Consider updating to a newer version for potential bug fixes and improvements.
contracts/modules/andromeda-schema/src/testing/mock.rs (1)
18-30: Add input validation and improve documentation.
While the initialization logic is correct, consider these improvements:
- Add validation for the schema_json_string format
- Document the expected schema format
- Make the assertion message more descriptive
Apply this diff:
pub fn proper_initialization(schema_json_string: String) -> (MockDeps, MessageInfo) {
+ // Validate schema JSON format
+ serde_json::from_str::<serde_json::Value>(&schema_json_string)
+ .expect("Invalid JSON schema format");
+
let mut deps = mock_dependencies_custom(&[]);
let info = mock_info("creator", &[]);
let msg = InstantiateMsg {
kernel_address: MOCK_KERNEL_CONTRACT.to_string(),
owner: None,
schema_json_string,
};
let env = mock_env();
let res = instantiate(deps.as_mut(), env, info.clone(), msg).unwrap();
- assert_eq!(0, res.messages.len());
+ assert_eq!(
+ 0,
+ res.messages.len(),
+ "Expected no messages during initialization"
+ );
(deps, info)
}Also add this documentation above the function:
/// Initializes a mock Schema ADO contract with the provided JSON schema.
///
/// # Arguments
///
/// * `schema_json_string` - A valid JSON string representing the schema structure.
/// Example format:
/// {
/// "type": "object",
/// "properties": {
/// "name": {"type": "string"},
/// "age": {"type": "integer"}
/// }
/// }
contracts/modules/andromeda-schema/src/mock.rs (2)
15-36: Consider using better error handling in instantiate method.
The unwrap() on line 34 could be replaced with proper error handling to make the mock more robust for testing error scenarios.
- .unwrap();
+ .map_err(|e| format!("Failed to instantiate schema contract: {}", e))?;66-81: LGTM! Consider documenting test usage patterns.
The helper functions are well-implemented. Consider adding documentation comments with example usage patterns to help other developers write tests effectively.
Add documentation like:
/// Creates a mock schema contract for testing.
///
/// # Example
/// ```
/// let contract = mock_andromeda_schema();
/// let code_id = app.store_code(contract);
/// ```
pub fn mock_andromeda_schema() -> Box<dyn Contract<Empty>> {contracts/data-storage/andromeda-form/src/testing/mock_querier.rs (1)
18-49: Consider enhancing documentation for test configuration.
While the function implementation is correct, consider documenting:
- The purpose of the hardcoded values in InstantiateMsg
- The relationship between MOCK_CONTRACT_ADDR and contract_balance
/// Alternative to `cosmwasm_std::testing::mock_dependencies` that allows us to respond to custom queries.
///
/// Automatically assigns a kernel address as MOCK_KERNEL_CONTRACT.
+///
+/// # Arguments
+/// * `contract_balance` - Initial balance for the MOCK_CONTRACT_ADDR
+///
+/// The function initializes an ADO contract with:
+/// * ado_type: "form" - Identifies this as a form contract
+/// * ado_version: "test" - Test version identifier
+/// * kernel_address: MOCK_KERNEL_CONTRACT - Standard test kernel addresscontracts/data-storage/andromeda-form/src/mock.rs (1)
156-186: Simplify query methods by removing unnecessary intermediates.
The query methods could be more concise while maintaining readability.
pub fn query_schema(&self, app: &mut MockApp) -> GetSchemaResponse {
let msg = QueryMsg::GetSchema {};
- let res: GetSchemaResponse = self.query(app, msg);
- res
+ self.query(app, msg)
}contracts/modules/andromeda-schema/src/execute.rs (1)
29-33: Ensure consistency in response construction
When combining the res response with action_response, verify that attributes, messages, and events are correctly merged without duplication or conflict. This ensures clear and accurate execution results.
packages/andromeda-data-storage/src/form.rs (3)
9-12: Consider adding comments to all fields in InstantiateMsg for clarity
The field schema_ado_address includes a comment, but the other fields do not. Adding comments to authorized_addresses_for_submission, form_config, and custom_key_for_notifications would enhance code readability and maintain consistency.
17-20: Add comments to fields in FormConfig for consistency
To maintain consistency and improve readability, consider adding comments to each field in FormConfig, similar to those in InstantiateMsg.
74-79: Add comments to fields in SubmissionInfo for clarity
Adding comments to the fields submission_id, wallet_address, and data in SubmissionInfo would improve code understanding and maintain consistency with other structs.
contracts/modules/andromeda-schema/src/testing/tests.rs (1)
303-310: Ensure consistent schema formatting in assertions
In the test_query_schema function, the expected schema string in the assert_eq! macro is a single-line string, which can be hard to read and maintain. Consider formatting the expected schema with proper indentation and line breaks to improve readability.
Refactor the assertion for better readability:
assert_eq!(
schema,
- "{\"$schema\":\"http://json-schema.org/draft-07/schema#\",\"additionalProperties\":false,\"properties\":{\"kernel_address\":{\"type\":\"string\"},\"owner\":{\"type\":[\"string\",\"null\"]},\"schema_json_string\":{\"type\":\"string\"}},\"required\":[\"kernel_address\",\"schema_json_string\"],\"title\":\"InstantiateMsg\",\"type\":\"object\"}".to_string()
+ serde_json::json!({
+ "$schema": "http://json-schema.org/draft-07/schema#",
+ "additionalProperties": false,
+ "properties": {
+ "kernel_address": { "type": "string" },
+ "owner": { "type": ["string", "null"] },
+ "schema_json_string": { "type": "string" }
+ },
+ "required": ["kernel_address", "schema_json_string"],
+ "title": "InstantiateMsg",
+ "type": "object"
+ })
+ .to_string()
);This makes the expected value more readable and easier to maintain.
contracts/data-storage/andromeda-form/src/execute.rs (3)
370-376: Enhance error handling in milliseconds_from_expiration
The milliseconds_from_expiration function currently handles only Expiration::AtTime and returns a generic error for other variants. Consider handling other expiration variants or providing a more descriptive error message to improve clarity.
For example:
pub fn milliseconds_from_expiration(expiration: Expiration) -> Result<Milliseconds, ContractError> {
match expiration {
Expiration::AtTime(time) => Ok(Milliseconds::from_nanos(time.nanos())),
+ Expiration::Never {} => Err(ContractError::CustomError {
+ msg: "Expiration is set to never expire.".to_string(),
+ }),
+ Expiration::AtHeight(height) => Err(ContractError::CustomError {
+ msg: format!("Expiration at block height {} is not supported.", height),
+ }),
_ => Err(ContractError::CustomError {
- msg: "Not supported expiration enum".to_string(),
+ msg: format!("Unsupported expiration variant: {:?}", expiration),
}),
}
}93-138: Handle validation errors more gracefully in execute_submit_form
When data validation fails, the error message returned may not provide sufficient information. Consider providing more detailed error messages or handling specific validation errors to improve the user experience.
68-74: Redundant retrieval of current time
The function execute_submit_form retrieves the current time using get_and_validate_start_time, but this function is designed for validating start times, which may not be necessary here. Consider using ctx.env.block.time directly to get the current time in nanoseconds.
contracts/data-storage/andromeda-form/src/testing/tests.rs (3)
282-288: Reduce redundant calls to START_TIME.load()
Between lines 282-288, START_TIME.load(&deps.storage).unwrap() is called multiple times. To improve efficiency, consider storing the result in a variable and reusing it.
Apply this diff to refactor the code:
let start_time = START_TIME.load(&deps.storage).unwrap();
-let expected_saved_start_time = if START_TIME.load(&deps.storage).unwrap().is_some() {
+let expected_saved_start_time = if start_time.is_some() {
Some(Milliseconds::from_nanos(execute_timestamp).plus_milliseconds(Milliseconds(1)))
} else {
None
};290-295: Remove commented-out debug code
The commented-out println! statements between lines 290-295 are unnecessary and can be removed to keep the codebase clean and maintainable.
Apply this diff to remove the commented code:
-// println!(
-// "//==================== endtime: {:?} ====================//",
-// end_time
-// );510-511: Enhance test assertions for clarity
On lines 510-511, after querying all submissions, you're asserting the length of the submissions vector. For better clarity and to ensure the submissions are as expected, consider asserting the contents of the submissions as well.
Example addition:
assert_eq!(all_submissions, vec![
// Expected SubmissionInfo entries
]);📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
- Cargo.toml (1 hunks)
- contracts/data-storage/andromeda-form/.cargo/config (1 hunks)
- contracts/data-storage/andromeda-form/Cargo.toml (1 hunks)
- contracts/data-storage/andromeda-form/examples/schema.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/contract.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/execute.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/lib.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/mock.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/query.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/state.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/testing/mock.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/testing/mock_querier.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/testing/mod.rs (1 hunks)
- contracts/data-storage/andromeda-form/src/testing/tests.rs (1 hunks)
- contracts/modules/andromeda-schema/.cargo/config (1 hunks)
- contracts/modules/andromeda-schema/Cargo.toml (1 hunks)
- contracts/modules/andromeda-schema/examples/schema.rs (1 hunks)
- contracts/modules/andromeda-schema/src/contract.rs (1 hunks)
- contracts/modules/andromeda-schema/src/execute.rs (1 hunks)
- contracts/modules/andromeda-schema/src/lib.rs (1 hunks)
- contracts/modules/andromeda-schema/src/mock.rs (1 hunks)
- contracts/modules/andromeda-schema/src/query.rs (1 hunks)
- contracts/modules/andromeda-schema/src/state.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/mock.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/mod.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/tests.rs (1 hunks)
- packages/andromeda-data-storage/src/form.rs (1 hunks)
- packages/andromeda-data-storage/src/lib.rs (1 hunks)
- packages/andromeda-modules/src/lib.rs (1 hunks)
- packages/andromeda-modules/src/schema.rs (1 hunks)
✅ Files skipped from review due to trivial changes (8)
- contracts/data-storage/andromeda-form/Cargo.toml
- contracts/data-storage/andromeda-form/examples/schema.rs
- contracts/modules/andromeda-schema/.cargo/config
- contracts/modules/andromeda-schema/Cargo.toml
- contracts/modules/andromeda-schema/examples/schema.rs
- contracts/modules/andromeda-schema/src/testing/mod.rs
- packages/andromeda-data-storage/src/lib.rs
- packages/andromeda-modules/src/lib.rs
🧰 Additional context used
📓 Learnings (1)
contracts/data-storage/andromeda-form/src/testing/mock_querier.rs (1)
Learnt from: crnbarr93
PR: andromedaprotocol/andromeda-core#576
File: contracts/data-storage/andromeda-graph/src/testing/mock_querier.rs:93-110
Timestamp: 2024-10-11T10:52:10.088Z
Learning: In the Rust file `contracts/data-storage/andromeda-graph/src/testing/mock_querier.rs`, panics caused by `unwrap()` are acceptable because the code is only used for testing.
🔇 Additional comments (37)
contracts/data-storage/andromeda-form/.cargo/config (1)
4-4: Verify schema example configuration.
The schema alias requires a corresponding example file in the examples directory.
✅ Verification successful
Schema example configuration is valid and properly set up
The verification confirms:
- Examples directory exists at the correct location
- Schema example file (
schema.rs) is present and properly configured with the required API definitions - The cargo config alias correctly points to this example
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify schema example configuration
# Test 1: Check if examples directory exists
echo "Checking for examples directory..."
fd -t d "examples" contracts/data-storage/andromeda-form
# Test 2: Look for schema example file
echo "Looking for schema example file..."
fd -t f "schema" contracts/data-storage/andromeda-form/examples
Length of output: 383
Script:
#!/bin/bash
# Let's verify the content of schema.rs to ensure it's properly configured
echo "Checking schema.rs content..."
cat contracts/data-storage/andromeda-form/examples/schema.rs
# Also check if there are any other cargo config references to this example
echo -e "\nChecking for other cargo config references..."
rg "schema.*example" contracts/data-storage/andromeda-form
Length of output: 534
contracts/data-storage/andromeda-form/src/lib.rs (2)
1-6: LGTM! Well-structured module organization.
The module structure follows a clean separation of concerns and includes all necessary components for a CosmWasm contract (contract, execute, query, state). The testing module is properly gated with the test configuration attribute.
8-9: LGTM! Good practice for test code isolation.
The mock module is correctly configured to be excluded from Wasm builds and only included when the testing feature is enabled. This ensures that test code doesn't bloat the production builds.
contracts/modules/andromeda-schema/src/lib.rs (1)
5-9: LGTM! Well-structured test configuration.
The test modules are properly configured:
- Test module is correctly gated with
#[cfg(test)] - Mock module is appropriately configured for non-wasm32 targets with testing feature
packages/andromeda-modules/src/schema.rs (2)
1-2: LGTM: Appropriate imports for contract message types
The imports correctly include the necessary dependencies for contract message attributes and serialization.
4-8: Add validation and documentation for schema_json_string
Consider the following improvements:
- Add input validation to ensure the schema_json_string is valid JSON
- Consider adding a maximum size limit to prevent excessive storage costs
- Add documentation comments explaining the expected JSON schema format and structure
contracts/modules/andromeda-schema/src/query.rs (1)
1-7: LGTM! Dependencies are well-organized.
The imports are appropriate and necessary for the schema validation functionality.
contracts/modules/andromeda-schema/src/testing/mock.rs (2)
1-16: LGTM! Well-organized imports and useful type alias.
The imports are logically grouped and the MockDeps type alias effectively simplifies the test code.
32-42: Verify sender permissions in tests.
The function should include test cases to verify that only authorized addresses (e.g., owner) can update the schema.
Let's verify the permission checks in the contract:
contracts/modules/andromeda-schema/src/mock.rs (3)
1-11: LGTM! Module configuration and imports are well-structured.
The conditional compilation attribute and imports are appropriate for a mock testing module.
38-57: LGTM! Funds handling is well-implemented.
The execute_update_schema method properly handles optional funds and message construction.
12-13: LGTM! Verify mock_ado macro implementation.
The struct design is clean and the mock_ado! macro usage appears correct.
✅ Verification successful
Let me gather more context about the MockContract trait to fully verify the macro implementation.
Mock implementation verified and provides correct trait functionality
The mock_ado! macro correctly implements:
MockContracttrait with proper generic constraints for Execute and Query messages- Address accessor method via the newtype pattern
- From conversion for convenient instantiation
The implementation aligns with testing requirements by providing contract execution capabilities through the MockApp integration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the mock_ado macro implementation to ensure it provides necessary trait implementations
rg -A 10 "macro_rules!\s+mock_ado"
Length of output: 860
Script:
#!/bin/bash
# Check the MockContract trait definition
rg -A 10 "trait\s+MockContract"
Length of output: 896
contracts/data-storage/andromeda-form/src/testing/mock_querier.rs (2)
1-16: LGTM! Well-organized imports and constant definition.
The imports are logically grouped and follow Rust best practices. The constant naming convention is appropriate for the testing context.
54-68: LGTM! Well-implemented error handling.
The raw_query implementation properly handles parsing errors and delegates to appropriate handlers. The error messages are clear and informative.
contracts/data-storage/andromeda-form/src/testing/mock.rs (3)
1-18: LGTM! Well-organized imports and mock setup.
The imports are logically grouped and the mock dependencies are properly set up following CosmWasm testing patterns.
1-183: Overall implementation looks solid!
The mock implementation provides comprehensive test coverage for the Form ADO contract operations. While there are opportunities for improvement in code organization and error handling, the core functionality is well-implemented and follows good testing practices.
68-135: Improve timestamp handling consistency across form operations.
Some form operation functions (submit_form, open_form, close_form) include timestamp handling while others (delete_submission, edit_submission) don't. Consider whether all operations should include timestamp validation for consistency.
Let's verify the timestamp requirements in the contract implementation:
Additionally, consider adding error context to improve debugging:
pub fn submit_form(/* params */) -> Result<Response, ContractError> {
let msg = ExecuteMsg::SubmitForm { data };
let info = mock_info(sender, &[]);
let mut env = mock_env();
env.block.time = Timestamp::from_nanos(timestamp_nanos);
- execute(deps, env, info, msg)
+ execute(deps, env, info, msg).map_err(|e| {
+ ContractError::Std(StdError::generic_err(format!(
+ "Failed to submit form: {}", e
+ )))
+ })
}contracts/data-storage/andromeda-form/src/mock.rs (2)
1-16: LGTM! Well-structured imports and proper test configuration.
The imports are comprehensive and the cfg attribute correctly ensures this code is only included in test builds.
189-210: LGTM! Well-implemented helper functions.
The helper functions are well-designed with:
- Proper use of trait bounds (
Into<String>) for flexibility - Clear parameter handling
- Good separation of concerns between contract creation and message construction
contracts/modules/andromeda-schema/src/execute.rs (1)
13-33: Verify the handling of action messages and responses
Confirm that call_action is appropriately used and that any side effects from actions are intended. Ensure that action messages are necessary for this contract's functionality.
contracts/modules/andromeda-schema/src/contract.rs (3)
27-58: Instantiation Function Implemented Correctly
The instantiate function properly initializes the contract and stores the provided schema. Error handling and state management are correctly implemented.
61-74: Execute Function Handles Messages Appropriately
The execute function correctly processes incoming messages by matching the ExecuteMsg variants. It delegates AMP messages to execute_amp_receive and others to handle_execute, ensuring appropriate handling.
77-83: Query Function Correctly Processes Queries
The query function effectively handles ValidateData and GetSchema queries, encoding responses properly. It defers unsupported queries to the base ADOContract, ensuring extensibility.
contracts/data-storage/andromeda-form/src/contract.rs (4)
100-114: Permissions for authorized addresses are correctly set
The implementation properly assigns permissions to authorized_addresses_for_submission, ensuring that only specified addresses can submit forms when required.
116-125: Event emission for custom notifications is accurate
The code correctly adds custom events when custom_key_for_notifications is provided, enabling integration with notification services like Telegram.
154-156: Unhandled QueryMsg variants are correctly delegated
The code appropriately delegates unhandled QueryMsg variants to ADOContract::default().query, ensuring that extended queries are managed by the base contract logic.
137-142: Ensure all ExecuteMsg variants are handled
Verify that all variants of ExecuteMsg are properly managed in the execute function to prevent unhandled cases.
#!/bin/bash
# Description: Check if all ExecuteMsg variants are handled in the execute function.
# Extract all variants from the ExecuteMsg enum
rg 'pub enum ExecuteMsg' -A 50 contracts/data-storage/andromeda-form/src/msg.rs | rg '^[ ]+[A-Za-z_]+'
# Ensure each variant is matched in the execute function
rg 'match msg' -A 20 contracts/data-storage/andromeda-form/src/contract.rscontracts/modules/andromeda-schema/src/testing/tests.rs (1)
1-310: Overall, well-structured tests and thorough schema validation
The test suite effectively covers various scenarios for schema validation, including valid inputs, missing fields, and incorrect data types. The use of the test_case macro improves readability and maintainability of tests.
contracts/data-storage/andromeda-form/src/testing/tests.rs (1)
586-599: Check for proper authorization in edit submission
In the test_submit_form_disallowed_multiple_submission_disallowed_edit test, lines 586-599, you correctly assert that editing a submission when edits are disallowed results in an error. Ensure that the unauthorized access is properly handled and consider adding more detailed assertions to validate the error content.
contracts/data-storage/andromeda-form/src/state.rs (5)
1-4: Imports are appropriate and necessary
All the required modules are imported correctly for state management and indexing functionalities. The use of andromeda_data_storage::form::SubmissionInfo and cw_storage_plus structures is appropriate.
6-13: State items are properly defined
The state items SCHEMA_ADO_ADDRESS, START_TIME, END_TIME, ALLOW_MULTIPLE_SUBMISSIONS, ALLOW_EDIT_SUBMISSION, and SUBMISSION_ID are correctly defined using Item. This ensures efficient storage and retrieval of singleton values in the contract's state.
15-23: Indexes in SubmissionIndexes are correctly set up
The SubmissionIndexes struct accurately defines MultiIndex for both submission_id and wallet_address. This facilitates efficient querying and retrieval of submission records based on these keys.
25-32: Implementation of IndexList for SubmissionIndexes is appropriate
The get_indexes method correctly returns an iterator over the indexes, integrating seamlessly with the storage system. This implementation adheres to the expected trait requirements.
34-48: submissions function correctly defines the indexed map
The submissions function properly creates an IndexedMap with the primary key (u64, Addr) and integrates the defined indexes. This setup ensures efficient storage, indexing, and querying of SubmissionInfo entries.
contracts/data-storage/andromeda-form/src/query.rs (3)
17-25: LGTM!
The get_schema function correctly retrieves the schema from the specified contract address.
27-42: LGTM!
The get_form_status function accurately determines the form status based on the current time and the saved start and end times.
56-65: LGTM!
The get_submission function correctly retrieves a specific submission based on the submission ID and wallet address.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
CHANGELOG.md (1)
33-33: Expand the changelog entry to better document this significant feature addition.
The current entry "Schema and Form ADO" is too brief for such a significant feature addition. Consider expanding it to highlight key capabilities and version information:
-Schema and Form ADO [(#591)](https://github.com/andromedaprotocol/andromeda-core/pull/591)
+Added Schema ADO (v1.0.0) and Form ADO (v1.0.0) [(#591)](https://github.com/andromedaprotocol/andromeda-core/pull/591)
+ - Schema ADO: JSON schema storage and validation capabilities
+ - Form ADO: Form submission management with schema validation
+ - Features include:
+ - Authorized submissions
+ - Configurable form timing
+ - Submission management (create/edit/delete)
+ - Comprehensive query system
| use cw_storage_plus::{Index, IndexList, IndexedMap, Item, MultiIndex}; | ||
|
|
||
| pub const SCHEMA_ADO_ADDRESS: Item<AndrAddr> = Item::new("schema_ado_address"); | ||
| pub const START_TIME: Item<Option<MillisecondsExpiration>> = Item::new("start_time"); |
There was a problem hiding this comment.
A bunch of these Items could be stored under one Item under the name Config
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- contracts/data-storage/andromeda-form/src/execute.rs (1 hunks)
🔇 Additional comments (4)
contracts/data-storage/andromeda-form/src/execute.rs (4)
154-157: Consider allowing users to delete their own submissions
Currently, only the owner or operator can delete submissions. Allowing users to delete their own submissions enhances data privacy and user control.
219-222: Fix potential unauthorized editing of submissions
The execute_edit_submission function relies on the wallet_address parameter provided by the caller, which can be manipulated to edit other users' submissions.
257-309: Simplify logic in execute_open_form function
The nested match statements in execute_open_form add unnecessary complexity. Simplifying the logic improves readability.
311-368: Simplify logic in execute_close_form function
Similar to execute_open_form, the execute_close_form function can be refactored to reduce complexity and enhance maintainability.
|
|
||
| pub fn get_submission( | ||
| deps: Deps, | ||
| submission_id: u64, |
There was a problem hiding this comment.
How are people supposed to remember the submission_id?
There was a problem hiding this comment.
They can query all submissions and can see the submission_id with their wallet addresses.
…dromeda-core into mdjakovic/schema-form
…rotocol/andromeda-core into mdjakovic/schema-form
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (11)
contracts/data-storage/andromeda-form/src/testing/mock_querier.rs (2)
18-49: Consider parameterizing the instantiation message.The function hardcodes values like "form" and "test" in the instantiation message. Consider making these parameters to increase test flexibility.
pub fn mock_dependencies_custom( contract_balance: &[Coin], + ado_type: Option<String>, + ado_version: Option<String>, ) -> OwnedDeps<MockStorage, MockApi, WasmMockQuerier> { // ... mock_info("sender", &[]), InstantiateMsg { - ado_type: "form".to_string(), - ado_version: "test".to_string(), + ado_type: ado_type.unwrap_or_else(|| "form".to_string()), + ado_version: ado_version.unwrap_or_else(|| "test".to_string()), kernel_address: MOCK_KERNEL_CONTRACT.to_string(), owner: None, },
83-105: Consider adding structured error handling for schema validation.The current implementation could benefit from more robust error handling and validation responses.
fn handle_schema_smart_query(&self, msg: &Binary) -> QuerierResult { - match from_json(msg).unwrap() { + match from_json(msg) { + Ok(query_msg) => match query_msg { SchemaQueryMsg::GetSchema {} => { // ... existing code ... } SchemaQueryMsg::ValidateData { data } => { // ... existing code ... } - _ => panic!("Unsupported Query"), + _ => SystemResult::Ok(ContractResult::Err("Unsupported query type".into())), + }, + Err(e) => SystemResult::Ok(ContractResult::Err(format!("Failed to parse query: {}", e).into())), } }contracts/modules/andromeda-schema/src/query.rs (2)
8-11: Consider adding input validation for storage access.While the function is well-structured, consider adding validation to ensure the storage is properly initialized before attempting to load the schema.
pub fn get_schema(storage: &dyn Storage) -> Result<GetSchemaResponse, ContractError> { + if !SCHEMA.may_load(storage)?.is_some() { + return Err(ContractError::CustomError { + msg: "Schema not initialized".to_string(), + }); + } let schema = SCHEMA.load(storage)?.to_string(); Ok(GetSchemaResponse { schema }) }
85-226: Add test cases for edge scenarios.The test suite is comprehensive but could benefit from additional edge cases:
- Empty arrays and objects
- Null values
- Maximum nesting depth
- Very large arrays/objects for performance testing
#[test] fn test_basic_type_matches_edge_cases() { // Empty array let schema = json!({ "type": "array", "items": { "type": "string" } }); let data = json!([]); assert!(basic_type_matches(&schema, &data)); // Empty object let schema = json!({ "type": "object", "properties": {}, "required": [] }); let data = json!({}); assert!(basic_type_matches(&schema, &data)); // Null values in non-required fields let schema = json!({ "type": "object", "properties": { "optional": { "type": "string" } } }); let data = json!({ "optional": null }); assert!(!basic_type_matches(&schema, &data)); }contracts/data-storage/andromeda-form/src/testing/tests.rs (6)
290-294: Remove commented out debug print statements.The code contains commented-out debug print statements that should be removed to maintain code cleanliness.
- // println!( - // "//==================== endtime: {:?} ====================//", - // end_time - // );
425-507: Consider parameterizing the test case with test_case macro.The test case
test_submit_form_allowed_multiple_submissioncontains multiple submissions with different test data. Consider using thetest_casemacro to make the test more maintainable and easier to extend with additional test cases.Example refactor:
#[test_case( vec![ ("user1", "valid_data1", 20000000000_u64), ("user1", "valid_data2", 30000000000_u64), // ... more test cases ]; "multiple valid submissions" )] fn test_submit_form_allowed_multiple_submission(test_cases: Vec<(&str, &str, u64)>) { // ... test setup ... for (user, data, timestamp) in test_cases { submit_form(deps.as_mut(), user, data.to_string(), timestamp).unwrap(); } // ... assertions ... }
656-662: Consider extracting the schema string to a constant.The schema JSON string is hardcoded in the test. Consider extracting it to a constant at the top of the file for better maintainability and reusability across tests.
+const TEST_SCHEMA: &str = "{\"properties\":{\"age\":{\"type\":\"number\"},\"name\":{\"type\":\"string\"}},\"required\":[\"name\",\"age\"],\"type\":\"object\"}"; let schema = query_schema(deps.as_ref()).unwrap(); assert_eq!( schema, GetSchemaResponse { - schema: "{\"properties\":{\"age\":{\"type\":\"number\"},\"name\":{\"type\":\"string\"}},\"required\":[\"name\",\"age\"],\"type\":\"object\"}".to_string(), + schema: TEST_SCHEMA.to_string(), } );
1-663: Comprehensive test coverage with room for improvements.The test file provides good coverage of the Form ADO functionality with:
- Parameterized tests for various form configurations
- Validation of form state transitions
- Error handling for invalid operations
- Multiple submission scenarios
However, consider adding tests for:
- Edge cases around timestamp boundaries
- Schema validation with invalid data formats
- Concurrent submission scenarios
- Form closure with pending submissions
508-511: Add descriptive assertion messages.The assertion for the number of submissions would be clearer with a descriptive message explaining the expected state.
- assert_eq!(all_submissions.len(), 7_usize); + assert_eq!( + all_submissions.len(), + 7_usize, + "Expected 7 submissions after successful submissions from multiple users" + );
282-289: Simplify start time validation logic.The start time validation logic can be simplified by using
mapinstead of explicit if-else.- let expected_saved_start_time = if START_TIME.load(&deps.storage).unwrap().is_some() { - Some(Milliseconds::from_nanos(execute_timestamp).plus_milliseconds(Milliseconds(1))) - } else { - None - }; + let expected_saved_start_time = START_TIME + .load(&deps.storage) + .unwrap() + .map(|_| Milliseconds::from_nanos(execute_timestamp).plus_milliseconds(Milliseconds(1)));contracts/modules/andromeda-schema/src/testing/tests.rs (1)
217-219: Consider parsing and comparing JSON structures instead of raw stringsComparing JSON data as raw strings can lead to brittle tests due to differences in formatting or property ordering. Parsing the JSON strings into
serde_json::Valueand comparing the resulting structures will make the test more robust and less sensitive to such changes.Apply this diff to improve the test:
fn test_query_schema() { let (deps, _) = proper_initialization(SCHEMA_INITIAL.to_string()); let query_res = query_schema(deps.as_ref()).unwrap(); let schema = query_res.schema; - assert_eq!( - schema, - "{\"properties\":{\"age\":{\"type\":\"number\"},\"name\":{\"type\":\"string\"}},\"required\":[\"name\",\"age\"],\"type\":\"object\"}".to_string() - ); + let expected_schema: serde_json::Value = serde_json::from_str(SCHEMA_INITIAL).unwrap(); + let actual_schema: serde_json::Value = serde_json::from_str(&schema).unwrap(); + assert_eq!(actual_schema, expected_schema); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
CHANGELOG.md(1 hunks)contracts/data-storage/andromeda-form/src/testing/mock_querier.rs(1 hunks)contracts/data-storage/andromeda-form/src/testing/tests.rs(1 hunks)contracts/modules/andromeda-schema/Cargo.toml(1 hunks)contracts/modules/andromeda-schema/src/contract.rs(1 hunks)contracts/modules/andromeda-schema/src/execute.rs(1 hunks)contracts/modules/andromeda-schema/src/query.rs(1 hunks)contracts/modules/andromeda-schema/src/testing/mock.rs(1 hunks)contracts/modules/andromeda-schema/src/testing/tests.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- CHANGELOG.md
- contracts/modules/andromeda-schema/Cargo.toml
- contracts/modules/andromeda-schema/src/contract.rs
- contracts/modules/andromeda-schema/src/execute.rs
- contracts/modules/andromeda-schema/src/testing/mock.rs
🔇 Additional comments (3)
contracts/data-storage/andromeda-form/src/testing/mock_querier.rs (1)
1-16: LGTM! Imports and constants are well organized.
The imports cover all necessary components for mocking and testing, and the constant follows naming conventions.
contracts/modules/andromeda-schema/src/query.rs (1)
1-7: LGTM! Dependencies are well-organized.
The imports are minimal and appropriate for the module's functionality.
contracts/data-storage/andromeda-form/src/testing/tests.rs (1)
348-348: Fix typo in error messages: 'opend' should be 'opened'.
Also applies to: 390-390
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)Cargo.toml(1 hunks)packages/andromeda-data-storage/src/lib.rs(1 hunks)packages/andromeda-modules/src/lib.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/andromeda-modules/src/lib.rs
- packages/andromeda-data-storage/src/lib.rs
- Cargo.toml
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
contracts/data-storage/andromeda-form/Cargo.toml (1)
6-6: Consider relaxing the Rust version requirementThe specified Rust version (1.75.0) is very recent. Consider using an older stable version (like 1.70.0) to ensure broader compatibility and easier development environment setup, unless there are specific features from 1.75.0 that are required.
contracts/modules/andromeda-schema/src/interface.rs (1)
1-6: Consider adding interface documentation for Form ADO integration.Since this Schema ADO will be used by the Form ADO (as mentioned in PR objectives), consider adding documentation comments that explain:
- How to properly reference and interact with this contract from other ADOs
- Best practices for schema validation integration
- Example usage patterns
This will help future developers understand the intended integration patterns.
contracts/data-storage/andromeda-form/src/interface.rs (1)
1-6: Consider event handling preparation for future enhancements.The PR objectives mention future plans to support custom keys in response events for notification services. While implementing the current interface, consider preparing the event emission structure to accommodate these future enhancements without requiring breaking changes.
Suggested considerations:
- Define a consistent event structure that can be extended
- Plan for backward compatibility when adding custom keys
- Consider implementing an event builder pattern
Would you like assistance in designing an extensible event structure?
contracts/modules/andromeda-schema/src/lib.rs (1)
1-4: LGTM! Well-structured module organization.The core module structure follows CosmWasm best practices with clear separation of concerns between contract entry points, execution logic, queries, and state management.
contracts/modules/andromeda-schema/src/contract.rs (1)
77-84: Consider explicit error handling for unsupported queriesThe current implementation silently delegates unsupported queries to ADOContract. Consider adding explicit error handling for unsupported queries to improve debugging and maintainability.
match msg { QueryMsg::ValidateData { data } => encode_binary(&validate_data(deps.storage, data)?), QueryMsg::GetSchema {} => encode_binary(&get_schema(deps.storage)?), - _ => ADOContract::default().query(deps, env, msg), + _ => { + // Log or handle unsupported query type explicitly + ADOContract::default().query(deps, env, msg) + } }contracts/data-storage/andromeda-form/src/contract.rs (2)
120-126: Consider makingnotification_serviceconfigurableCurrently, the
notification_serviceis hardcoded as"Telegram". To enhance flexibility and support multiple notification platforms, consider making thenotification_servicean input parameter or part of the configuration.Update the
InstantiateMsgstruct to includenotification_service:pub struct InstantiateMsg { // ... existing fields pub notification_service: Option<String>, }Then, modify the code to use the configurable
notification_service:if let Some(custom_key) = msg.custom_key_for_notifications { + let notification_service = msg.notification_service.unwrap_or_else(|| "Telegram".to_string()); response = response.add_event( cosmwasm_std::Event::new("custom_key") .add_attribute("custom_key", custom_key) - .add_attribute("notification_service", "Telegram"), + .add_attribute("notification_service", notification_service), ); }
35-129: Add documentation comments for public functionsConsider adding Rust documentation comments (
///) to the public functions (instantiate,execute,query,migrate,reply) to improve code readability and maintainability. This practice helps other developers understand the purpose and usage of these functions.Example:
/// Initializes the contract with the given parameters. pub fn instantiate( // function parameters ) -> Result<Response, ContractError> { // implementation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
contracts/data-storage/andromeda-form/Cargo.toml(1 hunks)contracts/data-storage/andromeda-form/src/contract.rs(1 hunks)contracts/data-storage/andromeda-form/src/interface.rs(1 hunks)contracts/data-storage/andromeda-form/src/lib.rs(1 hunks)contracts/modules/andromeda-schema/Cargo.toml(1 hunks)contracts/modules/andromeda-schema/src/contract.rs(1 hunks)contracts/modules/andromeda-schema/src/interface.rs(1 hunks)contracts/modules/andromeda-schema/src/lib.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/data-storage/andromeda-form/src/lib.rs
- contracts/modules/andromeda-schema/Cargo.toml
🔇 Additional comments (17)
contracts/data-storage/andromeda-form/Cargo.toml (1)
26-39: Verify schema validation capabilities
The dependencies seem aligned with the Form ADO requirements, but let's verify the schema validation capabilities of the chosen dependencies.
✅ Verification successful
Let me analyze the schema validation capabilities based on the search results. I'll generate a final response since I have enough information from the codebase search.
Schema validation capabilities are appropriate for Form ADO
The cw-json dependency is well-suited for the Form ADO's schema validation requirements:
- The codebase shows extensive use of schema validation patterns across different contracts, primarily using schemars for struct validation
- No other JSON schema validation crates are used in the workspace that would be more suitable
- The cw-json crate from the specified git repository is appropriate since:
- It provides JSON schema validation capabilities needed for form data validation
- It aligns with the CosmWasm ecosystem's patterns of schema validation
- Other contracts in the codebase use similar external dependencies for specialized validation needs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any other schema validation crates in use across the workspace
# that might be more suitable than cw-json
# Search for schema validation related dependencies in all Cargo.toml files
echo "Searching for schema validation crates in workspace:"
rg -l 'jsonschema|schema|validation' -g 'Cargo.toml'
# Check the specific implementation details of schema validation
echo -e "\nSearching for schema validation implementations:"
rg -A 5 'validate|schema' --type rust
Length of output: 67403
contracts/modules/andromeda-schema/src/interface.rs (3)
1-2: LGTM! Clean and focused imports.
The imports are well-organized and include all necessary types for the schema contract interface.
4-4: Verify CONTRACT_ID uniqueness across the Andromeda protocol.
While the ID "schema" is appropriate for this contract, we should ensure it doesn't conflict with other contract IDs in the protocol.
✅ Verification successful
CONTRACT_ID "schema" is unique across the Andromeda protocol
The search results confirm that the CONTRACT_ID "schema" is only used once in the codebase, specifically in the schema contract. All other contracts use distinct identifiers that clearly represent their respective purposes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for other occurrences of CONTRACT_ID definitions
# to ensure uniqueness across the protocol
# Search for CONTRACT_ID definitions
rg -g '!target/' 'pub const CONTRACT_ID: &str = ' -A 1
Length of output: 6001
6-6: Verify WASM binary existence and naming.
The contract interface references "andromeda_schema.wasm". Let's verify this binary is properly configured in the build system.
✅ Verification successful
WASM binary configuration is properly set up
The contract interface configuration is correct and consistent with the project setup:
- The contract ID "schema" is properly defined as a constant
- The Cargo.toml is configured with
cdylibtarget for WASM compilation - The WASM binary name follows the consistent naming pattern used across all Andromeda contracts
- The contract interface declaration matches other contracts in the ecosystem
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for WASM binary configuration
# Search for WASM binary name in Cargo.toml files
rg -g 'Cargo.toml' 'andromeda[_-]schema' -A 5
# Search for any references to this WASM name in build scripts
fd -g '*.{rs,sh,toml}' -X rg 'andromeda[_-]schema\.wasm'
Length of output: 606
Script:
#!/bin/bash
# Let's verify the build configuration and contract ID
# Check lib.rs for contract ID and wasm binary configuration
rg -g 'lib.rs' 'CONTRACT_ID' contracts/modules/andromeda-schema/ -B 2 -A 2
# Check if there are any other contract interface declarations for this contract
ast-grep --pattern 'contract_interface!($_, $_, $_)'
# Look for any build script configurations
fd -g 'build.rs' -X cat {}
Length of output: 8751
Script:
#!/bin/bash
# Let's check the contract ID definition and Cargo configuration
# Check for CONTRACT_ID constant in the schema contract
rg -g '*.rs' 'pub const CONTRACT_ID' contracts/modules/andromeda-schema/
# Check the Cargo.toml configuration for wasm target
cat contracts/modules/andromeda-schema/Cargo.toml
Length of output: 1202
contracts/data-storage/andromeda-form/src/interface.rs (3)
1-2: LGTM! Imports are well-organized and properly scoped.
The imports are correctly structured, bringing in the necessary types from both the data storage module and the standard Andromeda library.
4-4: Verify uniqueness of CONTRACT_ID across the Andromeda ecosystem.
While "form" is a descriptive and appropriate ID, we should ensure it doesn't conflict with existing contracts.
✅ Verification successful
Let me search for any other contract IDs to ensure we have a complete picture of existing IDs in the ecosystem.
CONTRACT_ID "form" is unique across the Andromeda ecosystem
After reviewing all CONTRACT_ID declarations in the codebase, I can confirm that the identifier "form" is only used once in the andromeda-form contract and does not conflict with any other contract IDs in the ecosystem. Each contract has its own unique identifier that clearly describes its purpose.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for other occurrences of CONTRACT_ID = "form"
# to ensure uniqueness across the codebase
echo "Searching for CONTRACT_ID declarations containing 'form'..."
rg -g '!target/' 'CONTRACT_ID.*=.*"form"'
Length of output: 271
Script:
#!/bin/bash
# Search for all CONTRACT_ID declarations in the codebase
echo "Searching for all CONTRACT_ID declarations..."
rg -g '!target/' 'CONTRACT_ID.*=.*".*"' -A 1
# Also search specifically in interface.rs files as they commonly define contract IDs
echo -e "\nSearching in interface.rs files..."
fd interface.rs --exec rg -l 'CONTRACT_ID'
Length of output: 8393
6-6: Verify the referenced WASM file path.
The contract interface references "andromeda_form.wasm". We should ensure this file will be generated with the correct name during the build process.
✅ Verification successful
WASM file name is correctly configured
The build configuration is correct and will generate the expected WASM file:
- The package name in Cargo.toml is "andromeda-form", which during build gets converted to "andromeda_form" (hyphens to underscores) as seen in the build script's
BUILD_TARGET=${CONTRACT//-/_}conversion. - The build script (
scripts/build.sh) handles the WASM file generation and correctly converts hyphens to underscores in the target name. - The generated WASM file will be placed in
artifacts/andromeda_form.wasm, matching exactly what's referenced in the contract interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the WASM file name configuration
# Check Cargo.toml for the package name
echo "Checking package name in Cargo.toml..."
rg -g 'Cargo.toml' 'name.*=.*"andromeda[_-]form"'
# Check for any other references to this WASM file
echo "Checking references to the WASM file..."
rg 'andromeda_form\.wasm'
Length of output: 452
Script:
#!/bin/bash
# Let's check the build configuration to see how the WASM file name is determined
# Check for any build scripts or configuration files that might handle WASM naming
echo "Checking for build configuration files..."
fd -e toml -e json -e sh -e rs build
# Check for any Cargo workspace configuration that might affect builds
echo "Checking workspace configuration..."
cat Cargo.toml
# Look for any build.rs files that might affect the output name
echo "Checking for build.rs files..."
fd build.rs
# Check for any other Rust files that might contain build configuration
echo "Looking for build-related code..."
rg -g '*.rs' 'wasm|build|artifact'
Length of output: 66182
Script:
#!/bin/bash
# Let's check the build configuration in the contract's directory
# Check Cargo.toml for build configuration
echo "Checking contract's Cargo.toml..."
cat contracts/data-storage/andromeda-form/Cargo.toml
# Check for any build scripts in the contract directory
echo "Checking for build scripts..."
fd -e sh -e rs build contracts/data-storage/andromeda-form/
# Check for any workspace build scripts that might affect WASM naming
echo "Checking workspace build scripts..."
cat scripts/build.sh
Length of output: 4789
contracts/modules/andromeda-schema/src/lib.rs (3)
5-6: LGTM! Proper test module configuration.
The testing module is correctly configured with conditional compilation, ensuring test code doesn't bloat the production binary.
8-9: LGTM! Excellent mock module configuration.
The mock module uses sophisticated conditional compilation to ensure it's only available during native testing while being completely excluded from WASM builds. This is a best practice for CosmWasm contract testing.
11-14: Verify SchemaContract interface implementation.
The module structure looks good with proper encapsulation. However, let's verify that the SchemaContract interface correctly implements all required functionality from the PR objectives.
✅ Verification successful
Interface implementation matches contract requirements
The SchemaContract interface correctly maps to the contract's implementation. The contract provides all required functionality:
- Schema validation via
ValidateDataquery - Schema updates through
UpdateSchemaexecute message - Schema queries via
GetSchemaquery message
The interface is properly encapsulated and the contract implementation in contract.rs handles all the required message types defined in schema.rs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify SchemaContract interface implementation matches requirements
# Check for required schema validation functionality
ast-grep --pattern 'fn validate_data($$$)'
# Check for schema update capabilities
ast-grep --pattern 'fn update_schema($$$)'
# Check for schema query functionality
ast-grep --pattern 'fn query_schema($$$)'
Length of output: 133
Script:
#!/bin/bash
# Let's try a different approach to find the interface implementation
# First, let's see the interface module content
cat contracts/modules/andromeda-schema/src/interface.rs
# As backup, let's also search for trait or struct definitions
ast-grep --pattern 'trait SchemaContract {
$$$
}'
ast-grep --pattern 'impl SchemaContract for $_ {
$$$
}'
Length of output: 443
Script:
#!/bin/bash
# Let's check the contract_interface macro and message types
# Check the contract_interface macro definition
rg -A 5 "macro_rules! contract_interface"
# Check the message types used in the interface
cat contracts/modules/andromeda-schema/src/schema.rs
# Also check if there are any direct implementations
rg -g '!target' -g '!*.md' "impl.*SchemaContract"
Length of output: 1852
Script:
#!/bin/bash
# Let's check the message types in the andromeda_modules crate
# since the interface uses them from there
# Find and check the schema module in andromeda_modules
fd -t f schema.rs
# Once found, let's look at its content to verify the message types
cat packages/modules/src/schema.rs
Length of output: 2552
Script:
#!/bin/bash
# Let's try the correct path for modules
cat packages/andromeda-modules/src/schema.rs
# Also check the contract implementation
cat contracts/modules/andromeda-schema/src/contract.rs
# And check if there are any test files that might show usage
fd -t f -e rs . contracts/modules/andromeda-schema/src/
Length of output: 4383
contracts/modules/andromeda-schema/src/contract.rs (3)
61-75: LGTM!
The execute function implementation follows good practices with proper context management and clear message handling separation.
86-89: LGTM!
The migrate function correctly delegates to ADOContract with proper contract information.
51-54: 🛠️ Refactor suggestion
Optimize JSON parsing and conversion
The current implementation performs unnecessary string conversions.
-let schema_json_value: Value =
- from_str(schema_json_string.as_str()).map_err(|_| ContractError::CustomError {
- msg: "Invalid JSON Schema".to_string(),
- })?;
-let schema_json = JSON::try_from(schema_json_value.to_string().as_str()).unwrap();
+let schema_json = JSON::try_from(schema_json_string.as_str())
+ .map_err(|e| ContractError::CustomError {
+ msg: format!("Invalid JSON Schema: {}", e),
+ })?;Likely invalid or redundant comment.
contracts/data-storage/andromeda-form/src/contract.rs (4)
28-30: 🛠️ Refactor suggestion
Update CONTRACT_NAME to match the package name without the crates.io: prefix
To maintain consistency with the package metadata, the CONTRACT_NAME should align with the package name defined in Cargo.toml, which is "andromeda-form". The prefix "crates.io:" is unnecessary and can be removed.
Apply this diff to update CONTRACT_NAME:
-const CONTRACT_NAME: &str = "crates.io:andromeda-form";
+const CONTRACT_NAME: &str = "andromeda-form";43-43: 🛠️ Refactor suggestion
Avoid unnecessary cloning of env
The env variable is cloned multiple times, which is redundant and can impact performance. Consider borrowing env or passing references to functions to improve efficiency.
Apply this diff to remove unnecessary clones:
Line 43:
-env.clone(),
+&env,Line 63:
-get_and_validate_start_time(&env.clone(), Some(start_time))?
+get_and_validate_start_time(&env, Some(start_time))?Line 69:
-let time_res = get_and_validate_start_time(&env.clone(), Some(end_time));
+let time_res = get_and_validate_start_time(&env, Some(end_time));Also applies to: 61-61, 63-63, 69-69
68-84: 🛠️ Refactor suggestion
Refactor end_time validation to avoid using unwrap()
Simplify the error handling logic by using pattern matching instead of unwrap(). This enhances code safety and readability by eliminating potential panics from unwrap() usage.
Refactored code:
let end_time = match msg.form_config.end_time {
Some(end_time) => match get_and_validate_start_time(&env, Some(end_time)) {
Ok(validated_time) => Some(milliseconds_from_expiration(validated_time.0)?),
Err(_) => {
let current_time = Milliseconds::from_nanos(env.block.time.nanos()).milliseconds();
let current_height = env.block.height;
return Err(ContractError::CustomError {
msg: format!(
"End time in the past. current_time {:?}, current_block {:?}",
current_time, current_height
),
});
},
},
None => None,
};75-81: 🛠️ Refactor suggestion
Define a specific error variant for EndTimeInPast
Instead of using ContractError::CustomError, create a dedicated error variant ContractError::EndTimeInPast for clearer and more structured error handling.
Add the new error variant in your ContractError enum:
EndTimeInPast {
current_time: u64,
current_height: u64,
},Update the error return statement:
return Err(ContractError::EndTimeInPast {
current_time,
current_height,
});
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
contracts/data-storage/andromeda-form/src/testing/tests.rs (1)
345-345: Fix typo in error message: 'opend' should be 'opened'There's a typo in the error messages on lines 345 and 387 where "opend" should be "opened". Please correct the typo for clarity.
Apply this diff to fix the typo:
- msg: format!("Not opened yet. Will be opend at {:?}", Milliseconds::from_nanos(2000000000000_u64)), + msg: format!("Not opened yet. Will be opened at {:?}", Milliseconds::from_nanos(2000000000000_u64)),Also applies to: 387-387
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
contracts/data-storage/andromeda-form/src/contract.rs(1 hunks)contracts/data-storage/andromeda-form/src/execute.rs(1 hunks)contracts/data-storage/andromeda-form/src/query.rs(1 hunks)contracts/data-storage/andromeda-form/src/state.rs(1 hunks)contracts/data-storage/andromeda-form/src/testing/tests.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- contracts/data-storage/andromeda-form/src/state.rs
🔇 Additional comments (11)
contracts/data-storage/andromeda-form/src/contract.rs (3)
26-26: Update CONTRACT_NAME to match the package name
The CONTRACT_NAME constant is currently set to "crates.io:andromeda-form", but it should match the package name "andromeda-form" as defined in Cargo.toml. Please remove the crates.io: prefix for consistency.
39-41: Avoid unnecessary cloning of env
The env variable is cloned when it's not necessary. Consider borrowing env or passing references to improve performance and reduce unnecessary cloning.
72-78: Define a specific error variant for EndTimeInPast
Instead of using ContractError::CustomError with a message, define a dedicated error variant like ContractError::EndTimeInPast to provide clearer error handling and messaging.
contracts/data-storage/andromeda-form/src/execute.rs (5)
109-133: Optimize submission existence check when multiple submissions are disallowed
When multiple submissions are not allowed, the current implementation retrieves up to MAX_LIMIT submissions to check if the user has already submitted. This could be inefficient and miss submissions beyond the limit. Consider optimizing the check by using may_load to check for the existence of any submission by the user.
148-177: Allow users to delete their own submissions
Currently, only the owner or operator can delete submissions. Consider allowing users to delete their own submissions to give them control over their data and comply with data privacy norms.
217-227: Fix potential unauthorized editing of submissions
The execute_edit_submission function relies on the wallet_address parameter provided by the caller to identify the submission owner, which can be exploited to edit others' submissions. Retrieve the wallet address from the stored submission and compare it to the sender's address to ensure only the submission owner can edit their submission.
261-333: Simplify the logic in execute_open_form
The nested matches in execute_open_form make the code complex. Refactor the function to simplify the logic for better readability and maintenance.
333-382: Simplify the logic in execute_close_form
Similar to execute_open_form, the execute_close_form function contains complex nested matches. Refactor to simplify the logic and improve code clarity.
contracts/data-storage/andromeda-form/src/query.rs (1)
48-55: Avoid using unwrap() on storage iteration results
Using unwrap() on results from storage iterations can cause the contract to panic if an error occurs during iteration. Handle potential errors by propagating them appropriately.
Consider modifying the code as follows:
let all_submissions: Vec<SubmissionInfo> = submissions()
.idx
.submission_id
.range(storage, None, None, cosmwasm_std::Order::Ascending)
- .map(|r| r.unwrap().1) // Extract the `SubmissionInfo` from the result
- .collect();
+ .map(|res| res.map(|(_, submission_info)| submission_info))
+ .collect::<Result<Vec<_>, _>>()?;contracts/data-storage/andromeda-form/src/testing/tests.rs (2)
110-128: Avoid comparing errors with dynamic messages in tests
Comparing errors that include dynamically formatted messages can make tests brittle. Match on the error type and use assertions to check for specific patterns in the error message.
344-347: Avoid comparing errors with dynamic messages in tests
In test_failed_close_form, avoid comparing errors with dynamic messages. Instead, match on the error type and assert on relevant parts of the message.
…-core into mdjakovic/schema-form
| } | ||
| Ok(()) | ||
| } | ||
| None => Err(ContractError::CustomError { |
There was a problem hiding this comment.
Shouldn't we consider this that the user wants the form to be opened from the start?
There was a problem hiding this comment.
The start time will be set at instantiation or only owner and operator can open/close the form.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
contracts/data-storage/andromeda-form/src/query.rs (2)
13-21: Consider enhancing error handling for schema queries.The function could benefit from more specific error handling to distinguish between different failure scenarios (e.g., invalid contract address, schema not found, etc.).
Consider applying this diff:
pub fn get_schema(deps: Deps) -> Result<GetSchemaResponse, ContractError> { let schema_ado_address = SCHEMA_ADO_ADDRESS.load(deps.storage)?; + let contract_addr = schema_ado_address + .get_raw_address(&deps) + .map_err(|e| ContractError::InvalidAddress { reason: e.to_string() })?; let res: SchemaResponse = deps.querier.query(&QueryRequest::Wasm(WasmQuery::Smart { - contract_addr: schema_ado_address.get_raw_address(&deps)?.into_string(), + contract_addr: contract_addr.into_string(), msg: encode_binary(&SchemaQueryMsg::GetSchema {})?, - }))?; + })).map_err(|e| ContractError::SchemaQueryError { reason: e.to_string() })?; let schema = res.schema; Ok(GetSchemaResponse { schema }) }
49-58: Consider adding specific error for missing submissions.The function could provide more informative errors when a submission is not found.
Consider applying this diff:
pub fn get_submission( deps: Deps, submission_id: u64, wallet_address: AndrAddr, ) -> Result<GetSubmissionResponse, ContractError> { let wallet_address = wallet_address.get_raw_address(&deps)?; let submission = - submissions().may_load(deps.storage, &(submission_id, wallet_address.clone()))?; + submissions() + .may_load(deps.storage, &(submission_id, wallet_address.clone()))? + .ok_or_else(|| ContractError::SubmissionNotFound { + id: submission_id, + address: wallet_address, + })?; - Ok(GetSubmissionResponse { submission }) + Ok(GetSubmissionResponse { + submission: Some(submission), + }) }contracts/data-storage/andromeda-form/src/testing/tests.rs (2)
164-164: Use More Specific Error AssertionsWhen asserting errors in tests (line 164), consider using pattern matching to assert on error variants rather than comparing the entire error objects. This enhances test resilience to changes in error messages.
631-632: Use Specific Error MatchingIn
test_submit_form_disallowed_multiple_submission_allowed_edit, the test asserts equality on the entire error object (line 631). Consider matching on the error type to make tests less fragile.Example refactor:
match res { ContractError::Unauthorized {} => { /* Test passes */ }, _ => panic!("Unexpected error type"), }contracts/data-storage/andromeda-form/src/state.rs (1)
11-17: Add documentation comments to theConfigstruct fieldsIncluding documentation comments for each field in the
Configstruct would improve code readability and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)contracts/data-storage/andromeda-form/src/execute.rs(1 hunks)contracts/data-storage/andromeda-form/src/query.rs(1 hunks)contracts/data-storage/andromeda-form/src/state.rs(1 hunks)contracts/data-storage/andromeda-form/src/testing/tests.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🔇 Additional comments (19)
contracts/data-storage/andromeda-form/src/query.rs (3)
1-12: LGTM! Imports are well-organized and necessary.
The imports are logically grouped and all are being used in the implementation.
23-35: LGTM! Clean implementation with proper validation.
The function correctly determines form status using the validation helper and handles errors appropriately.
37-47: Avoid using unwrap() on storage iteration results to prevent panics.
Using unwrap() on results from storage iterations can cause the contract to panic if an error occurs during iteration.
contracts/data-storage/andromeda-form/src/testing/tests.rs (4)
1-660: Comprehensive and Well-Structured Test Suite
The test suite is comprehensive and covers a wide range of scenarios for form instantiation, opening, closing, and submission. The use of the test_case macro enhances readability and maintainability.
110-128: 🛠️ Refactor suggestion
Avoid Comparing Errors with Dynamic Messages in Tests
In the test_invalid_instantiation function, the tests compare errors that include dynamically formatted messages (lines 110-128). This approach can lead to brittle tests because any change in the message formatting or content can cause the test to fail, even if the error type is correct.
Consider matching on the error type and validating the error messages using pattern matching or assertions that check for substrings. This will make the tests more resilient to changes in error messages.
Example refactor:
let err = invalid_initialization(...).unwrap_err();
match err {
ContractError::StartTimeInThePast { .. } => { /* Further assertions if needed */ },
_ => panic!("Unexpected error type"),
}209-210: 🛠️ Refactor suggestion
Avoid Comparing Errors with Dynamic Messages in Tests
Similarly, in the test_failed_open_form function, the test compares errors with dynamic messages (lines 209-210). To prevent brittle tests, match on the error type and assert on relevant parts of the error message if necessary.
Example refactor:
let err = open_form(...).unwrap_err();
match err {
ContractError::CustomError { msg } => {
assert!(msg.contains("Already opened"));
},
_ => panic!("Unexpected error type"),
}419-420: 🛠️ Refactor suggestion
Avoid Comparing Errors with Dynamic Messages in Tests
In test_failed_close_form (lines 419-420), the test compares errors containing dynamic messages. To improve test stability, match on the error variant and use assertions for dynamic content.
contracts/data-storage/andromeda-form/src/state.rs (5)
1-5: Import statements are appropriate and necessary
The imported modules and types are correctly included for the functionality provided.
7-9: Consider consolidating constants into the Config struct
The constants SCHEMA_ADO_ADDRESS and SUBMISSION_ID could be included within the Config struct to reduce the number of storage variables and simplify state management.
19-27: Index definitions are correctly set up
The SubmissionIndexes struct properly defines the multi-indexes for efficient querying of submissions.
29-36: Implementation of IndexList for SubmissionIndexes is correct
The get_indexes function accurately returns an iterator over the indexes, complying with the required trait implementation.
38-52: submissions function correctly initializes IndexedMap with indexes
The submissions function appropriately initializes the IndexedMap with the defined indexes, ensuring efficient data retrieval.
contracts/data-storage/andromeda-form/src/execute.rs (7)
263-316: Simplify logic in execute_open_form
The current implementation contains complex nested match statements, which can be simplified for better readability and maintainability.
Consider refactoring the function to reduce complexity:
pub fn execute_open_form(ctx: ExecuteContext) -> Result<Response, ContractError> {
nonpayable(&ctx.info)?;
let sender = ctx.info.sender;
ensure!(
ADOContract::default().is_owner_or_operator(ctx.deps.storage, sender.as_ref())?,
ContractError::Unauthorized {}
);
let mut config = CONFIG.load(ctx.deps.storage)?;
let current_time = Milliseconds::from_nanos(ctx.env.block.time.nanos());
let start_time = current_time.plus_milliseconds(Milliseconds(1));
match config.start_time {
Some(saved_start_time) if saved_start_time > start_time => {
config.start_time = Some(start_time);
CONFIG.save(ctx.deps.storage, &config)?;
}
Some(saved_start_time) => {
return Err(ContractError::CustomError {
msg: format!("Form is already open since {:?}", saved_start_time),
});
}
None => {
config.start_time = Some(start_time);
CONFIG.save(ctx.deps.storage, &config)?;
}
}
// Clear end_time if necessary
if let Some(saved_end_time) = config.end_time {
if start_time > saved_end_time {
config.end_time = None;
CONFIG.save(ctx.deps.storage, &config)?;
}
}
let response = Response::new()
.add_attribute("method", "open_form")
.add_attribute("sender", sender);
Ok(response)
}339-394: Simplify logic in execute_close_form
Similar to execute_open_form, the execute_close_form function can be simplified to improve readability and reduce complexity.
Consider the following refactored code:
pub fn execute_close_form(ctx: ExecuteContext) -> Result<Response, ContractError> {
nonpayable(&ctx.info)?;
let sender = ctx.info.sender;
ensure!(
ADOContract::default().is_owner_or_operator(ctx.deps.storage, sender.as_ref())?,
ContractError::Unauthorized {}
);
let mut config = CONFIG.load(ctx.deps.storage)?;
let current_time = Milliseconds::from_nanos(ctx.env.block.time.nanos());
let end_time = current_time.plus_milliseconds(Milliseconds(1));
match config.start_time {
Some(saved_start_time) if saved_start_time <= end_time => {
match config.end_time {
Some(saved_end_time) if saved_end_time > end_time => {
config.end_time = Some(end_time);
CONFIG.save(ctx.deps.storage, &config)?;
}
Some(saved_end_time) => {
return Err(ContractError::CustomError {
msg: format!("Form is already closed since {:?}", saved_end_time),
});
}
None => {
config.end_time = Some(end_time);
CONFIG.save(ctx.deps.storage, &config)?;
}
}
}
Some(saved_start_time) => {
return Err(ContractError::CustomError {
msg: format!("Form has not been opened yet. Scheduled to open at {:?}", saved_start_time),
});
}
None => {
return Err(ContractError::CustomError {
msg: "Form has not been opened yet".to_string(),
});
}
}
let response = Response::new()
.add_attribute("method", "close_form")
.add_attribute("sender", sender);
Ok(response)
}188-189: Reconsider requiring the form to be open for editing submissions
The execute_edit_submission function checks if the form is currently open before allowing edits. This might prevent users from correcting their submissions after the form has closed.
Ensure that restricting edits to open forms aligns with the intended functionality and user experience.
100-124: 🛠️ Refactor suggestion
Optimize submission existence check when multiple submissions are disallowed
When allow_multiple_submissions is false, the current implementation retrieves up to MAX_LIMIT submissions to check if the user has already submitted. This approach is inefficient and may not cover submissions beyond the limit.
Apply this diff to optimize the check by retrieving only a single submission:
- let submissions_by_address: Vec<SubmissionInfo> = submissions()
- .idx
- .wallet_address
- .prefix(sender.clone())
- .range(ctx.deps.storage, None, None, cosmwasm_std::Order::Ascending)
- .take(MAX_LIMIT as usize)
- .map(|r| r.unwrap().1)
- .collect();
- if submissions_by_address.is_empty() {
+ if submissions()
+ .idx
+ .wallet_address
+ .prefix(sender.clone())
+ .range(ctx.deps.storage, None, None, cosmwasm_std::Order::Ascending)
+ .next()
+ .is_none()
+ {
submissions().save(
ctx.deps.storage,
&(new_id.u64(), sender.clone()),
&SubmissionInfo {
submission_id: new_id.u64(),
wallet_address: sender.clone(),
data,
},
)?;
SUBMISSION_ID.save(ctx.deps.storage, &new_id)?;
} else {
return Err(ContractError::CustomError {
msg: "Multiple submissions are not allowed".to_string(),
});
}146-149: 🛠️ Refactor suggestion
Allow users to delete their own submissions
Currently, only the owner or operator can delete submissions. Consider allowing users to delete their own submissions to give them more control over their data and comply with data privacy regulations.
Modify the authorization logic to permit submission owners to delete their own submissions:
ensure!(
- ADOContract::default().is_owner_or_operator(ctx.deps.storage, sender.as_ref())?,
+ ADOContract::default().is_owner_or_operator(ctx.deps.storage, sender.as_ref())?
+ || sender == wallet_address.get_raw_address(&ctx.deps.as_ref())?,
ContractError::Unauthorized {}
);211-213:
Fix potential unauthorized editing of submissions
The execute_edit_submission function relies on the wallet_address parameter provided by the caller to identify the submission owner. This can be exploited by malicious users to edit other users' submissions by supplying a different wallet_address.
To prevent unauthorized editing, retrieve the wallet address from the stored submission and compare it to the sender's address:
let wallet_address = wallet_address.get_raw_address(&ctx.deps.as_ref())?;
if submissions()
.may_load(ctx.deps.storage, &(submission_id, wallet_address.clone()))?
.is_some()
{
- ensure!(
- wallet_address.clone() == sender.clone(),
- ContractError::Unauthorized {}
- );
+ ensure!(
+ wallet_address == sender,
+ ContractError::Unauthorized {}
+ );204-213:
Prevent unauthorized access in execute_edit_submission
The function currently allows any user to attempt editing a submission by specifying a wallet_address. This can lead to unauthorized access if the wallet_address does not match the sender.
Modify the code to use the stored wallet_address from the submission and ensure only the owner can edit it:
let submission = submissions()
.load(ctx.deps.storage, &(submission_id, wallet_address.clone()))
.map_err(|_| ContractError::CustomError {
msg: format!(
"Submission does not exist - Submission_id {:?}, Wallet_address {:?}",
submission_id, wallet_address
),
})?;
ensure!(
- wallet_address.clone() == sender.clone(),
+ submission.wallet_address == sender,
ContractError::Unauthorized {}
);Likely invalid or redundant comment.
Motivation
1. Schema ADO
Schema ADO stores a schema and has a query that can validate a data against the schema
2. Form ADO
Ado that can take submissions against a form. Form will be a schema ado that can validate submission against a valid json schema.
Implementation
1. Schema ADO
2. Form ADO
Testing
Unit test case are added.
Version Changes
Schema ADO: 1.0.0
Form ADO: 1.0.0
Notes
https://hackmd.io/@ECR5wF6JRgCIEKiGLzIABQ/SklY1wtHJJl
Future work
Form ADO: Want to attach custom key in response events provided by user in instantiatiate so that he can build his own service/telegram bot that can send push notification on each submission. This will go in response.add_event().
Summary by CodeRabbit
New Features
Documentation
Tests