Skip to content

Fix for #5898 - Converting native ETH to fiat and fiat to native ETH results in wrong values beign displayed on the Amount screen#5961

Merged
segun merged 4 commits intomainfrom
fix-conversion-bug-5898
Mar 23, 2023
Merged

Fix for #5898 - Converting native ETH to fiat and fiat to native ETH results in wrong values beign displayed on the Amount screen#5961
segun merged 4 commits intomainfrom
fix-conversion-bug-5898

Conversation

@segun
Copy link
Copy Markdown
Contributor

@segun segun commented Mar 13, 2023

Description

This PR fixes #5898, a bug that shows wrong results during etc/fiat conversion

Checklist

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

@segun
use setState callback to call onInputChange
299267b 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
@segun segun requested a review from a team as a code owner March 13, 2023 20:21
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 13, 2023

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.

@segun segun changed the title use setState callback to call onInputChange Fix for #5898 - Converting native ETH to fiat and fiat to native ETH results in wrong values beign displayed on the Amount screen Mar 14, 2023
@segun segun marked this pull request as draft March 14, 2023 16:33
@segun
E2E for currency switch
43504e1 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
@segun segun self-assigned this Mar 14, 2023
@segun segun marked this pull request as ready for review March 14, 2023 17:36
@segun
Lint fix
83ec12b 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
jpuri
jpuri reviewed Mar 15, 2023
View reviewed changes
@segun
Update snapshot
2af31c0 
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
@segun segun merged commit 7b5cce5 into main Mar 23, 2023
@segun segun deleted the fix-conversion-bug-5898 branch March 23, 2023 23:08
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2023
@bschorchit
Copy link
Copy Markdown
Contributor

bschorchit commented Mar 24, 2023

Has this been QAed? @seaona

@seaona
Copy link
Copy Markdown
Member

seaona commented Apr 4, 2023

@bschorchit this has not been QAd.
@segun notice that after having 2 dev reviews, the developer should add the needs-qa label, so the QA engineer can test it. Once QA has passed, the QA engineer adds the label QA-passed and merges the PR to main. Finally a release label is added

@seaona seaona added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Apr 4, 2023
@seaona
Copy link
Copy Markdown
Member

seaona commented Apr 13, 2023

I can see how the issue is fixed. I add the QA Pass label.

  • Conversion is correctly performed on Mainnet
Screencast.from.13-04-23.14.28.26.webm
  • Conversion is correctly performed on another networks
Screencast.from.13-04-23.14.29.31.webm
  • Conversion is correctly performed with ERC20 tokens
Screencast.from.13-04-23.14.32.09.webm

@seaona seaona added QA Passed QA testing has been completed and passed team-confirmations-secure-ux-PR PR from the confirmations team release-6.5.0 Issue or pull request that will be included in release 6.5.0 and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jpuri jpuri jpuri approved these changes

+1 more reviewer

@blackdevelopa blackdevelopa blackdevelopa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@segun segun

Labels

QA Passed QA testing has been completed and passed release-6.5.0 Issue or pull request that will be included in release 6.5.0 team-confirmations-secure-ux-PR PR from the confirmations team unit test coverage confirmed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] Converting native ETH to fiat and fiat to native ETH results in wrong values beign displayed on the Amount screen

6 participants

@segun @bschorchit @seaona @jpuri @blackdevelopa @hilvmason