-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix: refresh ownership status for nfts #8230
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
I have read the CLA Document and I hereby sign the CLA |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #8230 +/- ##
=======================================
Coverage 39.64% 39.64%
=======================================
Files 1233 1233
Lines 29830 29822 -8
Branches 2840 2840
=======================================
- Hits 11826 11824 -2
+ Misses 17315 17309 -6
Partials 689 689 ☔ View full report in Codecov by Sentry. |
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8b7371f9-a52f-4f8d-a736-65132be7d714 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @sahar-fehri can we confirm this also works on android? If possible can you add unit test coverage for this particular flow?
Also can you confirm that this bug no longer exists on your PR: #3406
9eb85d6
to
6996c55
Compare
hey @cortisiko I have tested it on Android also, no issues. I have added a unit test for my change, also i dnt see the problem described on PR: #3406 on my PR 🙏 |
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ ✅ ✅ ✅ ✅
Description
When a user transfers an NFT, he will still be able to see it on the NFT tab view.
This PR refreshes the NFTs owned by the user and displays only the ones currently owned by him.
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/MMASSETS-103
Manual testing steps
Screenshots/Recordings
Before
Account2 transferred the three nfts to Account test, and yet you still see the nfts displayed on account2.
Screen.Recording.2024-01-09.at.19.04.20.mov
After
Returning to account2 and refreshing the page, will remove the nft from the list.
final4.mov
Pre-merge author checklist
Pre-merge reviewer checklist