Unable to change rate or unit for track distance

[kavimuru]

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. Load NewDot on staging on an account with a workspace.
2. Click Settings.
3. Click Workspaces.
4. Click into your workspace.
5. Click Reimburse expenses.
6. Click the Unit dropdown menu, and select a different unit, while online.
7. Confirm you receive error message and unit does not change.

Expected Result:

Unit change and saved, with no error message for being offline.

Actual Result:

"An unexpected error occurred, please try again. If the error persists, please reach out to
concierge@expensify.com
.Your changes couldn't be saved. The workspace was modified while you were offline, please try again."

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.2.38-5
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:

Recording.1090.mp4

Expensify/Expensify Issue URL:
Issue reported by: @BegadTarek
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1670943899681949

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c6ee850ce85c0d80
• Upwork Job ID: 1605311368204656640
• Last Price Increase: 2022-12-20

[melvin-bot]

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

[joekaufmanexpensify]

Triaging:

From this SO: https://stackoverflow.com/c/expensify/questions/14418

• The "bug" is actually a bug: Yes, this is not expected
• The bug is not a duplicate report of an existing GH (close the GH and add any novel details to the original GH instead): There was a similar report, but we closed it without taking any action as we thought the issue was not reproducible. It's reproducible now, so I recommend we proceed with this issue.
• The bug is reproducible, following the reproduction steps: Yes, shown below.
• If you're unable to reproduce the bug, add the Needs reproduction label. Comment on the issue outlining the steps you took to try to reproduce the bug, your results and tag the issue reporter and the Applause QA member who created the issue. Ask them to clarify reproduction steps and/or to check the reproduction steps again. Close issue.:
• The GH template is filled out as fully as possible -- this means the GH body and title are clear (ie. could an external contributor understand it and work on it?): Yes.
• The GH was created by an Expensify employee or a QA tester: Yes,
• If the bug is an OldDot issue, you should decide whether it is widespread enough to justify fixing or it is better to wait for reunification. If it's better to wait, close the GH & provide this justification: N/A
• If there's a link to Slack, check the discussion to see if we decided not to fix it: We decided to fix it in Slack.
• Decide if the GH should be resolved by an External contributor or Internal engineer, add the appropriate label: assigning to engineering to decide.

[joekaufmanexpensify]

Reproduced on staging web:

[pecanoro]

I am going to see if I can see the cause of this bug since it seems it's reproducible!

[joekaufmanexpensify]

Sounds good! LMK if you plan to take this one/whether it should be internal/external.

[pecanoro]

Found the logs here! This is probably internal since the error is coming from the back-end. However, I am still researching.

[joekaufmanexpensify]

👍

[jasperhuangg]

Can help take a look over here, I wrote the original command so I can probably see what's changed

Can confirm this is also happening locally and on production.

[jasperhuangg]

Figured out why, but not sure how this got past QA

We expect an array of accountIDs here and here. But Auth::getAccountIDsByEmails returns an associative array of email => accountID that we pass directly in instead.

This causes Pusher to think that we're passing an empty array of accountIDs. We should update the function to throw a friendlier error message if we're trying to send a Pusher update to an empty array of accountIDs. We should also pass the correct accountIDs 😆

PR incoming...

[Beamanator]

Woah nice catch @jasperhuangg ! I literally just implemented a similar fix for SplitBill here: https://github.com/Expensify/Web-Expensify/pull/35817 but didn't think about making a friendlier error message, that's a great idea

Also FYI I'm looking into if this is also happening for AddMembersToWorkspace, since we got the same pusher error during that command (here)

[Beamanator]

Looks like there's even a few more places we may need to update:

• updateWorkspaceCustomUnit: https://github.com/Expensify/Web-Expensify/blob/0641db2b9d64efd90167a8e4c58e6f9c48790291/lib/PolicyAPI.php#L3338-L3339
• updateWorkspaceCustomUnitRate: https://github.com/Expensify/Web-Expensify/blob/0641db2b9d64efd90167a8e4c58e6f9c48790291/lib/PolicyAPI.php#L3743-L3744
• And possibly somewhere in the addMembersToWorkspace function, still looking for that fix

[jasperhuangg]

updateWorkspaceCustomUnit: https://github.com/Expensify/Web-Expensify/blob/0641db2b9d64efd90167a8e4c58e6f9c48790291/lib/PolicyAPI.php#L3338-L3339
updateWorkspaceCustomUnitRate: https://github.com/Expensify/Web-Expensify/blob/0641db2b9d64efd90167a8e4c58e6f9c48790291/lib/PolicyAPI.php#L3743-L3744

yup, I already have local changes fixing those @Beamanator

[jasperhuangg]

@Beamanator Should we open a separate issue that includes this? Or should we just fix them all separately and link them to this issue?

[Beamanator]

I'm thinking the addMembersToWorkspace issue is actually something separate, and I have an issue for this: https://github.com/Expensify/Expensify/issues/249216

So maybe you can fix the other 2 in this issue?

[jasperhuangg]

@Beamanator Sounds like a plan, I'll also include some type of log that should fire next time someone tries to pass an associative array.

[Beamanator]

@jasperhuangg I would add a 👍 emoji on your comment but somehow I can't at the moment 😅

[jasperhuangg]

@Beamanator sameee hahah I kept reaching for it–here, you can have these in the meantime 😆

👍👍👍👍👍👍👍👍👍👍👍

[jasperhuangg]

Weird, looks like Github isn't aware that I linked this issue in my PR 🤔

Regardless here's the PR fixing this: https://github.com/Expensify/Web-Expensify/pull/35826

[Beamanator]

Noice @jasperhuangg ! I'll review! Looks like you are trying to point to this PR by the way :D https://github.com/Expensify/Web-Expensify/pull/35826

[joekaufmanexpensify]

@BegadTarek Mind confirming your upwork handle/applying for this job so we can pay you the $250 reporting bonus?

[joekaufmanexpensify]

Changing prioritization so we can issue payment and then complete the rest of BZ checklist next week

[joekaufmanexpensify]

@BegadTarek offer for $250 sent via upwork!

[joekaufmanexpensify]

Looking for BZ volunteer to handle this payment on 2022-12-27 as I'm going to be OOO next week.

[joekaufmanexpensify]

@mallenexpensify is taking over as BZ here while I'm OOO next week. Thanks Matt! A few notes:

• BZ portion of BZ checklist is all set. There are a few remaining steps that @pecanoro needs to handle.
• This was internal PR in Expensify web repo. So the only payment needed is to the bug reporter. I just sent them a contract today.
• Because this was in the Expensify web repo, the issue automation did not work. So I added the hold for payment and changed to weekly manually. So the issue will not change back to daily automatically on the 27th, even though that's when we'll be all set to issue payment.

Let me know if you have any questions!

[pecanoro]

Ohh I forgot to update this. This was the offending PR but I am wondering why we didn't catch it for 3 months so I am thinking maybe something else changed in the meantime, but I could not find it.

[pecanoro]

@mallenexpensify Are we free to close?

[mallenexpensify]

I think so, I removed my assignment and added Joe back, since I didn't do anything (and I have no idea how 'credit' works out for BZ stuff)

[joekaufmanexpensify]

@mallenexpensify Did you pay the contributor who reported? No worries if not, I can do it otherwise!

[joekaufmanexpensify]

Actually, I was able to confirm we have not yet paid this out in upwork:

[joekaufmanexpensify]

@BegadTarek $250 sent and contract ended!

[joekaufmanexpensify]

Closed upwork posting!

[joekaufmanexpensify]

Bug is fixed, BZ checklist complete, and payment sent. This is all set!

[mallenexpensify]

oh man.. I was the opposite of 'help'.
Sorry I missed ya @BegadTarek

[BegadTarek]

Hahaha no worries, I thought there was probably a backlog due to the holidays.. hope you both have a happy new year