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.14k stars 2.64k forks source link

[$250] Expense - Missing spacing between currency and amount in unapprove system message #44975

Open lanitochka17 opened 1 week ago

lanitochka17 commented 1 week 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!


Version Number: 9.0.5-3 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

Expected Result:

The amount in the unapprove system message will have a space between the currency and amount

Actual Result:

The amount in the unapprove system message does not have a space between the currency and amount

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/8ab3e8c6-3c6c-434d-9b6e-82d0708294b1

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01791cd2da764f51cc
  • Upwork Job ID: 1810328690048713566
  • Last Price Increase: 2024-07-08
Issue OwnerCurrent Issue Owner: @ikevin127
melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @MariaHCD (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

github-actions[bot] commented 1 week ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
lanitochka17 commented 1 week ago

We think that this bug might be related to #wave-collect - Release 1

lanitochka17 commented 1 week ago

@MariaHCD FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

MariaHCD commented 1 week ago

Not able to reproduce the issue on dev:

<img width="940" alt="Screenshot 2024-07-08 at 3 56 51 PM" src="https://github.com/Expensify/App/assets/12268372/ec189b97-9656-496a-81e3-19d561a90c1f">

Will try with the MYR currency

Julesssss commented 1 week ago

Oops sorry @MariaHCD please ignore that ^

Julesssss commented 1 week ago

I'm going to check it off the list regardless as it is a new feature, and a minor issue.

MariaHCD commented 1 week ago

Sure, I was just about to say this doesn't need to block the deploy

MariaHCD commented 1 week ago

Able to reproduce with MYR

Screenshot 2024-07-08 at 4 05 39 PM Screenshot 2024-07-08 at 4 09 24 PM

Looks like the backend might be storing it incorrectly?

cc: @Beamanator do you have any context on the unapproved report action?

Beamanator commented 1 week ago

Ooh that's interesting... I did just add this kind of stuff to the backend for the Unapproved report action, but I wonder if this is a bug in Currency::format? For that specific currency? Is MYR different than RM? What's RM?

MariaHCD commented 1 week ago

From a quick search:

RM is the abbreviation for the Malaysian Ringgit, and MYR is the currency code.

So possible it's because of something with this specific currency

zulsadiq commented 1 week ago

RM is the currency symbol so there is no space, because with £ or $ you don't need a space.

I'm wondering if it should be showing RM also for the initial approved message too?

I can submit a proposal to fix this if you can confirm the behaviour? Although looks like the fix could be backend based on @Beamanator 's comment above.

melvin-bot[bot] commented 1 week ago

📣 @zulsadiq! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
zulsadiq commented 1 week ago

Contributor details Your Expensify account email: zulfus@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01078c2aff6a1baa79?mp_source=share

melvin-bot[bot] commented 1 week ago

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

Beamanator commented 1 week ago

@MariaHCD let me know if you want me to take over this one, it looks like it won't be too hard to debug, I'd start by looking at what we store in the DB for the Approved report action, compare to Unapproved, and check the difference

If we find that this indeed is specific to the Unapproved report action somehow, then we should add this to #wave-control

MariaHCD commented 1 week ago

Go for it, @Beamanator :) Thanks

Beamanator commented 1 week ago

Ok so I created the request in MYR but my workspace default currency is EGP and I'm even seeing this with EGP!

Screenshot 2024-07-11 at 2 44 08 PM
Beamanator commented 1 week ago

Also, it's pretty obvious that the currency doesn't match the amount in Unapprove 😅

Beamanator commented 1 week ago

Hmm I gotta get back to this Monday, didn't have enough time today

Beamanator commented 5 days ago

OOO today lawl will check tomorrow

melvin-bot[bot] commented 1 day ago

@Beamanator Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Beamanator commented 1 day ago

Ok I'm figuring out we somehow have this \u00a0 "non-breaking space" in the Approved report actions' message, and no space in originalMessage 🤔

Beamanator commented 1 day ago

Huh so when i sign out / in, it actually shows up correctly always 🙃

OpenReport response:

           "message": [
                        {
                            "type": "COMMENT",
                            "html": "unapproved EGP\u00a0100.00",
                            "text": "unapproved EGP\u00a0100.00",
                            "isEdited": false,
                            "whisperedTo": [],
                            "isDeletedParentAction": false,
                            "deleted": ""
                        }
                    ],
                    "originalMessage": {
                        "IOUReportID": 1961298029364531,
                        "amount": 10000,
                        "currency": "EGP",
                        "html": "unapproved EGP100.00",
                        "lastModified": "2024-07-18 23:43:21.687"
                    },
Beamanator commented 1 day ago

bringing to slack - https://expensify.slack.com/archives/C03TQ48KC/p1721347465767429