Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Filter by connected user #1783

Merged
merged 5 commits into from
Nov 5, 2021
Merged

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Nov 4, 2021

Summary

This PR makes the recent activity not to show all the activities from all accounts anymore.

Now, only the connected wallet will be displayed.

Not included

It doesn't prevent the process that check if tx are mined in the background (aka: Updater) to stop checking on other account's tx.

This is why you might hear a mooooo even thought there's "nothing pending" for the current account.
A future PR will address this.

To Test

  1. Use 2 accounts to do some operations.
  2. Each account should have only their own activities

Background

This is part of what Leandro is doing for loading orders from the API: #1639
However, his Pr is more ambitious, and will take a bit more to complete.
This will be just a temporary feature while we don't have the loading of orders from the API.

@anxolin anxolin marked this pull request as ready for review November 4, 2021 18:50
@anxolin anxolin requested review from W3stside, elena-zh and a team and removed request for W3stside and elena-zh November 4, 2021 18:50
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2021

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

@MareenG
Copy link

MareenG commented Nov 5, 2021

I am again experiencing the issue I reported in #1639 (comment). This time also for Chrome.
I opened a new issue this time #1785

@MareenG
Copy link

MareenG commented Nov 5, 2021

This is why you might hear a mooooo even thought there's "nothing pending" for the current account.

I also got toast notification for orders made with a different account.
I guess this also falls under the changes that will be addressed in the future?

@anxolin
Copy link
Contributor Author

anxolin commented Nov 5, 2021

I am again experiencing the issue I reported in #1639 (comment). This time also for Chrome.
I opened a new issue this time #1785

@MareenG I won't address this one in this PR, because is not 100% related and we will need to add a few changes that will be undone in a upcoming PR (Order APIs)

I also got toast notification for orders made with a different account.
I guess this also falls under the changes that will be addressed in the future?

yep, what i mentioned in the description

@anxolin anxolin requested a review from alfetopito November 5, 2021 13:32
@elena-zh
Copy link

elena-zh commented Nov 5, 2021

Hey @anxolin , should approve/wrap/unwrap transactions follow this logic in this PR?
Currently, I see that these transactions are not hidden for the 2nd account
https://watch.screencastify.com/v/zneYcMPegxdXpvxrxexW

Btw, in this video you can see an issue reported by Mareen that an order from one network is displayed for another network.

anxolin and others added 2 commits November 5, 2021 18:48
Co-authored-by: Leandro Boscariol <[email protected]>
@anxolin
Copy link
Contributor Author

anxolin commented Nov 5, 2021

Hey @anxolin , should approve/wrap/unwrap transactions follow this logic in this PR?
Currently, I see that these transactions are not hidden for the 2nd account
https://watch.screencastify.com/v/zneYcMPegxdXpvxrxexW

It should! Let me review what was wrong.

Btw, in this video you can see an issue reported by Mareen that an order from one network is displayed for another network.

YEp, that's what I mentioned. This part I won't fix it. I will wait for the order API that leandro is doing just to not put effort in sth that will go away soon. This issue is already in production and there's not much complaints about that

@anxolin
Copy link
Contributor Author

anxolin commented Nov 5, 2021

Hey @anxolin , should approve/wrap/unwrap transactions follow this logic in this PR?
Currently, I see that these transactions are not hidden for the 2nd account

For me is working. I tried for example unwrapping, each account has its own tx.

Nov-05-2021.18-51-09.mp4
Nov-05-2021.18-48-16.mp4

Elena, I will merge it, we still will do some tests for the RC before deploying this, if the issue persists we will look further.

@anxolin anxolin merged commit 18eb5d9 into release/1.5 Nov 5, 2021
@alfetopito alfetopito deleted the filter-activity-by-connected-account branch November 5, 2021 19:48
@elena-zh
Copy link

elena-zh commented Nov 8, 2021

Hey @anxolin , I have retested changes.
Today everything works as expected to me: activity history is changed when change accounts (orders and transactions).
I have not encountered any new issues besides the mentioned above about pending orders which appear in another networks.

nenadV91 pushed a commit that referenced this pull request Nov 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants