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

[Bug] Stop donation / Cancel subscription not showing on Collective page. #136

Closed
decentralauren opened this issue Jan 30, 2024 · 26 comments
Assignees
Labels
P1 - High Important issues that should be resolved soon.

Comments

@decentralauren
Copy link

decentralauren commented Jan 30, 2024

Screenshot 2024-01-30 at 4 18 34 PM
Subscription does not display on Collective screen.

I have a 5-day subscription streaming G$, and do not see the ability to cancel my subscription on the Collective screen.

The out-standing subscription may be the cause of the error in ticket 135

@decentralauren
Copy link
Author

OK the option DID appear, but many minutes after the donation was made. @krisbitney should we expect this kind of lag? If so we should adjust the language in the post-donation modal to say "may take a while to show up" etc. cc @patpedrosa

@krisbitney
Copy link
Contributor

OK the option DID appear, but many minutes after the donation was made. @krisbitney should we expect this kind of lag? If so we should adjust the language in the post-donation modal to say "may take a while to show up" etc. cc @patpedrosa

I added a performance optimization in this PR: #137

The delay depends on multiple factors.

  • The transaction needs to finalize
  • The Subgraph needs to update with the new data from the smart contract
  • The app needs to see the updated Subgraph data

I made it so the app checks more often if there is any new data in the Subgraph for this specific thing.

@vldkhh
Copy link

vldkhh commented Jan 31, 2024

verified

@vldkhh vldkhh closed this as completed Jan 31, 2024
@L03TJ3
Copy link
Collaborator

L03TJ3 commented Feb 1, 2024

@vldkhh this was not merged yet, so needs re-qa'd. if it works for you its for PR to go over it again.

@L03TJ3 L03TJ3 reopened this Feb 1, 2024
@krisbitney
Copy link
Contributor

This is fixed in #137

@vldkhh
Copy link

vldkhh commented Feb 1, 2024

@decentralauren @L03TJ3 see no issues with that

0A93532D-8FAA-43E0-97D9-2F92D7E9EF08.MP4

@vldkhh
Copy link

vldkhh commented Feb 1, 2024

@decentralauren close if you agree

@decentralauren
Copy link
Author

Works as expected with G$ stream

I am unable to test with other currencies as I receive the "UNPREDICTABLE_GAS_LIMIT" error when donating.
@vldkhh were you able to test with non-G$?

@patpedrosa
Copy link

I've noticed that there's a small delay in showing on the profile & collective pages that the stream has stopped, which is not good UX. We're also missing a modal for confirming the donation has stopped. I have added it to our list for sprint 8.

@vldkhh
Copy link

vldkhh commented Feb 7, 2024

@krisbitney @decentralauren I'm not able to stop my donation after using Celo
image

@krisbitney
Copy link
Contributor

@vldkhh Whether you use Celo or not is unrelated to stopping the donation, but I think I may have found the issue. I think this PR should fix it: #178

@vldkhh
Copy link

vldkhh commented Feb 7, 2024

@krisbitney checked on the deployment above, and it is still reproducing + fetchfullnames info logs keep increasing. could you double check it?

1BACF17F-5838-4C87-8893-8FC6B4E207BB.MP4

@krisbitney
Copy link
Contributor

@krisbitney checked on the deployment above, and it is still reproducing + fetchfullnames info logs keep increasing. could you double check it?

1BACF17F-5838-4C87-8893-8FC6B4E207BB.MP4

The log isn't something I added. I'll leave that to your team.

If the stop donation error is still reproducing, it's a smart contract issue. The front end only supplies the inputs to the smart contract calls. The deleteFlow method is very simple.

const tx = await sdk.deleteFlow(signer, collective, '0');

I double checked and confirmed the correct collective address is being passed in. The other parameters are a constant and the signer.

@sirpy

@L03TJ3
Copy link
Collaborator

L03TJ3 commented Feb 7, 2024

Ye that log is mine. I am trying to figure out what makes the fetching of transactions end up in an endless cycle @vldkhh

@krisbitney not asking you to fix it but do you have any idea what can cause this? It seems to be the fetch query for donations, but I haven't figured out yet why?

@krisbitney
Copy link
Contributor

Ye that log is mine. I am trying to figure out what makes the fetching of transactions end up in an endless cycle @vldkhh

@krisbitney not asking you to fix it but do you have any idea what can cause this? It seems to be the fetch query for donations, but I haven't figured out yet why?

No, I didn't realize it was happening. I'll take a look.

@krisbitney
Copy link
Contributor

krisbitney commented Feb 8, 2024

@L03TJ3 I don't think useFetchFullNames is being called endlessly due to repeated fetching transactions. The useMemo hook that depends on the donations and claims arrays isn't repeatedly called, suggesting the same donations and claims arrays are used.

This TODO comment is likely related to the poll interval having been 1 second:

  //todo-fix: donations is called endlessly
  const donations: Transaction[] | undefined = useRecentDonations(collective, maxN, donationsPollInterval);

The useRecentTransactions hook isn't endlessly called. I will update as I learn more.

@L03TJ3
Copy link
Collaborator

L03TJ3 commented Feb 8, 2024

@krisbitney

place a log in supporttranasctionlistitem
and watch console.

that is what I traced back to useRecentTransactions.

@krisbitney
Copy link
Contributor

@krisbitney

place a log in supporttranasctionlistitem and watch console.

that is what I traced back to useRecentTransactions.

I see it. I think the issue might be the flowing balance. It should probably be isolated in its own component, kind of like the FlowingBalanceRowItem component.

@L03TJ3
Copy link
Collaborator

L03TJ3 commented Feb 8, 2024

@krisbitney isn't this likely to happen to any component where we try and merge the transactions?

@krisbitney
Copy link
Contributor

@krisbitney isn't this likely to happen to any component where we try and merge the transactions?

I think it should only affect the SupoprtTransactionListItem and its children

@krisbitney
Copy link
Contributor

krisbitney commented Feb 8, 2024

I confirmed that the issue is the flowing balances. I did so by adjusting the flow update frequency to see if that affected the console logs. Now I'm just thinking about how to refactor the transaction list items.

@krisbitney
Copy link
Contributor

@L03TJ3 A fix is in this PR: #180

@decentralauren
Copy link
Author

@vldkhh @L03TJ3 can you confirm the fix / retest so we can close?

Copy link

Stale issue message

@decentralauren decentralauren added P1 - High Important issues that should be resolved soon. and removed 🙊 stale labels Apr 23, 2024
@decentralauren decentralauren assigned vldkhh and unassigned krisbitney Apr 23, 2024
@decentralauren
Copy link
Author

@vldkhh please retest :)

@vldkhh
Copy link

vldkhh commented Apr 24, 2024

@decentralauren works for me
Snag_2fc5e90e

@vldkhh vldkhh closed this as completed Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 - High Important issues that should be resolved soon.
Projects
None yet
Development

No branches or pull requests

5 participants