feat(examples): Gnofundme #2085
Conversation
…n getters, reshaped testfiles
…cussion about the project
github-actions
bot
added
the
🧾 package/realm
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2085 +/- ##
==========================================
- Coverage 54.95% 54.31% -0.65%
==========================================
Files 481 434 -47
Lines 67407 64242 -3165
==========================================
- Hits 37044 34891 -2153
+ Misses 27341 26534 -807
+ Partials 3022 2817 -205 ☔ View full report in Codecov by Sentry. |
| contributors []*Contributor | ||
| } | ||
|
|
||
| func NewCampaign(title string, description string, goal uint, begin time.Time, deadline time.Time, owner *User, beneficiary *User) *Campaign { |
There was a problem hiding this comment.
How will you check if the string variables aren't empty? I think it would be great if you add some conditionals to avoid this issue. Maybe a minimum of 5 chars for the title, 10 for description, a goal of more than 0, etc.
There was a problem hiding this comment.
Hey, I was wondering if I add my checks on the realm (can cost more gas) or in the front (react), is it secure to do it on the front?
There was a problem hiding this comment.
If you have a safe testing of your front, it should be ok :-)
| current: 0, | ||
| begin: begin, | ||
| deadline: deadline, | ||
| owner: owner, |
There was a problem hiding this comment.
Won't be the owner the same as the caller? If not, shouldn't we add an extra requirement to avoid people requesting funds in the name of others?
There was a problem hiding this comment.
you can see here the different types of campaigns I found on the net. So for some campaigns, someone car create a campaign for another person or organism. But I still need to figure out how do some verification on that cases.
There was a problem hiding this comment.
I agree with you about campaigns for 3p, but you should have a mechanism to avoid people to abuse about the image of others to exploit a campaign.
| users = avl.NewTree() | ||
| } | ||
|
|
||
| func NewUser(name string, email string) string { |
There was a problem hiding this comment.
What happen if the variables are empty? Also you can change the input part from
(name string, email string) to (name, email string)
since both are strings (but that's a matter of taste)
There was a problem hiding this comment.
Same thing here for verification, the other issue is as we're talking about web3, a user is supposed to be identified only by his address, but I may use profile pkg, who is a work in progress, to identify people. I'll need a bunch of people's opinions to take decision on how to handle this.
| "time" | ||
|
|
||
| "gno.land/p/demo/avl" | ||
| pkg "gno.land/p/demo/gnofundme" |
There was a problem hiding this comment.
I think this name pkg could be better called to avoid confusion since it's not the only package you're calling. This would help the readability of your code.
There was a problem hiding this comment.
thanks for the advise, I'll modify it before the final version.
| // the avl.tree key will be address to simplify the fetching | ||
| users.Set(caller.String(), user) | ||
|
|
||
| user = pkg.NewUser("user2", "email2", beneficiary) |
There was a problem hiding this comment.
Same as previous with the other user
| } | ||
|
|
||
| // verify that the benefeciary exists | ||
| b, ok := users.Get(beneficiary.String()) |
There was a problem hiding this comment.
It will exist because you created it in line 49, but you're doing the order in reverse. You should first check the user exists, and if it doesn't, create a user for that beneficiary. Same as with the owner.
| // get the user if he exists | ||
| user, ok := users.Get(userAddress.String()) | ||
| if !ok { | ||
| panic("user is not registered") |
There was a problem hiding this comment.
Why panic here but not in line 84? I think it would be better if you add panic in all the possible errors that could happen.
There was a problem hiding this comment.
thanks, didn't pay attention. (modified)
| parts := strings.Split(path, "/") | ||
| switch { | ||
| case len(parts) == 2 && parts[0] == "campaign": | ||
| return renderCampaign(parts[1]) |
There was a problem hiding this comment.
It would be a good practice (and a better customer experience) if you first check if that campaign exists before rendering something that couldn't even be an id for a campaign. (Imagine someone puts campaign/foo).
There was a problem hiding this comment.
do you think what I did is good or not?
There was a problem hiding this comment.
Sorry, my bad. This is correct then.
| var b bytes.Buffer | ||
| c := campaign.(*pkg.Campaign) | ||
| b.WriteString( | ||
| ufmt.Sprintf( |
There was a problem hiding this comment.
Maybe changing this ufmt.Sprintf into 1 line instead of 3? To avoid a long path of 42 lines doing the same and reduce it into 18 (less than the half)
There was a problem hiding this comment.
If you mean the new lines after "ufmt.Sprintf(", I understand. It's because I copied a code as it is from another example. But I'll make the modifications.
GnoFundMe is a fundraising/crowdfunding platform built with React, TypeScript and Tailwind CSS.
It allows you to connect your Adena Wallet to identify and make actions/transactions.
I created this PR to open a space for discussion/suggestions from anyone that wants to participate and give ideas about this project, the repo of the project with the front is here.
I have this miro where I put my ideas, anyone is welcome to modify and suggest things. Feel free to participate.
Contributors' checklist...
BREAKING CHANGE: xxxmessage was included in the description