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

Handle cases without currency symbols #567

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Conversation

Li357
Copy link
Contributor

@Li357 Li357 commented Aug 21, 2023

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/309211
$ https://github.com/Expensify/Expensify/issues/309227

Tests

Added two unit tests to test cases without currency symbols and without passing a currency string at all.

QA

  1. What does QA need to do to validate your changes?
  2. What areas to they need to test for regressions?

Run the QA steps in the linked issues and verify that the error does not occur.

@Li357 Li357 requested a review from a team as a code owner August 21, 2023 14:13
@Li357 Li357 self-assigned this Aug 21, 2023
@Li357 Li357 requested a review from techievivek August 21, 2023 14:14
@melvin-bot melvin-bot bot requested review from deetergp and removed request for a team August 21, 2023 14:14
@techievivek
Copy link
Contributor

@Li357 QQ: Did you confirm if those 2 issues are no longer reproducible?

@Li357
Copy link
Contributor Author

Li357 commented Aug 21, 2023

Just confirmed https://github.com/Expensify/Expensify/issues/309211 is no longer producible, https://github.com/Expensify/Expensify/issues/309227 seems a little longer to set up

@Li357
Copy link
Contributor Author

Li357 commented Aug 21, 2023

@techievivek Verified both are not reproducible!

@techievivek
Copy link
Contributor

I confirmed the other one is not reproducible with this change.

Copy link
Contributor

@techievivek techievivek left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the quick fix.

@techievivek techievivek merged commit 5ab0cf3 into main Aug 21, 2023
3 checks passed
@techievivek techievivek deleted the andrewli-fixcurrency branch August 21, 2023 14:52
@techievivek
Copy link
Contributor

@Li357 Let's bump the new version in Web-E and I can CP that to staging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants