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

Bugfix/sync improvements #2417

Merged
merged 9 commits into from
Mar 22, 2021
Merged

Bugfix/sync improvements #2417

merged 9 commits into from
Mar 22, 2021

Conversation

rickycodes
Copy link
Contributor

@rickycodes rickycodes commented Mar 22, 2021

Description

This PR fixes a couple of a small bugs. The main one is that when syncing with the extension there was a case where if JSON.parse failed you'd end up in a catch, but we never sent the user back so they'd get stuck in a loading state. This also simplifies some of the loading logic and ensures we display the correct message when syncing (there were instances where if you had deleted the wallet and then went back to sync you'd see the delete messaging instead). Lastly I removed the SyncWithExtension view since we're actually not using that any more.

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #https://github.com/MetaMask/mobile-planning/issues/83

@rickycodes rickycodes requested a review from a team as a code owner March 22, 2021 15:30
@rickycodes rickycodes force-pushed the bugfix/sync-improvements branch from fad78a5 to e6854fd Compare March 22, 2021 15:34
@rickycodes rickycodes force-pushed the bugfix/sync-improvements branch from 7ecd9fa to ac8ef1f Compare March 22, 2021 15:57
@rickycodes rickycodes added 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 Mar 22, 2021
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.

LGTM!

@rickycodes rickycodes removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Mar 22, 2021
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.

QA Passed 👍🏽

@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Mar 22, 2021
@rickycodes rickycodes merged commit 448df32 into develop Mar 22, 2021
@rickycodes rickycodes deleted the bugfix/sync-improvements branch March 22, 2021 19:54
estebanmino pushed a commit that referenced this pull request Mar 22, 2021
* Add missing onErrorSync call

* Add loadingMsg

* Simplify

* Remove unused SyncWithExtension view

* Update snapshots

* Add loadingMsg initial state

* always fail sync

* Revert "always fail sync"

This reverts commit 25ea4ba.
estebanmino pushed a commit that referenced this pull request Mar 24, 2021
* Add missing onErrorSync call

* Add loadingMsg

* Simplify

* Remove unused SyncWithExtension view

* Update snapshots

* Add loadingMsg initial state

* always fail sync

* Revert "always fail sync"

This reverts commit 25ea4ba.
rickycodes added a commit that referenced this pull request Jan 31, 2022
* Add missing onErrorSync call

* Add loadingMsg

* Simplify

* Remove unused SyncWithExtension view

* Update snapshots

* Add loadingMsg initial state

* always fail sync

* Revert "always fail sync"

This reverts commit 25ea4ba.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release QA Passed A successful QA run through has been done v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants