Add optional img url to dao #138
Conversation
…d make sure write tests that make sure updating still works
… need to work on updating config to do the same. finally add tests for both criterias
|
Will Squash and Merge. |
contracts/cw3-dao/src/contract.rs
Outdated
| @@ -40,6 +41,11 @@ const DEFAULT_LIMIT: u32 = 10; | |||
| const INSTANTIATE_GOV_TOKEN_REPLY_ID: u64 = 0; | |||
| const INSTANTIATE_STAKING_CONTRACT_REPLY_ID: u64 = 1; | |||
|
|
|||
| pub fn validate_image_url(url: &String) -> bool { | |||
There was a problem hiding this comment.
public to make it easier to test and maybe re-use later
contracts/cw3-dao/src/contract.rs
Outdated
| @@ -51,13 +57,25 @@ pub fn instantiate( | |||
|
|
|||
| msg.threshold.validate()?; | |||
|
|
|||
| match &msg.image_url { | |||
There was a problem hiding this comment.
since this is done twice we can pull this out into a function, tho maybe too much rn
There was a problem hiding this comment.
Rust's Option has a map method for making this sort of stuff easier. Here's an example using it. If you modify validate_image_url to return Result<(), ContractError> something like this will do:
msg.img_url.map(validate_image_url)?;
contracts/cw3-dao/src/tests.rs
Outdated
| fn test_validate_image_url() { | ||
| assert_eq!( | ||
| validate_image_url(&"https://fefeqfq.com/imgur.jpg".to_string()), | ||
| true | ||
| ); |
There was a problem hiding this comment.
pulled this out here, otherwise we'd have a lot of code to test each individual type of string + all the other code for msg
|
Will fix clippy once overall code logic gets approved! |
contracts/cw3-dao/src/contract.rs
Outdated
| @@ -51,13 +57,25 @@ pub fn instantiate( | |||
|
|
|||
| msg.threshold.validate()?; | |||
|
|
|||
| match &msg.image_url { | |||
There was a problem hiding this comment.
Rust's Option has a map method for making this sort of stuff easier. Here's an example using it. If you modify validate_image_url to return Result<(), ContractError> something like this will do:
msg.img_url.map(validate_image_url)?;
contracts/cw3-dao/src/contract.rs
Outdated
| @@ -40,6 +41,11 @@ const DEFAULT_LIMIT: u32 = 10; | |||
| const INSTANTIATE_GOV_TOKEN_REPLY_ID: u64 = 0; | |||
| const INSTANTIATE_STAKING_CONTRACT_REPLY_ID: u64 = 1; | |||
|
|
|||
| pub fn validate_image_url(url: &String) -> bool { | |||
| let re = Regex::new(r"(https:)([/|.|\w|\s|-])*\.(?:jpg|gif|png)").unwrap(); | |||
There was a problem hiding this comment.
Compiling a regex here makes me a little nervous as this is reasonably computationally expensive. I almost wonder if we should do validation here at all? Seems possible that frontends should be charged with determining if this is an image they can display.
There was a problem hiding this comment.
Agree, the frontend could have additional validation for uploading (making sure link is valid IMG) and then when the link if being grabbed.
|
@ezekiiel Updated code (no longer doing regex). |
|
ACK, will update |
|
@JakeHartnell added changes 🙏 |
Amazing! Can you squash these commits into some meaningful messages? Then we're good to go. 🚀 Thanks for this. |
|
Ughhhhh, annoying not having branch protection rules. 😦 |
No description provided.