[HOLD for payment 2023-08-17] [$1000] The 'unit' in the track distance in offline mode doesn't change to kilometers

[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. Go to web chrome
2. Turn off your wifi
3. Create a new workspace and go to reimburse expenses
4. Try to change the unit to kilometers
5. Notice that it doesn't change to kilometers

Expected Result:

Kilometers should be selected upon clicking kilometers in the unit dropdown

Actual Result:

The kilometer is not selected on clicking kilometers in the unit dropdown

Workaround:

unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.97-2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

error-2023-04-08_20.24.12.mp4

Recording.174.mp4

Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680965013944919

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cc512639a78229d9
• Upwork Job ID: 1648348722814038016
• Last Price Increase: 2023-05-16

[MelvinBot]

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

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[trjExpensify]

Oh, interesting! I can reproduce this. You don't have to create a new workspace either. It's simply just not possible to change the unit when offline. I wonder whether this was missed during the offline API refactors? If so, it would need to be internal I think.

Going to add Engineering for a second opinion.

[MelvinBot]

Triggered auto assignment to @amyevans (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[trjExpensify]

@amyevans do you have any thoughts on this one? Will it need to be internal?

[amyevans]

Oops, sorry I missed this earlier in the week!

Interesting, so we're hitting this block:

App/src/pages/workspace/reimburse/WorkspaceReimburseView.js

Lines 168 to 173 in ea3d656

	if (!distanceCustomUnit) {
	Log.warn('Policy has no customUnits, returning early.', {
	policyID: this.props.policy.id,
	});
	return;
	}

and return early.

That is hit because when we create a workspace optimistically on the FE, this is the only data generated:

And it isn't until we call OpenWorkspaceReimburseView (when online) that we get full policy data back from the API, which includes the custom units data (which is keyed by ID), for example:

I think to fully support changing the distance unit on a new workspace while offline, we'd have to generate custom unit IDs on the FE, which sounds like a bigger undertaking than is prudent for the scope of a bug report.

I see we use the Offline with Feedback (50% opacity) pattern on the Workspace Initial Page when generated offline though, so maybe we should just be properly extending that pattern to the Reimburse Expenses page. Does that sound right to you @trjExpensify? If so, we should be fine to open up to External I think.

[MelvinBot]

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

[trjExpensify]

Nice, thanks Amy!

I see we use the Offline with Feedback (50% opacity) pattern on the Workspace Initial Page when generated offline though, so maybe we should just be properly extending that pattern to the Reimburse Expenses page.

Yeah, creating a workspace is Pattern B. What do you mean by extending it to the reimburse expenses page? As in, allow them to set a rate and change the unit like they can invite a member on the Manage members page etc?

[amyevans]

I mean that, currently, the reimburse expenses page is full opacity when offline:

which implies to the user that they can set a rate and change the unit when in fact they cannot. So I'm suggesting we update the reimburse expenses page to be half opacity.

And for what it's worth looking at the code, we do already have the <OfflineWithFeedback> component wrapping the Track distance section, so I'm not sure why it isn't 50% opacity - but that seems like the bug to fix.

[amyevans]

Anyway let's label External to get some community input 😄

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[trjExpensify]

Can you explain a bit more on that one @ShogunFire? We don't need to account for the push to page aspect anymore, but I don't think we're doing optimistic customUnit IDs yet?

[ShogunFire]

Sorry @trjExpensify you are right, the problem is still here

[trjExpensify]

👋 @amyevans any cycles for this one?

[amyevans]

Last week was harder with some OOO days, but I'm planning to dedicate some time here this week

[trjExpensify]

Niceee! Did you get a chance? Interestingly, it came up again here recently.

[amyevans]

I haven't put a ton of time into it yet, but I've done a bit of research. It looks like we generate the distance custom unit in Web-E here. I'm not sure if we should be duplicating all of the logic in App now (default distance category based on IP, default IRS rate, etc) or only generating the IDs 🤔

[amyevans]

Started some very rough draft PRs:
https://github.com/Expensify/Web-Expensify/pull/38312
#23600

[amyevans]

PRs are ready to go for real now 🌮

[trjExpensify]

Niceee! Thanks, @amyevans :)

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.52-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Optimistic custom unit IDs #23600

If no regressions arise, payment will be issued on 2023-08-17. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @fedirjh requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[trjExpensify]

Okay, so time to settle up! Confirming payments as:

• @fedirjh - $1,000 for C+ review of the internal PR
• @priya-zha - $250 for the bug report.

Offers sent!

[trjExpensify]

@fedirjh, paid!

[trjExpensify]

@priya-zha - paid!