-
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
[Awaiting checklist] [$250] Distance Request -"Distance" option does not appear in QAB for a gmail account #40790
Comments
Triggered auto assignment to @twisterdotcom ( |
@twisterdotcom FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
We think that this bug might be related to #wave-collect - Release 1 |
ProposalPlease re-state the problem that we are trying to solve in this issue.We show P2P details for submit expense distance even when the user is not under beta What is the root cause of that problem?In chat options: App/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js Line 109 in c90b46e
We currently only check if the action is not App/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js Line 136 in c90b46e
App/src/libs/OptionsListUtils.ts Line 1995 in f421923
What changes do you think we should make in order to solve the problem?We have extra checks in place for App/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js Line 121 in c90b46e
App/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js Line 128 in c90b46e
So we should follow the same pattern and update the --- a/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js
+++ b/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js
@@ -124,26 +122,14 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF
undefined,
undefined,
undefined,
- !isCategorizeOrShareAction,
+ !isCategorizeOrShareAction && (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE),
isCategorizeOrShareAction ? 0 : undefined,
); Result: |
@neil-marcellini keen for your take on this one, so going to make you the internal reviewer ahead of time. I agree though, it's odd to allow a QAB but not direct to it. 40790.mp4 |
Job added to Upwork: https://www.upwork.com/jobs/~013f4eac573fdf1cb2 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.The user is able to do P2P distance request even though they're not in What is the root cause of that problem?The problem here is not that "Distance" does not show, it's expected that it will not show if the user is not in The issue is the user's able to do P2P distance request in the first place, even though they're not in
The root cause of this is the user can still see the What changes do you think we should make in order to solve the problem?Update this to
So it will not show any option in If there's any other section of What alternative solutions did you explore? (Optional)NA |
ProposalPlease re-state the problem that we are trying to solve in this issue.The quick action bar shows the "Record distance" option when a request for What is the root cause of that problem?When buildOnyxDataForMoneyRequest we doesn't pass isOneOnOneChat into What changes do you think we should make in order to solve the problem?We will check condition isOneOnOneChat from report then pass into buildOnyxDataForMoneyRequest + const isOneOnOneChat = ReportUtils.isOneOnOneChat(chatReport);
// STEP 5: Build Onyx Data
const [optimisticData, successData, failureData] = buildOnyxDataForMoneyRequest(
chatReport,
iouReport,
optimisticTransaction,
optimisticCreatedActionForChat,
optimisticCreatedActionForIOUReport,
iouAction,
optimisticPersonalDetailListAction,
reportPreviewAction,
optimisticPolicyRecentlyUsedCategories,
optimisticPolicyRecentlyUsedTags,
isNewChatReport,
optimisticTransactionThread ?? {},
optimisticCreatedActionForTransactionThread ?? {},
shouldCreateNewMoneyRequestReport,
policy,
policyTagList,
policyCategories,
optimisticNextStep,
+ isOneOnOneChat,
); Optional: for out of date data we can hide the quick action menu item at this line when What alternative solutions did you explore? (Optional) |
Reviewing proposals. |
I'll take a peek in a bit |
Here's what's happening.
The solution should be to fix the bug in point 3. I will see if I can find the issue for that. I can't find any other issue like that. Please lmk if you find something, but otherwise let's fix it here. Screen.Recording.2024-04-29.at.10.14.26.AM.mov |
@neil-marcellini I agree with this, my proposal is doing just that. |
Alright, shall we assign @nkdengineer? |
Updated ProposalUpdated proposal to match new expectations as per comments c,c, @getusha @neil-marcellini |
Okay, this is with you then @getusha. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@GandalfGwaihir's proposal looks good to me. |
Current assignee @neil-marcellini is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 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-06-18. 🎊 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:
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-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-06-20. 🎊 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:
|
Payment Summary:
|
Bump on this checklist @getusha. |
@twisterdotcom, @neil-marcellini, @getusha, @nkdengineer Huh... This is 4 days overdue. Who can take care of this? |
Bump @getusha. |
@twisterdotcom, @neil-marcellini, @getusha, @nkdengineer Huh... This is 4 days overdue. Who can take care of this? |
This fell through the cracks, will complete the checklist. |
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: [@getusha] The PR that introduced the bug has been identified. Link to the PR: #40748 Regression Test Proposal
Do we agree 👍 or 👎 |
@twisterdotcom accepted the offer, could you end it? ty! |
Done |
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.4.64-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:
Issue found when executing PR #39413
Action Performed:
Expected Result:
User should be navigated to the Distance tab
Actual Result:
No Distance tab / user is directed to the Manual request tab
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6458086_1713825666275.Screen_Recording_2024-04-23_at_1.13.14_AM.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @getushaThe text was updated successfully, but these errors were encountered: