Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.54k stars 2.88k forks source link

[$1000] IOU-Offline shows the name of the currency, after going online it shows the symbol of currency #12573

Closed kavimuru closed 1 year ago

kavimuru commented 2 years ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Go to URL https://staging.new.expensify.com/
  2. Create a money request from User A to user B
  3. Go offline
  4. Create another money request
  5. As user A, go to the IOU details and cancel this IOU request
  6. Make sure the request has been cancelled and the IOU preview has been updated (the message should be greyed out)
  7. Back online

Expected Result:

In offline, the currency sign should be displayed immediately

Actual Result:

Offline shows the name of the currency, after going online it shows the sign of the currency

Workaround:

unknown

Platform:

Where is this issue occurring?

Version Number: 1.2.25-0 Reproducible in staging?: y Reproducible in production?: y Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Bug5812702_11720_Web_1

https://user-images.githubusercontent.com/43996225/200688413-94b68f66-5288-47a2-81ad-eb485dcd0a90.mp4

Expensify/Expensify Issue URL: Issue reported by: Applause internal team Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

melvin-bot[bot] commented 2 years ago

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

jliexpensify commented 2 years ago

@kavimuru I'm actually not seeing this behaviour on my side (I'm also in Staging):

image

Can this be reliably reproduced? Videos would be great. Assigning back to you for further investigation.

kavimuru commented 2 years ago

@jliexpensify I can repro with CAD and the video already attached in the bug is for the currency UAH

https://user-images.githubusercontent.com/43996225/200691806-8c23a7b0-f9d7-40d4-a2e3-3d3acaec9c22.mp4

jliexpensify commented 2 years ago

@kavimuru thanks for the video - your reproduction steps were in the wrong order in the original issue so i fixed that.

But I've tested again, multiple times - I still can't reproduce. I'm still getting the same currency symbol for both requests:

image

melvin-bot[bot] commented 1 year ago

@kavimuru Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 1 year ago

@kavimuru 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

melvin-bot[bot] commented 1 year ago

@kavimuru Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

melvin-bot[bot] commented 1 year ago

@kavimuru 12 days overdue now... This issue's end is nigh!

melvin-bot[bot] commented 1 year ago

This issue has not been updated in over 14 days. @kavimuru eroding to Weekly issue.

jasperhuangg commented 1 year ago

Hey guys, I can repro! @jliexpensify notice how the symbol changes from CA$ to C$ at roughly the 4 second mark on the money request report item in the background:

https://user-images.githubusercontent.com/31285285/204367138-a672ca88-cb75-40d8-8583-c2dbe82830bb.mov

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @strepanier03 (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~01b26c2eca8bb1766c

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

melvin-bot[bot] commented 1 year ago

Current assignee @jasperhuangg is eligible for the External assigner, not assigning anyone new.

jasperhuangg commented 1 year ago

Will be looking into this one today myself, but feel free to submit proposals if I haven't submitted a PR yet!

jasperhuangg commented 1 year ago

Actually, this one might have to be internal, it looks like there's a mismatch between the optimistic report action we're building in the front-end and the one that's returned from OpenReport. Looks like this only happens when you request money in a few of the currencies where the symbols and names differ.

When we first request money

html: "Requested CA$34.00 from 9@test.com"
isEdited: false
text: "Requested CA$34.00 from 9@test.com"
type: "COMMENT"

OpenReport response

html: "Requested C$34.00 from 9@test.com"
isEdited: false
text: "Requested C$34.00 from 9@test.com"
type: "COMMENT"

I'll look more into this after lunch

jliexpensify commented 1 year ago

Awesome stuff, thanks @jasperhuangg !

jasperhuangg commented 1 year ago

Actually after some further investigating it appears that this happens is because the Intl built in library doesn't provide symbols for a number of the currencies we support, so it just returns its currency code instead as a default. In the back-end, we have an actual map of currency code to symbol, which is why we're able to get the symbols.

The currency picker is intended to show the ISO4217 standard currencies, which our back-end already uses.

jasperhuangg commented 1 year ago

Still pending discussion here, not overdue!

melvin-bot[bot] commented 1 year ago

@jasperhuangg Whoops! This issue is 2 days overdue. Let's get this updated quick!

jasperhuangg commented 1 year ago

Good discussion happening in the Slack thread, this is not overdue!

jasperhuangg commented 1 year ago

Ionatan and I are more or less on the same page now, just working on ideating a solution

jasperhuangg commented 1 year ago

PR incoming...

jasperhuangg commented 1 year ago

Still working on that PR! Hope to have something tested up today!

jasperhuangg commented 1 year ago

PRs up and ready for review!

jasperhuangg commented 1 year ago

Realized the testing for https://github.com/Expensify/App/pull/13432 is actually a lot more involved than I had previously assumed so I'm putting the PR back on draft while I iron out the testing steps and screen captures.

jasperhuangg commented 1 year ago

PRs are both under review, I'm also in @iwiznia 's NewDot DMs to clarify a few things 👀

jasperhuangg commented 1 year ago

Just a quick update on this one, more great discussion is happening here in Slack about the best approach to solve this issue. We may have to HOLD this on a separate issue that @aldo-expensify is working on, but as for now I'm gonna put the Planning label back on.

jasperhuangg commented 1 year ago

This is solved in Aldo's PR! https://github.com/Expensify/Web-Expensify/pull/35845

jasperhuangg commented 1 year ago

@aldo-expensify assigning ya so you can also get credit on this!

melvin-bot[bot] commented 1 year ago

@jasperhuangg, @aldo-expensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

aldo-expensify commented 1 year ago

Redoing the PHP API PR: https://github.com/Expensify/Web-Expensify/pull/35908

(Salt PR was salted adding the missing dependency)

melvin-bot[bot] commented 1 year ago

@jasperhuangg, @aldo-expensify Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 1 year ago

@jasperhuangg, @aldo-expensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

aldo-expensify commented 1 year ago

No overdue, the https://github.com/Expensify/Web-Expensify/pull/35908 is in staging

jasperhuangg commented 1 year ago

https://github.com/Expensify/Web-Expensify/pull/35908 is on prod, closing!