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.

Sign up for GitHub

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

Jump to bottom

Fix transaction history #2245

Merged
merged 23 commits into from
Mar 10, 2021
Merged

Fix transaction history #2245

merged 23 commits into from
Mar 10, 2021

Conversation

andrepimenta
Copy link
Member

@andrepimenta andrepimenta commented Feb 15, 2021 •
edited
Loading

Description

This PR refactors the transactions history list.

@ibrahimtaveras00 for testing we should make sure that:

  • Transactions are always shown correctly filtered by account and network. For both default networks and also custom networks.
  • Submitting a new transaction is always correctly reflected on the list without any errors.
  • When updating from a previous app version, the transaction list should not be lost for default networks. For custom networks there is a possibility of older transactions not appearing since we are now filtering by chainId and it was not present on older transactions. But making a new transaction should still work with the correct nonce.
  • Every type of transaction should be tested and should be reflected on the list with the correct status (pending, succeeded or failed). Switching networks or accounts or any other action on the app should not affect the transaction, its status should always be correctly synced with the blockchain.

If any of those items above don't work let me know and will fix it!

@andrepimenta andrepimenta requested a review from a team as a code owner February 15, 2021 16:00
@andrepimenta andrepimenta added DO-NOT-MERGE Pull requests that should not be merged needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Feb 16, 2021
@ibrahimtaveras00 ibrahimtaveras00 added QA in Progress QA has started on the feature. and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Mar 3, 2021
@wachunei wachunei mentioned this pull request Mar 4, 2021
Closed
ibrahimtaveras00
ibrahimtaveras00 suggested changes Mar 5, 2021
View reviewed changes
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see the purchases tab in txn history upon buying eth via Transak

@ibrahimtaveras00 ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed QA in Progress QA has started on the feature. labels Mar 5, 2021
ibrahimtaveras00
ibrahimtaveras00 approved these changes Mar 8, 2021
View reviewed changes
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to test release build on testflight, it worked
QA Passed 👍🏽

@estebanmino
Revert "589"
88a4bb0 
This reverts commit b9663d9.
@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Mar 8, 2021
@andrepimenta andrepimenta removed the DO-NOT-MERGE Pull requests that should not be merged label Mar 10, 2021
@andrepimenta
Merge branch 'develop' into fix/transactions-issues
0597dfd 
# Conflicts:
#	package.json
#	yarn.lock
estebanmino
estebanmino approved these changes Mar 10, 2021
View reviewed changes
Copy link
Contributor

@estebanmino estebanmino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@andrepimenta andrepimenta merged commit f3f662f into develop Mar 10, 2021
@andrepimenta andrepimenta deleted the fix/transactions-issues branch March 10, 2021 15:54
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
@andrepimenta @estebanmino
Fix transaction history (#2245)
405f263 
* Transactions view refactor

* Filter by chainId

* Add controllers to be tested

* Prefer chainId overe chainId

* Update controllers for QA

* Update controllers

* Update yarn.lock

* Import Transaction Controller from metamask repo

* Update metamask-controllers-v6.0.1.tgz

* Update metamask-controllers-v6.0.1.tgz

* Update metamask-controllers-v6.0.1.tgz

* Now integrity working

* Update metamask-controllers-v6.0.1.tgz

* Update controllers zip version

* 589

* Revert "589"

This reverts commit b9663d9.

* Bump controllers

Co-authored-by: Esteban Mino <efmino@uc.cl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebanmino estebanmino estebanmino approved these changes

@ibrahimtaveras00 ibrahimtaveras00 ibrahimtaveras00 approved these changes

Assignees
No one assigned
Labels
needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) next release QA Passed A successful QA run through has been done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andrepimenta @estebanmino @ibrahimtaveras00 @omnat