-
Notifications
You must be signed in to change notification settings - Fork 3k
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-07-26] [$250] [Hold] Distance rates- Deleted distance rate is not disabled when submitting distance expense offline #44154
Comments
Triggered auto assignment to @OfstadC ( |
@OfstadC 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 can still select pending-delete distance rate and will result in an error when submitted. What is the root cause of that problem?We are not filtering any distance rate that is pending-delete when showing the rate list. App/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx Lines 62 to 72 in 68374a5
What changes do you think we should make in order to solve the problem?Filter out any pending-delete rate. To do that:
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Distance rates- Deleted distance rate is not disabled when submitting distance expense offline What is the root cause of that problem?We don't have any check for disabled rates on App/src/pages/iou/request/step/IOURequestStepDistanceRate.tsx Lines 62 to 72 in 68374a5
What changes do you think we should make in order to solve the problem?We need to first return mileageRates[rateID] = {
rate: rate.rate,
currency: rate.currency,
unit: distanceUnit.attributes.unit,
name: rate.name,
customUnitRateID: rate.customUnitRateID,
pendingAction: rate.pendingAction,
}; Then add return {
text: rate.name ?? rateForDisplay,
alternateText: rate.name ? rateForDisplay : '',
keyForList: rate.customUnitRateID,
value: rate.customUnitRateID,
isSelected: lastSelectedRateID ? lastSelectedRateID === rate.customUnitRateID : !!(rate.name === CONST.CUSTOM_UNITS.DEFAULT_RATE),
isDisabled: rate.pendingAction === 'delete',
pendingAction: rate.pendingAction,
}; What alternative solutions did you explore? (Optional)If we also need to grey out the option when updated offline, we nee to also return App/src/pages/workspace/distanceRates/PolicyDistanceRatesPage.tsx Lines 113 to 118 in 68374a5
const sections = Object.values(rates).map((rate) => {
const rateForDisplay = DistanceRequestUtils.getRateForDisplay(rate.unit, rate.rate, rate.currency, translate, toLocaleDigit);
return {
text: rate.name ?? rateForDisplay,
alternateText: rate.name ? rateForDisplay : '',
keyForList: rate.customUnitRateID,
value: rate.customUnitRateID,
isSelected: lastSelectedRateID ? lastSelectedRateID === rate.customUnitRateID : !!(rate.name === CONST.CUSTOM_UNITS.DEFAULT_RATE),
isDisabled: rate.pendingAction === 'delete',
pendingAction:
rate.pendingAction ??
rate.pendingFields?.rate ??
rate.pendingFields?.enabled ??
rate.pendingFields?.currency ??
(policy?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD ? policy?.pendingAction : undefined),
};
}); |
Proposal Updated
|
Proposal Updated
|
Proposal Updated
|
Following this Slack thread before proceeding |
@mallenexpensify I wonder if the Offline Distance request issues should be consolidated since they might have the same/similar solution? 🤔 |
Asking about consolidating, should have an update tomorrow 🤞 |
Thanks Matt! |
Any update @mallenexpensify ? 😃 |
@paultsimura can you please comment so I can assign to you? |
👋🏼 |
@mallenexpensify Is there any action I need to take here? |
No @OfstadC , @paultsimura is going to to plan to holistically address offline distance bugs. He'll comment here with an update once he's reviewed all issues (and either raise a PR or recommend making the issue external) |
@OfstadC Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@mallenexpensify I have a quite simple fix for this – if I put up a PR, who's going to be the reviewer? |
📣 @paultsimura 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.9-5 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-07-26. 🎊 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:
|
@shubham1206agra I think you may need to accept the offer in Upwork? It's not showing an option to Approve payment |
Payment Summary:
|
@OfstadC Offer accepted |
@shubham1206agra paid via Upwork, payment confirmation updated above. @shubham1206agra plz complete the BZ checklist above. |
@arosiclair, @OfstadC, @paultsimura, @shubham1206agra Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@shubham1206agra I think you still need to 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:
|
@shubham1206agra do we want a test case for this? If so, please provide the steps. If not, please provide reasoning why not. Thx |
@mallenexpensify We don't need any test steps here as this was an edge case that we missed during initial implementation, and the bug should not happen again in future. |
@arosiclair, @OfstadC, @paultsimura, @shubham1206agra Huh... This is 4 days overdue. Who can take care of this? |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.0-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: https://expensify.testrail.io/index.php?/tests/view/4652159
Issue reported by: Applause - Internal Team
Action Performed:
Precondition:
Expected Result:
In Step 6, app should prevent the deleted distance rate from being selected when submitting a distance distance
Actual Result:
In Step 6, user can submit a distance expense with the deleted distance rate, which results in error when returning online
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6520132_1718947996614.20240621_132706.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @OfstadCThe text was updated successfully, but these errors were encountered: