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] Update selected network when delete network manually inserted #5219

Merged
merged 6 commits into from
Dec 28, 2022
Merged

[FIX] Update selected network when delete network manually inserted #5219

merged 6 commits into from
Dec 28, 2022

Conversation

tommasini
Copy link
Contributor

@tommasini tommasini commented Nov 9, 2022 •
edited by cortisiko
Loading

Description
When comparing two rpc url, and one was inserted manually, we were comparing the selected network url (example: "https://rpc.gnosischain.com") with the rpc url of the network we want to delete (example: "https://rpc.gnosischain.com") and this is false, only when it's true we change the network to mainnet

Proposed Solution
Comparing two rpc url with an existing function, that converts the rpc url to an object for we to be sure they are the same.

Test Cases
Case 1:

  • inserted manually xdai chain
  • deleted on Settings/Networks
    Case 2:
  • Inserted binance smart chain via network picker
  • deleted on Settings/Networks
    Case 3:
  • Inserted Avalanche network via network picker
  • Change selected network to goerili
  • Deleted avalanche network on Settings/Networks
  • Expected to remain on goerli network

Screenshots/Recordings
https://recordit.co/DtgAlb5F23

Issue

Progresses #5117

Checklist

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

@tommasini tommasini requested a review from a team as a code owner November 9, 2022 16:18
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2022

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.

@tommasini tommasini added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Nov 14, 2022
blackdevelopa
blackdevelopa approved these changes Dec 16, 2022
View reviewed changes
Copy link
Contributor

@blackdevelopa blackdevelopa left a comment

Choose a reason for hiding this comment

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

LGTM

@tommasini tommasini added Mobile QA board needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Dec 16, 2022
@sethkfman sethkfman added the release-5.14.0 Issue or pull request that will be included in release 5.14.0 label Dec 21, 2022
@olenapankina olenapankina 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 Dec 22, 2022
@olenapankina olenapankina self-requested a review December 23, 2022 18:14
olenapankina
olenapankina suggested changes Dec 23, 2022
View reviewed changes
Copy link

@olenapankina olenapankina left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested on
Samsung Galaxy M21 Android 12, One UI Core version 4.1
iPhone Xr iOS 16.1.1

Issue_01: User is switched to the Mainnet after deleting custom network while on a different custom network
Steps to reproduce:

  1. Select Avalanche network via network picker
  2. Change selected network to goerili
  3. Go to Settings/Network
  4. Open Avalanche network
  5. Note that the network is not changed to Avalanche
  6. Click delete
  7. Go back to the wallet

Actual result: user is on Mainnet 5219_Issue_01.mov
Expected result: user is on goerli network

Note: the bug will not be reproducible if you delete the network by hard pressing Avalanche network and deleting it from the pop-up. 5219_expected.mov

@olenapankina olenapankina 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 Dec 23, 2022
@tommasini tommasini added needs-qa Any New Features that needs a full manual QA prior to being added to a release. needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed needs-qa Any New Features that needs a full manual QA prior to being added to a release. Mobile QA board labels Dec 26, 2022
Fatxx
Fatxx approved these changes Dec 26, 2022
View reviewed changes
Copy link
Contributor

@Fatxx Fatxx left a comment

Choose a reason for hiding this comment

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

LGTM

@tommasini tommasini added Mobile QA board and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Dec 26, 2022
@tommasini tommasini added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Dec 26, 2022
@olenapankina olenapankina 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 Dec 27, 2022
@olenapankina olenapankina self-requested a review December 28, 2022 19:54
olenapankina
olenapankina approved these changes Dec 28, 2022
View reviewed changes
Copy link

@olenapankina olenapankina left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested on
Samsung Galaxy M21 Android 12, One UI Core version 4.1
iPhone Xr iOS 16.1.1
fix/5117-network-picker-not-update-correctly: commit 87c241a

User is able to switch the network, the network picker is updated accordingly. Issue_01 confirmed fixed.
e2e tests passed

@olenapankina olenapankina merged commit e94e0f0 into main Dec 28, 2022
@olenapankina olenapankina deleted the fix/5117-network-picker-not-update-correctly branch December 28, 2022 20:00
@github-actions github-actions bot locked and limited conversation to collaborators Dec 28, 2022
@olenapankina olenapankina added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@olenapankina olenapankina olenapankina approved these changes

@ullashb ullashb ullashb approved these changes

@blackdevelopa blackdevelopa blackdevelopa approved these changes

@Fatxx Fatxx Fatxx approved these changes

Assignees
No one assigned
Labels
QA Passed A successful QA run through has been done release-5.14.0 Issue or pull request that will be included in release 5.14.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tommasini @Fatxx @blackdevelopa @ullashb @olenapankina @sethkfman