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

izarutskaya · 2023-12-13T20:39:24Z

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: 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:

Open the app
Click on green plus and click request money -> distance
Open start/finish
Search any location and select the result
Again open same field used in step 3
Ensure that location action is disabled
Remove current location text and select 'Use current location', it will trigger error
Now enable location access and again select 'Use current location'
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?

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 · 2023-12-13T20:39:28Z

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

melvin-bot · 2023-12-13T20:39:29Z

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

melvin-bot · 2023-12-13T20:39:30Z

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 · 2023-12-13T20:39:34Z

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

ZhenjaHorbach · 2023-12-13T20:54:36Z

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

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

Plus here(optional)

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

Line 143 in 3df907c

Like here

App/src/pages/iou/WaypointEditor.js

Line 167 in 3df907c

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 · 2023-12-14T02:22:43Z

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 · 2023-12-14T08:29:35Z

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 · 2023-12-14T09:32:23Z

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 · 2023-12-18T22:48:01Z

Hey @mananjadhav can you review the proposals please?

melvin-bot · 2023-12-20T16:00:06Z

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

mananjadhav · 2023-12-20T16:14:15Z

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

melvin-bot · 2023-12-26T11:35:20Z

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

melvin-bot · 2023-12-27T11:06:57Z

@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 · 2023-12-27T16:01:46Z

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

mananjadhav · 2023-12-27T16:24:41Z

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

mananjadhav · 2023-12-28T16:50:07Z

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 · 2023-12-28T16:50:15Z

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

mountiny · 2023-12-28T18:02:17Z

Double checking the expected results

mountiny · 2023-12-28T20:14:45Z

Thanks!

@ZhenjaHorbach can you raise a pr please?

ZhenjaHorbach · 2023-12-28T20:16:43Z

Thanks!

@ZhenjaHorbach can you raise a pr please?

Hello )
No problem
I'll do this now

melvin-bot · 2024-01-01T11:47:06Z

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

melvin-bot · 2024-01-01T17:44:26Z

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

mountiny · 2024-01-02T13:40:10Z

Pr is in review

melvin-bot · 2024-01-03T17:03:01Z

@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 · 2024-01-04T00:02:32Z

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

stephanieelliott · 2024-01-08T17:06:29Z

PR is on staging

stephanieelliott · 2024-01-10T20:11:04Z

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

mananjadhav · 2024-01-19T04:31:49Z

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 · 2024-01-19T19:11:00Z

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 · 2024-01-20T02:50:40Z

@stephanieelliott When should I expect payment?

mananjadhav · 2024-01-20T21:44:54Z

@stephanieelliott When should I expect payment?

@kaushiktd Payment regarding?

kaushiktd · 2024-01-21T02:51:03Z

Yes @mananjadhav

JmillsExpensify · 2024-01-22T15:37:22Z

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

kaushiktd · 2024-01-23T14:09:47Z

@JmillsExpensify any update on my payment?

stephanieelliott · 2024-01-23T15:15:53Z

@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 · 2024-01-23T15:17:33Z

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

izarutskaya added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 13, 2023

melvin-bot bot assigned stephanieelliott Dec 13, 2023

melvin-bot bot changed the title ~~Web - IOUs - location text does not change on selecting 'Use current location'~~ [$500] Web - IOUs - location text does not change on selecting 'Use current location' Dec 13, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 13, 2023

melvin-bot bot assigned mananjadhav Dec 13, 2023

melvin-bot bot added the Overdue label Dec 18, 2023

melvin-bot bot removed the Overdue label Dec 18, 2023

melvin-bot bot added the Overdue label Dec 25, 2023

melvin-bot bot removed the Overdue label Dec 27, 2023

melvin-bot bot assigned mountiny Dec 28, 2023

ZhenjaHorbach mentioned this issue Dec 28, 2023

Location text does not change on selecting Use current location #33726

Merged

50 tasks

melvin-bot bot added the Overdue label Jan 1, 2024

melvin-bot bot removed the Overdue label Jan 2, 2024

melvin-bot bot added the Engineering label Jan 3, 2024

stephanieelliott added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jan 4, 2024

stephanieelliott added the Awaiting Payment Auto-added when associated PR is deployed to production label Jan 10, 2024

stephanieelliott changed the title ~~[$500] Web - IOUs - location text does not change on selecting 'Use current location'~~ [HOLD for payment 2024-01-16] [$500] Web - IOUs - location text does not change on selecting 'Use current location' Jan 10, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 17, 2024

stephanieelliott closed this as completed Jan 19, 2024

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

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

Comments

izarutskaya commented Dec 13, 2023 • edited by mountiny Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 13, 2023

melvin-bot bot commented Dec 13, 2023

melvin-bot bot commented Dec 13, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Dec 13, 2023

ZhenjaHorbach commented Dec 13, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

DylanDylann commented Dec 14, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

kaushiktd commented Dec 14, 2023 • edited Loading

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

What is the root cause of that problem?

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

Video:-

allstarsmen commented Dec 14, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Result

stephanieelliott commented Dec 18, 2023

melvin-bot bot commented Dec 20, 2023

mananjadhav commented Dec 20, 2023

melvin-bot bot commented Dec 26, 2023

melvin-bot bot commented Dec 27, 2023

melvin-bot bot commented Dec 27, 2023

mananjadhav commented Dec 27, 2023 • edited Loading

mananjadhav commented Dec 28, 2023

melvin-bot bot commented Dec 28, 2023

mountiny commented Dec 28, 2023

mountiny commented Dec 28, 2023

ZhenjaHorbach commented Dec 28, 2023

melvin-bot bot commented Jan 1, 2024

melvin-bot bot commented Jan 1, 2024

mountiny commented Jan 2, 2024

melvin-bot bot commented Jan 3, 2024

stephanieelliott commented Jan 4, 2024

stephanieelliott commented Jan 8, 2024

stephanieelliott commented Jan 10, 2024

mananjadhav commented Jan 19, 2024

stephanieelliott commented Jan 19, 2024

kaushiktd commented Jan 20, 2024

mananjadhav commented Jan 20, 2024

kaushiktd commented Jan 21, 2024

JmillsExpensify commented Jan 22, 2024

kaushiktd commented Jan 23, 2024

stephanieelliott commented Jan 23, 2024

kaushiktd commented Jan 23, 2024 • edited Loading

izarutskaya commented Dec 13, 2023 •

edited by mountiny

Loading

ZhenjaHorbach commented Dec 13, 2023 •

edited

Loading

kaushiktd commented Dec 14, 2023 •

edited

Loading

allstarsmen commented Dec 14, 2023 •

edited

Loading

mananjadhav commented Dec 27, 2023 •

edited

Loading

kaushiktd commented Jan 23, 2024 •

edited

Loading