[HOLD for payment 2023-10-09] [$500] [Distance] - Ability to request money without filling start and finish points #27383

kbecciv · 2023-09-13T17:56:39Z

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:

Request money -> distance
Change /distance in the URL to /participants
Choose a workspace then click request

Expected Result:

The app should redirect the user to https://staging.new.expensify.com/request/new/distance

Actual Result:

The app allows users to request money

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.69.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
Notes/Photos/Videos: Any additional supporting documentation

participants-bug.webm

Recording.4449.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01bf360a8a0ff38420
Upwork Job ID: 1702018479558987776
Last Price Increase: 2023-09-20
Automatic offers:

DylanDylann | Contributor | 26773601

dukenv0307 · 2023-09-13T17:56:40Z

Proposal

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

[Distance] Ability to request money without filling start and finish points

What is the root cause of that problem?

We don't allow user to back to the first step if the request is a distance request

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Line 66 in 4d104f5

if (!isDistanceRequest && props.iou.id) {

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Line 78 in 4d104f5

    
           if (!isDistanceRequest && ((props.iou.amount === 0 && !props.iou.receiptPath) || shouldReset)) {

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

We should remove the check !isDistanceRequest to make user can reset to the first step

What alternative solutions did you explore? (Optional)

melvin-bot · 2023-09-13T17:56:44Z

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

melvin-bot · 2023-09-13T17:56:44Z

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

melvin-bot · 2023-09-13T17:56:46Z

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-09-13T17:56:48Z

Triggered auto assignment to @tjferriss (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

melvin-bot · 2023-09-13T17:56:53Z

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

DylanDylann · 2023-09-13T18:50:23Z

Proposal

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

Ability to request money without filling start and finish points

What is the root cause of that problem?

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 79 to 81 in 7deca7a

    
           if (!isDistanceRequest && ((props.iou.amount === 0 && !props.iou.receiptPath) || shouldReset)) { 
        
               navigateBack(true); 
        
           }

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 67 to 69 in 7deca7a

    
           if (!isDistanceRequest && props.iou.id) { 
        
               navigateBack(true); 
        
           }

Let's see this code, we only navigate back if itn't distance request. It seems that we want to prevent user come back in distance request

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

We should update the logic to allow users:

Coming back if it isn't distance request
Coming back if it is distance request and there are no waypoint (no route)

To do that we can get waypoint from transaction like this

const transaction = TransactionUtils.getTransaction(props.iou.transactionID);
const isEmptyWaypoint = _.isEmpty(lodashGet(transaction, 'comment.waypoint.waypoint0', {}))

and in here

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 79 to 81 in 7deca7a

    
           if (!isDistanceRequest && ((props.iou.amount === 0 && !props.iou.receiptPath) || shouldReset)) { 
        
               navigateBack(true); 
        
           }

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 67 to 69 in 7deca7a

    
           if (!isDistanceRequest && props.iou.id) { 
        
               navigateBack(true); 
        
           }

update the condition to

(!isDistanceRequest || (isDistanceRequest && isEmptyWaypoint))

instead of only !isDistanceRequest

What alternative solutions did you explore? (Optional)

Same like the main solution but we can use isEmptyRoute instead of isEmptyWaypoint

const isEmptyWaypoint = _.isEmpty(lodashGet(transaction, 'comment.routes', {}))

joekaufmanexpensify · 2023-09-14T15:21:08Z

@tjferriss I'll grab this one as you were the second assignee, and this issue doesn't need two BZ assignees.

joekaufmanexpensify · 2023-09-14T15:28:56Z

Reproduced and agreed that this is a bug. Makes sense to me that we'd just redirect back to https://staging.new.expensify.com/request/new/distance, as that's what we seem to do for manual and scan in this same exact scenario.

CC @neil-marcellini in case you have thoughts on this.

joekaufmanexpensify · 2023-09-17T08:26:00Z

Proposals pending review.

joekaufmanexpensify · 2023-09-18T08:06:45Z

Same

joekaufmanexpensify · 2023-09-19T08:02:11Z

@allroundexperts Could you please take a look at the proposals on this issue? TY!

allroundexperts · 2023-09-19T20:22:24Z

Thanks for your proposal @dukenv0307. In this case, I think its more safer to redirect the user back based on waypoints rather than the amount. This is also what we use when deciding to fetch a route (so its better for consistency as well). As such, I think @DylanDylann's proposal is better.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-09-19T20:22:30Z

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

joekaufmanexpensify · 2023-10-09T20:39:08Z

Great, ty! Changing this to weekly then in the meantime.

joekaufmanexpensify · 2023-10-16T14:53:36Z

Still held

joekaufmanexpensify · 2023-10-23T14:04:45Z

Held

joekaufmanexpensify · 2023-10-30T14:05:25Z

Same

joekaufmanexpensify · 2023-11-06T16:18:53Z

Follow up PR is merged and should be deployed this week!

melvin-bot · 2023-11-07T06:12:47Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

joekaufmanexpensify · 2023-11-13T14:56:17Z

@allroundexperts I see the PR this one was held on got deployed. We can make more progress on this now. Is that right?

allroundexperts · 2023-11-13T22:07:10Z

Yep, we can proceed now. I'm not sure if the second PR caused the regression or not. @DylanDylann Can you please confirm?

DylanDylann · 2023-11-14T11:08:02Z

@allroundexperts Yep, but It seems the regression is fixed by another PR

allroundexperts · 2023-11-14T11:09:14Z

Okay. So it seems this caused 2 regressions. So just a note that the payment should be 1/4.

DylanDylann · 2023-11-14T11:20:41Z

@allroundexperts The first regression is not caused our PR. You can check this comment for more detail

allroundexperts · 2023-11-14T15:30:00Z

Gotcha so half it is then!

joekaufmanexpensify · 2023-11-14T15:32:58Z

Got it. And #28308 was the only thing remaining for this issue, so we're good to do BZ checklist now, issue payment (after 7 day hold) and close?

allroundexperts · 2023-11-14T15:34:23Z

Correct @joekaufmanexpensify.

joekaufmanexpensify · 2023-11-14T15:36:05Z

Sweet. Mind taking care of your portion of the BZ checklist?

joekaufmanexpensify · 2023-11-20T14:29:24Z

bumped in slack

allroundexperts · 2023-11-20T22:03:58Z

Checklist

Modify MoneyRequestConfirmPage for Distance Request #25707
https://github.com/Expensify/App/pull/25707/files#r1399807663
I think better testing would have caught this. We already have a point about deep links in the checklist. As such, discussion on Slack is not needed.
A regression test would help.

Regression Test

Request money -> distance
Change /distance in the URL to /participants

Verify that you are redirected to the new distance request page.

Do we 👍 or 👎 ?

joekaufmanexpensify · 2023-11-21T15:23:45Z

Great, ty! BZ checklist is all set.

joekaufmanexpensify · 2023-11-21T15:29:29Z

All set to issue payment. There was a regression here, so the county due here is:

@DylanDylann - $250 for PR (paid via upwork).
@allroundexperts - $250 for C+ (paid via NewDot).
@hichamcc - $50 for reporting (paid via Upwork).

joekaufmanexpensify · 2023-11-21T15:31:00Z

@DylanDylann $250 sent and contract ended!

joekaufmanexpensify · 2023-11-21T15:33:47Z

Upwork job is closed, so opened new one to pay @hichamcc: https://www.upwork.com/jobs/~01282aaf23d83dd5cc

joekaufmanexpensify · 2023-11-21T15:34:35Z

@hichamcc offer sent for $50!

joekaufmanexpensify · 2023-11-21T15:34:50Z

@allroundexperts could you please request $250 via NewDot and confirm here once done?

joekaufmanexpensify · 2023-11-21T15:43:40Z

@hichamcc $50 sent and contract ended!

JmillsExpensify · 2023-11-22T18:47:34Z

$250 payment approved for @allroundexperts based on this comment.

joekaufmanexpensify · 2023-11-27T15:02:30Z

Great! Closing as this is all set.

kbecciv 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 Sep 13, 2023

melvin-bot bot changed the title ~~[Distance] - Ability to request money without filling start and finish points~~ [$500] [Distance] - Ability to request money without filling start and finish points Sep 13, 2023

melvin-bot bot assigned joekaufmanexpensify Sep 13, 2023

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

melvin-bot bot assigned tjferriss Sep 13, 2023

melvin-bot bot assigned allroundexperts Sep 13, 2023

joekaufmanexpensify unassigned tjferriss Sep 14, 2023

melvin-bot bot assigned roryabraham Sep 19, 2023

roryabraham changed the title ~~[$500] [Distance] - Ability to request money without filling start and finish points~~ [$100] [Distance] - Ability to request money without filling start and finish points Sep 20, 2023

This comment was marked as outdated.

Sign in to view

roryabraham changed the title ~~[$100] [Distance] - Ability to request money without filling start and finish points~~ [$500] [Distance] - Ability to request money without filling start and finish points Sep 20, 2023

This comment was marked as outdated.

Sign in to view

roryabraham assigned DylanDylann Sep 20, 2023

joekaufmanexpensify added Weekly KSv2 and removed Daily KSv2 labels Oct 9, 2023

allroundexperts mentioned this issue Nov 20, 2023

Modify MoneyRequestConfirmPage for Distance Request #25707

Merged

57 tasks

joekaufmanexpensify closed this as completed Nov 27, 2023

[HOLD for payment 2023-10-09] [$500] [Distance] - Ability to request money without filling start and finish points #27383

[HOLD for payment 2023-10-09] [$500] [Distance] - Ability to request money without filling start and finish points #27383

Comments

kbecciv commented Sep 13, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

dukenv0307 commented Sep 13, 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)

melvin-bot bot commented Sep 13, 2023

melvin-bot bot commented Sep 13, 2023

melvin-bot bot commented Sep 13, 2023 • edited by joekaufmanexpensify Loading

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Sep 13, 2023

melvin-bot bot commented Sep 13, 2023

DylanDylann commented Sep 13, 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)

joekaufmanexpensify commented Sep 14, 2023

joekaufmanexpensify commented Sep 14, 2023

joekaufmanexpensify commented Sep 17, 2023

joekaufmanexpensify commented Sep 18, 2023

joekaufmanexpensify commented Sep 19, 2023

allroundexperts commented Sep 19, 2023 • edited Loading

melvin-bot bot commented Sep 19, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

joekaufmanexpensify commented Oct 9, 2023

joekaufmanexpensify commented Oct 16, 2023

joekaufmanexpensify commented Oct 23, 2023

joekaufmanexpensify commented Oct 30, 2023

joekaufmanexpensify commented Nov 6, 2023

melvin-bot bot commented Nov 7, 2023

joekaufmanexpensify commented Nov 13, 2023

allroundexperts commented Nov 13, 2023

DylanDylann commented Nov 14, 2023

allroundexperts commented Nov 14, 2023

DylanDylann commented Nov 14, 2023

allroundexperts commented Nov 14, 2023

joekaufmanexpensify commented Nov 14, 2023

allroundexperts commented Nov 14, 2023

joekaufmanexpensify commented Nov 14, 2023

joekaufmanexpensify commented Nov 20, 2023

allroundexperts commented Nov 20, 2023

Checklist

Regression Test

joekaufmanexpensify commented Nov 21, 2023

joekaufmanexpensify commented Nov 21, 2023

joekaufmanexpensify commented Nov 21, 2023

joekaufmanexpensify commented Nov 21, 2023

joekaufmanexpensify commented Nov 21, 2023

joekaufmanexpensify commented Nov 21, 2023

joekaufmanexpensify commented Nov 21, 2023

JmillsExpensify commented Nov 22, 2023

joekaufmanexpensify commented Nov 27, 2023

kbecciv commented Sep 13, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Sep 13, 2023 •

edited by joekaufmanexpensify

Loading

allroundexperts commented Sep 19, 2023 •

edited

Loading