Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

round 8 bug / improve list #7947

Closed
20 of 21 tasks
thelostone-mc opened this issue Nov 26, 2020 · 77 comments
Closed
20 of 21 tasks

round 8 bug / improve list #7947

thelostone-mc opened this issue Nov 26, 2020 · 77 comments

Comments

@thelostone-mc
Copy link
Member

thelostone-mc commented Nov 26, 2020

MAJOR

  • checkIsCurator function is broken as the logic doesn't fit the data (map collections array and check curators on each one).
  • removeCollection doesn't work not id is found.
  • fix to 2 decimals in prediction curve UI FIX: 913db65
  • make background image in RoutingPolicy as optional FIX: fc9b24a
  • add new field name in RoutingPolicy to make it easier to identify FIX: fc0c8bd
  • in UI customer-name is not appended to URL image FIX: 76ff438
  • CLR banner not being updating in UI despite setting image in GrantCLR.logo FIX: 97370ea
  • Appending / URL to endpoint should not return 404 FIX: 5e56dff
  • If grant is part of multiple rounds -> the round names are not appended
  • made CLR banners into accordion so that it is collapsible and store that boolean in local-storage
  • edit grant broken. FIX: 7547b5f

MINOR

  • ensure error are cleared after submitting a match pledge FIX: c7767fc
  • fix broken collections UI FIX: 821de8c
  • fix alignment in prediction curve UI image
  • Show My Items on landing only if user is logged in FIX: 1b33dcd
  • update text in Did you know section FIX: fe0b6f1
  • Show Add to collection in card only if user is logged in and has a collection of their own image
  • Sidebar should auto-close after canceling grant
  • Remove Current & Past Sponsors in UI FIX: 42c4d73
  • Hide prediction curve when a grant is not part of a CLR FIX: ad530e8
  • related grants styles
@thelostone-mc thelostone-mc changed the title round 8 bug list round 8 bug / improve list Nov 27, 2020
@owocki
Copy link
Contributor

owocki commented Nov 27, 2020

just tested, feels pretty rough. here are my QA notes (all in latest chrome + metamask):

showstoppers

  • i cant actually test the 'new matching partner' stuff bc no collections or categories show up ( https://bits.owocki.com/2Nu0xE25 & https://bits.owocki.com/RBu974Pp )
    aditya: this works as discussed offline

  • when i try to edit a grant i get this error https://bits.owocki.com/jkuYqY4y error in console is error: grant creation failed with status: 400 and message: error: zcash_payout_address must be a transparent address

  • CANCEL GRANT does not seem to work. when i type in CONFIRM in the alert bar, nothing happens. nothing to let me know if my grant was actually canceled or not.
    aditya: the grant cancellation works, the sidebar should auto close (has been captured above)

  • cannot create a grant. it just says 'verify form errors' when there are no actual form errors https://bits.owocki.com/DOuogor5 - i think this happened when i had a validation issue but then fixed it. i could never get past validation again without refreshing page.
    aditya: tested this offline with same steps. it works. make sure you have latest + cache is cleared + do a hard reload
    octavio: looking the gif the problem was because the second handle without @, and there was no error report for that field.

  • on zksync checkout, i was taking to a new page checkout.zksync.com which had the following issues

  1. on my grants page, i see this error after going to zksync https://bits.owocki.com/z8u454jZ - looks related to this https://bits.owocki.com/YEuQKQxX
  2. i am never taken to the 'success, you checked out, tweet about it now!' modal after checkout

major

attention to detail stuff

  • I don't see the 'show idle grants' filter on the left-hand side of the nav anymore. are we not filtering out idle grants anymore? if so, why not? that wasn't a requirement.

  • 'become a matching partner' is lopsided https://bits.owocki.com/2Nu0xjZy
    aditya: pull in the latest code, you should you create a grant button as well https://user-images.githubusercontent.com/5358146/100470286-f5956580-30fd-11eb-8d83-27468a7ca9e0.png

  • the page refreshes as I click around the nav; I thought we were fixing that this round?
    aditya: that is being worked on by @aamustapha. It might get in a little late

  • if I remove my grant from cart on the side cart the 'remove from cart' button does not change on the UI https://bits.owocki.com/qGulyvWn
    octavio: I added the "remove from cart" on the vue components if the component is added but the sidebar cart is not build on vue so is not reactive, this will be solved when we migrate the sidebar to vuejs, but at least for now people can see the grants they added when scrolling.
    octavio: done.

  • it'd be nice if i could click/unclick this label to check/uncheck the box https://bits.owocki.com/mXu5j5Wl

  • the 'back to grants' breadcrumb that used to be consistent across each grant page + cart is no longer on the grant detail page https://bits.owocki.com/p9urlrvN
    octavio: was added today please pull changes.

  • i setup an 'infra tech' round and made it active and ran 'create_page_cache', but it never showed up on the left navbar as a round https://bits.owocki.com/6quxkQmO
    aditya: tested this out and works. make sure Grant CLR is active and end date is in the future

  • follow button on grant detail page does nothing if youre logged out (we should tell the user they should login first)

could not test

  • a grant being in multiple CLR rounds at once
    aditya: tested this

  • emails - never got any
    aditya: got emails for grant creation + contribution + cancellation

  • self-service matching partner stuff; i was blocked by the showerstopped issue above.
    aditya: this was due to a grant with a weird title in live -> fixing that up should fix it. This works in live and I've tested this as well

  • i could not test many of the trustbonus items, due to lack of setup on some of them, and lack of actually having those services on others. will need to make sure it's well tested elsewhere.

  • videos - where do i add a video to my grant? i wasn't compelled to in any place?
    Octavio: on grant edition you have a button to add videos urls on the description

  • does the 'last update' field actually update when the grant is edited, @octavioamu ? that'll be important
    Octavio: yes you can check next to the title, I will update without reload the page.

@octavioamu
Copy link
Contributor

on grant creation, why are we forcing people to add an @ symbol? seems like needless user work, we can just add it on the backend if needed. don't introduce unnecessary friction pls

I added better validation for this but extract the username of any format seems overkill to me. I was looking on some of the possible patterns https://regex101.com/r/s6JoZ5/3

@owocki
Copy link
Contributor

owocki commented Nov 28, 2020

testing latest build

@thelostone-mc
Copy link
Member Author

thelostone-mc commented Nov 28, 2020

@chibie could you take a look at the twitter handle !
let's make it such that it auto adds an @ if it's not present when frontend sends it to the backend ?

@owocki looks like there edit grants had to be fixed up a bit cause it was setting ETH payout address to undefined on edit.
7547b5f fixes it
So when you do pull it in and test -> do ensure the ETH address for the grant is set in admin

@zoek1 zoek1 mentioned this issue Nov 30, 2020
@zoek1
Copy link
Contributor

zoek1 commented Nov 30, 2020

@owocki I managed the twitter handle in the PR #7962

@zoek1
Copy link
Contributor

zoek1 commented Nov 30, 2020

Also I'm still getting the error: zcash_payout_address must be a transparent address when i try to edit any grant, the fix isn't merged yet?

@thelostone-mc
Copy link
Member Author

thelostone-mc commented Nov 30, 2020

@zoek1 could you make sure your grant has

  • zcash_payout_address has value '0x0'
  • admin_address has a valid ETH address ?

and you have the latest changes

@owocki
Copy link
Contributor

owocki commented Nov 30, 2020

I added better validation for this but extract the username of any format seems overkill to me.

i just dont want the grant creation form to fail validation bc of the lack of an @ symbol. seems this could be accomplished by just doing an alphanumeric regex. if we want to ensure consistence on the backend, the @ symbol can be added on hte backend.

more testing notes:

  • after i add a grant to cart on grant detila page, the side cart no longer shows. where do i go to checkout?

  • i can't seem to test a grant being in multiple CLRs bc i can't make a contribution + get it to store in the DB... it'd be good to know someone has tested this
    aditya: i've tested this

  • Grant 1018: UI

  • Grant 1018 in CLR table

  • Grant 1018 effective CLR (5956 + 864)

  • when i create a grant contribution on the frontend, i dont see anything in the DB
    (i wanted to test to make sure that when i check 'include this in CLR' it actually stores that state in the DB)
    aditya: I have a feeling your local setup isn't right or something is wonky. Tesing this for a 3rd time with the latest code !

Untitled

@octavioamu
Copy link
Contributor

octavioamu commented Nov 30, 2020

after i add a grant to cart on grant detila page, the side cart no longer shows. where do i go to checkout?

I just pushed a commit adding the sidecart back

@apbendi
Copy link
Contributor

apbendi commented Dec 1, 2020

Twitter verification on the Trust Bonus tab is broken in the grants branch, as far as I can tell. Seems like the regression came from this PR, which I commented on with a bit more context: #7962

@zoek1, can you expand on why you were modifying the twitter verification endpoint? That endpoint is used for the Trust Bonus. Was it your intention to change the behavior of the Trust Bonus?

@apbendi
Copy link
Contributor

apbendi commented Dec 1, 2020

Attempting to test SMS verification from the Trust Bonus tab gives the following error from the backend:

, uri, response, 'Unable to fetch record')
web_1            | twilio.base.exceptions.TwilioRestException:
web_1            | HTTP Error Your request was:
web_1            |
web_1            | GET /PhoneNumbers/REDACTED
web_1            |
web_1            | Twilio returned the following information:
web_1            |
web_1            | Unable to fetch record: Authenticate
web_1            |
web_1            | More information may be available here:
web_1            |
web_1            | https://www.twilio.com/docs/errors/20003

Has our API key or auth token changed? (Reminder not to post these here)

@apbendi
Copy link
Contributor

apbendi commented Dec 1, 2020

Need a bit more direction on how to regression test Google Verification: #7631 (comment)

Should still be working but would like to test locally

@zoek1
Copy link
Contributor

zoek1 commented Dec 1, 2020

@apbendi my intention wasn't to modify the trust bonus, just the twitter grant verification. The error is due to wrong conditional statement. The fix for that is here #7974

@danlipert
Copy link
Contributor

danlipert commented Dec 1, 2020

  • Editing an ETH grant now throws an error when trying to save. The error displays in the UI and in the console the error is: _detail-component.js:99 error: grant edit failed with status: 400 and message: error: zcash_payout_address must be a transparent address
    Seems that for ETH grants we are trying to validate their zcash address which doesn't apply here.

edit: fixed via #7977

@danlipert
Copy link
Contributor

danlipert commented Dec 1, 2020

  • When creating a grant, the validation fails if the twitter username does not start with an "@" symbol. While I appreciate the attention to detail to validate the twitter handle starting with @, its bad UX as the placeholder text shows a username without the @ symbol.

edit: fixed via #7976 allows starting @ symbol or not in frontend while it is stripped in backend per @owocki 's comment above

@danlipert
Copy link
Contributor

While checking out with multiple ETH grants in my cart via zksync, I received an error alert in the UI while I was redirected to the zksync checkout. Zksync checkout worked perfectly as expected, and when finished I was returned to the grants page and received the success modal, but during this time if I return to the cart page the alert is shown which reads "Insufficient balance to complete checkout". I don't know if this refers to the zksync balance being insufficient as I a) did have enough funds in my wallet to complete checkout and b) did not have any L2 funds at this time, but either way if that is "expected behavior" its definitely confusing/scary for users and the copy and alert style need to be changed.

Screen Shot 2020-12-01 at 7 56 54 PM

@thelostone-mc
Copy link
Member Author

^ @mds1

@mds1
Copy link
Contributor

mds1 commented Dec 1, 2020

Hey @danlipert. The insufficient balance modal should only show if you have insufficient funds in both L1 and L2. Can you clarify the steps to reproduce? I'm not sure I follow.

but during this time if I return to the cart page the alert is shown which reads "Insufficient balance to complete checkout"

This is the main part I'm confused on. The success modal redirects you off the cart page, so did you then go back the cart, and the cart was not cleared? Or did you add new grants to your cart after? Or something else?

Also, what was in your cart at the time of that second checkout? It looks like you may have had an empty "amount" field based on the console error messages showing vaule=""

@danlipert
Copy link
Contributor

@mds1 I had sufficient funds in L1 but not L2. Not sure exactly how to reproduce. What happened was the zksync flow opened in a new tab, and i went back to the cart tab just to check and the error was there while I was going through the zksync flow. Once I finished the zksync flow I was redirected back to the success modal screen. I took the screenshot while I was going through the zksync flow. I'm pretty sure all the amount fields had something in them as I got the appropriate number of txns in the zksync checkout explorer link list, but they were all very small amounts.

@mds1
Copy link
Contributor

mds1 commented Dec 1, 2020

@danlipert Ahhh I see, got it, thanks for the additional info! I'll look into this today

@willsputra
Copy link
Contributor

willsputra commented Dec 1, 2020

Browser: Brave Mac 1.17.73

Major

  • zkSync Checkout: Tried contributing 0.5 DAI to two grants with optional tip, got a "Transactions batch summary fee is too low" error tx | error @mds1
    will: didn't seem to get this error again. looks like this is fixed.
  • Grant Contribution Success Email: I contributed 0.5 DAI but email shows 1.0000 SAI contrib | email

Minor

  • Quick View: Wall of Love numbers scroll down as you scroll down. screenshot
  • Grant Detail: Description font is different screenshot
  • zkSync Checkout: Are "Grants Round 8+ Dev Fund" and "Grants Round 7+ Development Fund" different? Current optional tip goes to "Grants Round 7+ Development Fund" screenshot
  • Edit Grant: Including @ on the twitter handle field results in @@ edit | displayed

@danlipert
Copy link
Contributor

danlipert commented Dec 1, 2020

Thanks @willsputra ! I also just got the SAI / DAI issue but only for one of the grants (gitcoin dev fund optional contribution) so probably has something to do with when the grant was created back when you could specify a specific type of token you accepted. I also received 4 emails for the 4 rolled up zksync contributions :/ But was able to successfully check out. @willsputra the error you got I believe has to do with the calculation of the zksync fee being larger than what you would have saved by using it or something like that.

  • batch email not working, receiving separate emails per contrib EDIT: this PR was rolled back before deploy due to bugs
    aditya: Doubtful ! Given that subscription creation happens in celery. I'm not sure how we'd be able to wait for all celery tasks to be processed and only then send an email ! scope lift is working on it -> but I'm not sure how we'd actually do this

  • gitcoin dev fund grant contribution email shows contributing 5 SAI when I actually contributed 0.15 DAI https://zkscan.io/transactions/sync-tx:484dd93f81ab2de7f2bd0da457b11bc8b8fdc6138d3dffb36495e94bde1801a3
    Screen Shot 2020-12-02 at 1 08 19 AM

aditya: reverting the 1 mail -> multi contribution PR would fix this

@octavioamu
Copy link
Contributor

octavioamu commented Dec 1, 2020

  • related grants need pagination and layout

@owocki
Copy link
Contributor

owocki commented Dec 1, 2020

  • i dont think the 'anonymize my contribution' is working. i just checked the box + made a contribution and it showed up in the feed/transaction list as being from me. ( https://bits.owocki.com/jkuYymEA ) it should show the gitcoinbot in those places instead (grants/tasks.py:182 is supposed to do this)

@owocki
Copy link
Contributor

owocki commented Dec 1, 2020

i just got 8 emails from making 8 contributions in 1 cart. is the bulk checkout email going to make it in?

aditya: Doubtful ! Given that subscription creation happens in celery. I'm not sure how we'd be able to wait for all celery tasks to be processed and only then send an email ! scope lift is working on it -> but I'm not sure how we'd actually do this

@jdorfman
Copy link

jdorfman commented Dec 2, 2020

You are additionally contributing 1.25 DAI to the Gitcoin Mainainter Grant

Replace withMaintainer

image

@owocki
Copy link
Contributor

owocki commented Dec 2, 2020

@owocki
Copy link
Contributor

owocki commented Dec 2, 2020

@owocki
Copy link
Contributor

owocki commented Dec 2, 2020

@owocki
Copy link
Contributor

owocki commented Dec 2, 2020

@owocki
Copy link
Contributor

owocki commented Dec 2, 2020

@spm32
Copy link
Collaborator

spm32 commented Dec 2, 2020

Grant in question: https://gitcoin.co/grants/1602/gallerist-by-collectorshub

@chibie
Copy link
Contributor

chibie commented Dec 2, 2020

Share Cart URL shows blank page.

cc: @jdorfman

@vs77bb
Copy link
Contributor

vs77bb commented Dec 2, 2020

  • getting the same request as @ceresstation from rachel, good ghosting.

shows is CLR Eligible in the backend, but when checking out it doesn’t show.

https://gitcoin.co/_administrationgrants/grant/1112/change/

image

@mds1
Copy link
Contributor

mds1 commented Dec 2, 2020

@vs77bb @ceresstation If you are a team member of a grant, and add that grant to your cart, it will say "Not eligible for CLR" because you don't get matched for donating to a grant you are part of. Both of those grants seem to be working properly

image

We can change that message to me more clear in this case. Let me know if you have any suggestions!

@spm32
Copy link
Collaborator

spm32 commented Dec 2, 2020

  • +1 on making that message more clear for this specific case I'd say something like "Unable to fund a grant you own" or "Unable to fund your own grant"

@owocki
Copy link
Contributor

owocki commented Dec 2, 2020

kevinowocki@local /Users/kevinowocki/Sites/gitcoin/web~ % time curl "https://gitcoin.co/grants/cards_info?page=1&limit=6&sort_option=weighted_shuffle&network=mainnet&keyword=&state=active&collections_page=1&category=&type=all" > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  157k  100  157k    0     0   7127      0  0:00:22  0:00:22 --:--:-- 37765
curl  > /dev/null  0.02s user 0.01s system 0% cpu 22.705 total

this API is super slow + causes the grants index page to be slow.

EDIT: added some indexes in 07a6e67 and its now down to 2s. not super fast but at least not showstopp'y slow anymore

kevinowocki@local /Users/kevinowocki~ % time curl "https://gitcoin.co/grants/cards_info?page=1&limit=6&sort_option=weighted_shuffle&network=mainnet&keyword=&state=active&collections_page=1&category=&type=all" > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  157k  100  157k    0     0  64186      0  0:00:02  0:00:02 --:--
curl  > /dev/null  0.02s user 0.01s system 0% cpu 2.527 total

@spm32
Copy link
Collaborator

spm32 commented Dec 2, 2020

owocki added a commit that referenced this issue Dec 2, 2020
@owocki
Copy link
Contributor

owocki commented Dec 3, 2020

@vs77bb
Copy link
Contributor

vs77bb commented Dec 3, 2020

image

@vs77bb
Copy link
Contributor

vs77bb commented Dec 3, 2020

maybe a power-user thing, the 'Sort By' isn't persistent as i scroll down to view grants. makes it hard to sort by matching.

image

@owocki
Copy link
Contributor

owocki commented Dec 3, 2020

https://bits.owocki.com/yAuZvErY

round 7 image still showing u8p

@teohhanhui
Copy link

TxBatch failed. Reason: zkSync API response error: code 103; message: Tx is incorrect

Screenshot

Screenshot_2020-12-04 zkSync

@mds1
Copy link
Contributor

mds1 commented Dec 3, 2020

Thanks @teohhanhui, I'll relay this to the zkSync team now. What browser and wallet were you using?

@teohhanhui
Copy link

What browser and wallet were you using?

Firefox 84.0b7 (Manjaro Linux)
WalletConnect (Authereum)

@developerfred
Copy link
Contributor

@zoek1 #8027

@vs77bb
Copy link
Contributor

vs77bb commented Dec 4, 2020

  • seems like matching in the Matic Round is off -- shows high matching numbers with no donations?

https://gitcoin.co/grants/1530/beyondnft

image

@vs77bb
Copy link
Contributor

vs77bb commented Dec 4, 2020

  • when you scroll far enough down the grants page you start getting many duplicate grants loading of the same grant image

aditya: not a bug , looks like user created them multiple times. I've removed the duplicates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests