search

BuidlGuidl / grants.buidlguidl.com

https://grants-bg.vercel.app
MIT License
3 stars 3 forks source link

Allow admin edit grants #94

Closed technophile-04 closed 6 months ago

technophile-04 commented 6 months ago

Description

https://github.com/BuidlGuidl/grants.buidlguidl.com/assets/80153681/2e1e415c-3cbb-4f82-b977-70454037f924

Umm, not sure what's the best option to put edit Icon, please feel free to suggest other options 🙌

vercel[bot] commented 6 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
grants-bg ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2024 2:32pm
carletex commented 6 months ago

Thanks Shiv!

I think we should limit the edition to "proposed" status, because editing in "submitted" status can get very messy if we change askAmount after first 50% payment.

Agree! Another option is to allow edits on on submitted too, but disabling the askAmount (like you can edit typos, change desc, etc... but don't change the amount)

technophile-04 commented 6 months ago

When I edit the askAmount (either using "." or "," as decimal separator, in firebase it changes internally from number to string. It does not affect the grants workflow, but after completion we get error on homepage because it can't calculate totalEthGranted. I get this error:

Ohhh shit sorry my bad, I thought <input type="number" /> would return value as number but it instead return string value, fixed at https://github.com/BuidlGuidl/grants.buidlguidl.com/pull/94/commits/b77e0a945dfebce2a26fc5c9769f603f57766818, tysm for the catch 🙌

I think we should limit the edition to "proposed" status, because editing in "submitted" status can get very messy if we change askAmount after first 50% payment.

Agree! Another option is to allow edits on on submitted too, but disabling the askAmount (like you can edit typos, change desc, etc... but don't change the amount)

Yup disabled the askAmount for now when the grant status is submitted at frontend on https://github.com/BuidlGuidl/grants.buidlguidl.com/pull/94/commits/093df8f2207d01821060d5ddb16d106d1943ac24

Not sure if this makes sense, but should we add some kind of max limit to askAmount in FE or BE to prevent strange behaviours or typos ending in wrong ETH amounts being paid to grantees?

Yup makes sense ! I initially thought 25 ETH would be good max limit but I think we are also planning to allow ERC 20 like OP tokens in the future so not sure what the best max limit would be :(

Thanks all for the reviews 🙌

carletex commented 6 months ago

Great stuff @technophile-04 !! And thanks @Pabl0cks for the review!

Pushed a tiny naming thing!

Merging!!