Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] [distance] Distance - Stop waypoint is filled with Finish address #26725

Closed
6 tasks done
lanitochka17 opened this issue Sep 4, 2023 · 16 comments
Closed
6 tasks done
Assignees

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 4, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue found when executing PR #26591

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to + > Request money > Distance
  3. Add addresses in start and finish waypoint
  4. Click Add stop
  5. Click on the Stop field and clear it and save
  6. Click Add stop again
  7. Click Finish and add an address and save it

Expected Result:

The Stop field is not populated

Actual Result:

The address selected for Finish waypoint is now also populated in Stop waypoint

Workaround:

Unknown

Platforms:

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

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

Version Number: 1.3.63-0

Reproducible in staging?: Yes

Reproducible in production?: No

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

Bug6188431_20230905_024928.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0126bac11a54e11305
  • Upwork Job ID: 1698813869209538560
  • Last Price Increase: 2023-09-04
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Sep 4, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Sep 4, 2023

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

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

@neil-marcellini
Copy link
Contributor

Looks like a legit bug

@bondydaa bondydaa added the External Added to denote the issue can be worked on by a contributor label Sep 4, 2023
@melvin-bot melvin-bot bot changed the title [distance] Distance - Stop waypoint is filled with Finish address [$500] [distance] Distance - Stop waypoint is filled with Finish address Sep 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0126bac11a54e11305

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

melvin-bot bot commented Sep 4, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

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

@Pujan92
Copy link
Contributor

Pujan92 commented Sep 4, 2023

@neil-marcellini it seems a backend issue https://expensify.slack.com/archives/C049HHMV9SM/p1693855330395669

@neil-marcellini
Copy link
Contributor

Ah right nice, we just merged a fix for that, it will be deployed soon so this is not a blocker.

@neil-marcellini neil-marcellini self-assigned this Sep 4, 2023
@neil-marcellini neil-marcellini removed the DeployBlockerCash This issue or pull request should block deployment label Sep 4, 2023
@bondydaa
Copy link
Contributor

bondydaa commented Sep 4, 2023

ah thanks @Pujan92!

@neil-marcellini
Copy link
Contributor

Here's the merged backend PR that fixes this, but we need to CP it.

@neil-marcellini neil-marcellini added Daily KSv2 and removed Hourly KSv2 labels Sep 4, 2023
@bondydaa
Copy link
Contributor

bondydaa commented Sep 4, 2023

@lanitochka17 can you retest against staging API, we CP'd a PR that should have fixed this.

@bondydaa bondydaa removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Sep 4, 2023
@napster125
Copy link
Contributor

I can't reproduce it now.

@DylanDylann
Copy link
Contributor

@bondydaa I still can reproduce on staging now

@DylanDylann
Copy link
Contributor

DylanDylann commented Sep 5, 2023

@neil-marcellini @bondydaa This is another bug and can be fixed from FE

Proposal

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

Distance - Stop waypoint is filled with Finish address

What is the root cause of that problem?

Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {
comment: {
waypoints: {
[`waypoint${newLastIndex}`]: {},
},
},

Currently, in the beginning, waypoint1 will be the start point, waypoint2 will be the finish point. When creating a new stop point, start and finish point will remain. The stop point will be waypoint3.

if (index === 0) {
descriptionKey += 'start';
waypointIcon = Expensicons.DotIndicatorUnfilled;
} else if (index === lastWaypointIndex) {
descriptionKey += 'finish';
waypointIcon = Expensicons.Location;
} else {
descriptionKey += 'stop';
waypointIcon = Expensicons.DotIndicator;
}

But while displaying, the finish point is the last point (in this context, last point is waypoint3). It caused this bug

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

When adding a new stop point we should change the index of the finish point to the last point. In this context, when adding new stop point, this stop point will be waypoint2 and the finish point will move to waypoint3.

Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {
comment: {
waypoints: {
[`waypoint${newLastIndex}`]: {},
},
},

[`waypoint${newLastIndex-1}`]: null,
[`waypoint${newLastIndex}`]: lodashGet(existingWaypoints, `waypoint${newLastIndex-1}`, {})

Result

Screen.Recording.2023-09-05.at.11.58.44.mp4

What alternative solutions did you explore? (Optional)

In case, we want to remain the finish point always be waypoint 2.

if (index === 0) {
descriptionKey += 'start';
waypointIcon = Expensicons.DotIndicatorUnfilled;
} else if (index === lastWaypointIndex) {
descriptionKey += 'finish';
waypointIcon = Expensicons.Location;
} else {
descriptionKey += 'stop';
waypointIcon = Expensicons.DotIndicator;
}

we can update here to display the finish point when index is 1

With this approach, the stop point will display below the finish point
Screenshot 2023-09-05 at 12 04 20

@lanitochka17
Copy link
Author

The issue does not reproduce in build 1.3.64-0.

Recording.46.mp4

@bondydaa
Copy link
Contributor

bondydaa commented Sep 5, 2023

okay thanks for confirming @lanitochka17 .

@DylanDylann please follow the process https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#raising-jobs-and-bugs for reporting new bugs instead of adding it to an existing different bug.

I'm going to close this out.

@bondydaa bondydaa closed this as completed Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants