[HOLD for payment 2024-01-16] [$500] Web - IOUs - location text does not change on selecting 'Use current location'

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: v1.4.12-0
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
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation: @

Action Performed:

1. Open the app
2. Click on green plus and click request money -> distance
3. Open start/finish
4. Search any location and select the result
5. Again open same field used in step 3
6. Ensure that location action is disabled
7. Remove current location text and select 'Use current location', it will trigger error
8. Now enable location access and again select 'Use current location'
9. Observe now that on main distance page, it still displays older location instead of 'Your location', open the same field as step 3 and observe that inside it displays 'Your location' properly

Expected Result:

App should display 'Your location' on selecting 'Use current location' in any waypoint

Actual Result:

App displays older location value instead of 'Your location' on selecting 'Use current location' in any waypoint if location access is enabled after error is triggered

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6312011_1702489356189.windows_chrome_-_your_location_not_displayed.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0168cfb95cbebc1039
• Upwork Job ID: 1735036732238655488
• Last Price Increase: 2023-12-27
• Automatic offers:
• ZhenjaHorbach | Contributor | 28071864

Issue Owner

Current Issue Owner: @stephanieelliott

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0168cfb95cbebc1039

[melvin-bot]

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

[melvin-bot]

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

[ZhenjaHorbach]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Web - IOUs - location text does not change on selecting 'Use current location'

What is the root cause of that problem?

We don't pass name param

App/src/components/AddressSearch/index.js

Lines 338 to 342 in 3df907c

	const location = {
	lat: successData.coords.latitude,
	lng: successData.coords.longitude,
	address: CONST.YOUR_LOCATION_TEXT,
	};

As a result we pass undefined value inside Transaction.saveWaypoint

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 169 in 3df907c

name: values.name,

As a result, the previous value is used

What changes do you think we should make in order to solve the problem?

In order to fix this, we can use the implementation from WaypointEditor

we can set name: values.name || null,

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 169 in 3df907c

name: values.name,

Plus here(optional)

App/src/pages/iou/request/step/IOURequestStepWaypoint.js

Line 143 in 3df907c

name: values.name,

Like here

App/src/pages/iou/WaypointEditor.js

Line 167 in 3df907c

name: values.name || null,

What alternative solutions did you explore? (Optional)

As alternative, we can pass name param (CONST.YOUR_LOCATION_TEXT or null ) or set name === null by default in Transaction.saveWaypoint (But since we use Transaction.saveWaypoint in two places. It is not very required )

                const location = {
                    lat: successData.coords.latitude,
                    lng: successData.coords.longitude,
                    address: CONST.YOUR_LOCATION_TEXT,
	            name: CONST.YOUR_LOCATION_TEXT, //As alternative null
                };

App/src/components/AddressSearch/index.js

Lines 338 to 342 in 3df907c

	const location = {
	lat: successData.coords.latitude,
	lng: successData.coords.longitude,
	address: CONST.YOUR_LOCATION_TEXT,
	};

[DylanDylann]

Proposal

Please re-state the problem that we are trying to solve in this issue.

App displays older location value instead of 'Your location' on selecting 'Use current location' in any waypoint if location access is enabled after error is triggered

What is the root cause of that problem?

we don't pass the name field to the location here.

App/src/components/AddressSearch/index.js

Lines 338 to 342 in b950c1e

	const location = {
	lat: successData.coords.latitude,
	lng: successData.coords.longitude,
	address: CONST.YOUR_LOCATION_TEXT,
	};

So that the name field will be undefined. Currently, when updating waypoints we are using MERGE method, and the old name value will be retained. It caused this bug

What changes do you think we should make in order to solve the problem?

In here

App/src/libs/actions/Transaction.ts

Line 61 in b950c1e

Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION : ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {

We can combine the old transaction and the new change, then use SET method instead of MERGE method as we did here
#30290 (comment)
Note that: MERGE method caused many similar bugs so we usually use SET method when updating waypoints like here

App/src/libs/actions/Transaction.ts

Line 162 in b950c1e

Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, newTransaction);

What alternative solutions did you explore? (Optional)

If we still use MERGE method, I think we should have a general solution instead of fixing case by case (It is so easy to miss similar bugs elsewhere)

App/src/libs/actions/Transaction.ts

Line 61 in b950c1e

Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION : ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {

In here we can check if the name field is undefined we will set it to null or address field

[kaushiktd]

Please re-state the problem that we are trying to solve in this issue.

Web - IOUs - location text does not change on selecting 'Use current location'

What is the root cause of that problem?

When we select use current location name field value show as undefined in selectWaypoint
https://github.com/Expensify/App/blob/main/src/pages/iou/request/step/IOURequestStepWaypoint.js#L169

name: [values.name],

What changes do you think we should make in order to solve the problem?

https://github.com/Expensify/App/blob/main/src/pages/iou/request/step/IOURequestStepWaypoint.js#L169

Implement more robust handling of undefined values for name in the Transaction.saveWaypoint method by providing a default value explicitly:

name: [values.name](http://values.name/) !== undefined ? [values.name](http://values.name/) : CONST.YOUR_LOCATION_TEXT,

This code explicitly checks for undefined values and assigns CONST.YOUR_LOCATION_TEXT as a fallback to ensure consistency.

Video:-

https://drive.google.com/file/d/1gl9HvRNJUpybpImBi6ey1z4u-ktt-tNd/view?usp=sharing

[allstarsmen]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Web - IOUs - location text does not change on selecting 'Use current location'

What is the root cause of that problem?

We use 'Onyx.merge()' to update the 'waypoint' object, but the 'Onyx.merge()' will ignore the property with 'undefined' value, so in this case, all properties in 'waypoint' will be updated except the 'name' property.

What changes do you think we should make in order to solve the problem?

In here

App/src/libs/actions/Transaction.ts

Line 60 in b950c1e

function saveWaypoint(transactionID: string, index: string, waypoint: RecentWaypoint | null, isDraft = false) {

We should set the name of the current location to an empty string.

What alternative solutions did you explore? (Optional)

No

Result

Before.mov

After.mov

[stephanieelliott]

Hey @mananjadhav can you review the proposals please?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[mananjadhav]

I’ll reproduce the issue and review the proposals by tomorrow.

[melvin-bot]

@mananjadhav, @stephanieelliott Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@mananjadhav @stephanieelliott this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[mananjadhav]

I'll review the proposals by tomorrow. Was out of office earlier and now finishing up priority PRs.

[mananjadhav]

I don't think we should be using Onyx.set. Adding null looks like a straight forward solution. Okay with @ZhenjaHorbach's proposal here.

We can work out in the PR places where saveWaypoint is called and accordingly decide the change in the saveWaypoint or the params.

🎀 👀 🎀 C+ reviewed.

[melvin-bot]

Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

@mananjadhav, @stephanieelliott, @mountiny, @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@mananjadhav, @stephanieelliott, @mountiny, @ZhenjaHorbach Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mountiny]

Pr is in review

[melvin-bot]

@mananjadhav @stephanieelliott @mountiny @ZhenjaHorbach this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[stephanieelliott]

Adjusting labels to reflect the PR being in review, seems they didn't update automatically like they were supposed to

[stephanieelliott]

PR is on staging

[stephanieelliott]

This was deployed to production on January 9, updating title and labels to reflect 7 day hold

[mananjadhav]

I couldn't pinpoint the specific PR that must've caused this. I also don't think we need a regression test for this.

@stephanieelliott This is ready for payout, can you please post a payment summary?

[stephanieelliott]

Summarizing payment on this issue:

• Contributor: @ZhenjaHorbach $500 PAID via Upwork
• Contributor+: @mananjadhav $500 please request via Newdot

Upwork job is here: https://www.upwork.com/jobs/~1737876297360310272"

[kaushiktd]

@stephanieelliott When should I expect payment?

[mananjadhav]

@stephanieelliott When should I expect payment?

@kaushiktd Payment regarding?

[kaushiktd]

Yes @mananjadhav

[JmillsExpensify]

$500 payment approved to @mananjadhav based on this comment.

[kaushiktd]

@JmillsExpensify any update on my payment?

[stephanieelliott]

@kaushiktd can you clarify what specifically you are expecting payment for? No payment is due on this issue as your proposal was not selected and you were not hired for this job. For more context into how and when Expensify issues payment please see contributing.md

[kaushiktd]

It seems I made a mistake. Sorry! I misunderstood this issue for the one I got selected. 😅