Show rate not valid error after deleting per diem rate#84547
Draft
Show rate not valid error after deleting per diem rate#84547
Conversation
When a per diem rate is deleted, the CUSTOM_UNIT_OUT_OF_POLICY violation was not being added to affected transactions (unlike distance rate deletion which does this). This caused the "Rate not valid" error banner to not appear when viewing the expense, and allowed duplication to proceed with an invalid rate resulting in an unexpected error. Two fixes: 1. Update deleteWorkspacePerDiemRates to optimistically add CUSTOM_UNIT_OUT_OF_POLICY violations to affected transactions, mirroring the pattern in deletePolicyDistanceRates. 2. Add a per diem fallback to the isCustomUnitOutOfPolicy check in MoneyRequestView.tsx that directly validates the rate still exists in the policy, similar to the existing distance request fallback. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
7 tasks
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Hi! I'd like to work on this issue. Can I get assigned? 🙏 |
Contributor
|
@MelvinBot pull main and fix conflict |
Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Contributor
Author
|
Merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
When a per diem rate is deleted from a workspace, two issues occur:
Root cause: Unlike
deletePolicyDistanceRateswhich optimistically addsCUSTOM_UNIT_OUT_OF_POLICYviolations to affected transactions when a distance rate is deleted,deleteWorkspacePerDiemRatesonly updates the policy custom unit and never touches transaction violations. Additionally,MoneyRequestView.tsxhas a fallback check(isDistanceRequest && !rate)for distance requests but no equivalent for per diem requests.This PR makes two fixes:
Primary fix (PerDiem.ts): Updates
deleteWorkspacePerDiemRatesto accepttransactionIDsAffectedandtransactionViolationsparameters and optimistically addCUSTOM_UNIT_OUT_OF_POLICYviolations to affected transactions — mirroring the existing pattern indeletePolicyDistanceRates. The workspace per diem pages (WorkspacePerDiemPage.tsxandWorkspacePerDiemDetailsPage.tsx) are updated with transaction/violation selectors to find and pass affected transactions, following the same pattern as the distance rate pages.Secondary fix (MoneyRequestView.tsx): Adds a per diem fallback to the
isCustomUnitOutOfPolicycheck that directly validates the rate still exists in the policy viagetPerDiemRateCustomUnitRate, similar to the existing distance request fallback. This provides a safety net even if the violation is missing for any reason.Fixed Issues
$ #82584
PROPOSAL: #82584 (comment)
Tests
hasCustomUnitOutOfPolicyViolationguard prevents it)Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari