-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-01-17] [$500] Distance - Able to request money by distance by adding a stop same as finish point without enterring start point From #29895
Comments
Triggered auto assignment to @strepanier03 ( |
Job added to Upwork: https://www.upwork.com/jobs/~01f177f4854fa6146e |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Next Button Activated with Same Stop and Finish Point in Request Money What is the root cause of that problem?In App/src/components/DistanceRequest/index.js Lines 183 to 191 in 6cd9c84
Now to generate validatedWaypoints the code is in file Src/libs/TransactionUtils.js App/src/libs/TransactionUtils.ts Lines 416 to 445 in 6cd9c84
This function fails in a edge case where starting waypoints are invalid because here lastWaypointIndex is equal to -1 in starting which suggests that first waypoint will always be valid when waypoints size > 1 . What changes do you think we should make in order to solve the problem?We have to have increment lastWaypointIndex till we find some valid waypoint and then it is good to go . Here is my solution
Here I have introduced a new variable named as foundFirstValidWaypoint which is false in starting and once we find a validwaypoint we will set it to true till the time we will increment lastWaypointIndex in each iteration . What alternative solutions did you explore? (Optional)We can add a check that the start point should always be there. App/src/components/DistanceRequest/index.js Line 185 in 7601f89
leading to above adding the same condition below App/src/components/DistanceRequest/index.js Lines 158 to 160 in 7601f89
Also here the same condition App/src/components/DistanceRequest/index.js Lines 225 to 227 in 7601f89
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane ( |
Linked PR #27045 |
Doubled checked and both staging and prod show the error. |
@strepanier03 I think you misread the steps. You can follow one of the videos for information. |
I think this one should be closed in favor of #28477 |
This comment was marked as outdated.
This comment was marked as outdated.
@DylanDylann I don't think so as in the above case validatedWaypoints size will be 2 |
This comment was marked as off-topic.
This comment was marked as off-topic.
@strepanier03, @rushatgabhane Huh... This is 4 days overdue. Who can take care of this? |
After retesting I see where I messed up the steps and was able to create the expense. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@rushatgabhane friendly bump on the proposal here when you get a chance. |
This issue has not been updated in over 15 days. @rojiphil, @danieldoglas, @strepanier03, @rushatgabhane eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
This was merged today, should be deployed to staging soon. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.23-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-01-17. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
|
@strepanier03 could you please add payment summary. Thank you! 🙇 |
Yup, will do, today is the release day so that is the plan. |
Payment summary
|
@strepanier03 Accepted the offer |
@rakshitjain13 - After reviewing this more carefully, I'm not sure why the payment comment had your name listed. I don't see that you worked on the PR or the review for this bug. I am going to need to cancel the contract and we won't be able to pay it. If you can link me to the work you did for this bug, I can reconsider but at this point, I don't think there's any work related to this that you need to be paid for. Is that correct? |
@rakshitjain13 is a bug reporter 😄 |
Got it thank you @0xmiroslav , looks like it got in right before we paused the program so I'll get it sorted. |
@rakshitjain13 sorry about the confusion there, that's my bad. I have sent over a new offer for the right price as the original one I sent was for $500 and it should be $50. |
@strepanier03 No worry! Thank You . I thought Expensify is giving New Year gifts 🤫 |
All that's left is the manual request payment for @rushatgabhane Payment summary is here. Closing and assigning Jason. |
$500 payment approved for @rushatgabhane based on this summary. |
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: 1.3.86.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: @rakshitjain13
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697639775327799
Action Performed:
Action Performed:
Expected Result:
It should not create request with not valid waypoints
Actual Result:
Able to create distance request with duplicate waypoints
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Android: Native
Screen.Recording.2023-10-16.at.7.29.43.PM.mov
Android: mWeb Chrome
Screen.Recording.2023-10-17.at.1.24.34.AM.mov
iOS: Native
Screen.Recording.2023-10-16.at.7.21.36.PM.mov
iOS: mWeb Safari
Screen.Recording.2023-10-16.at.7.21.36.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2023-10-16.at.7.36.52.PM.mov
Screen.Recording.2023-10-16.at.7.41.28.PM.mov
MacOS: Desktop
Screen.Recording.2023-10-16.at.7.33.40.PM.mov
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: