Repository navigation
Allow $0 distance rates and $0 distance expenses, including splits - #102914
NicolasBonet wants to merge 26 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…stance-rate # Conflicts: # src/components/SelectionList/ListItem/SplitListItem.tsx # src/libs/PolicyDistanceRatesUtils.ts
|
@thesahindia Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6926c27195
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… Split when the rate is on another workspace
…stance-rate # Conflicts: # src/pages/iou/DynamicSplitExpensePage.tsx
|
@thesahindia we can do the review while auth is deployed, the idea is to speed up the reviewing process so we can merge as soon as Auth is in prod 🙏 |
BUG#4:1: Create a workspace distance rate of €0.00/mile. Expected Result: Actual Result: RF-4.mov |
Bug#5:Test steps: 1: Create a workspace distance rate of $1.00/mile. Expected result: The original 10 miles should not be duplicated across the two splits. Aactual result: The two split amounts may be $5 + $5, while both retain the original 10-mile distance. RF-5.mov |
|
BUG#6: When splitting an expense, clicking the RF-6.mp4 |
|
BUG#7: When splitting a zero-amount expense, changing the rate updates the amount, but the RF-7.mp4 |
|
BUG#8: In Workspace A, configure a positive mileage rate and set the workspace as the default. Then, create a track distance expense. Next, change the default workspace to Workspace B and open the distance expense created in Self-DM. If the calculated amount rounds to zero despite the positive mileage rate, the Split option is incorrectly displayed. Clicking Split navigates to the “Hmm... it's not here” page instead of opening the split expense flow. RF-8.mp4 |
BUG#9:Preconditions:
Steps:
Expected: The app should handle the historical $10 amount consistently. The action should not silently convert both $5 shares to $0 merely because the workspace's rate changed. Actual: Both split amounts become $0.00. Video_2026-10-16_15-46-37.mp4 |
- Split a nonzero expense whose rate is now $0 with its historical rate - Give a $0 split of a positive rate expense no distance, and only accept $0 splits on a $0 rate - Put the remainder of a $0 rate distance on the first split, like Make splits even - Lock a split's amount only when the split itself is on a $0 rate - Look up the distance rate across workspaces before offering Split
|
@marufsharifi thanks for the thorough testing! Fixed in 743e1a9:
Could you re-check, please? |
|
@marufsharifi what's your ETA on the re-review? |
@trjExpensify, I am working on this, i will update here in a few hours. thanks. |
BUG#1:Steps to Reproduce:
Expected Result: Actual Result: RF-1.mov |
BUG#2:Preconditions: Desktop Chrome, online. Use your own unreported expense and a workspace rate of $0.01 per mile. Steps:
Expected Result: Split should be unavailable for this rounded-to-zero positive-rate expense, matching the current correction’s stated behavior. Actual Behavior: Search selection offers Split, but choosing it opens RF-2.mp4 |
BUG#3:Test Steps:
Expected Behavior: The initial equal split should contain $10.00 and 10 miles per row, consistently preserving the current expense. Actual Behavior: Both rows show $10.00 and 20 miles, often with a calculated $0.50/mile merchant description. The mileage has doubled during initialization. RF-3.mp4 |
|
Confirmation#1:
The error message says cc @flaviadefaria could you please confirm this. thanks. CF-1.mov |
This error message has always referred to the distance amount, even when the rate wasn’t $0. But I agree that it’s particularly unclear in this scenario. I think clarifying the message would be helpful regardless of the rate, so let’s update it across all scenarios, not just when the rate is $0. We can go with your suggestion:
|
BUG#4Steps: 1: Create two distance rates, QA Original and QA Other, both at $1.00/mile. Expected Behavior: The rate should change to Actual Behavior: The rate remains outdated when splitting the expense. Rf-4.mov |
…ility, edited amounts, distance error copy
|
Thanks @marufsharifi! Fixed in bf8855f:
Could you re-check, please? |
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index e3215535..37c2e4b4 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -1392,8 +1392,8 @@ const translations: TranslationDeepObject<typeof en> = {
totalAmountGreaterThanOriginal: (amount: string) => `Der Gesamtbetrag ist um ${amount} höher als die ursprüngliche Ausgabe.`,
totalAmountLessThanOriginal: (amount: string) => `Der Gesamtbetrag ist um ${amount} geringer als die ursprüngliche Ausgabe.`,
splitExpenseZeroAmount: 'Bitte gib einen gültigen Betrag ein, bevor du fortfährst.',
- splitExpenseZeroDistance: 'Bitte gib eine gültige Entfernung ein, bevor du fortfährst.',
- splitExpenseZeroTotalAmountNotEditable: (amount: string) => `Der Betrag kann nicht bearbeitet werden, wenn die ursprüngliche Summe ${amount} beträgt.`,
+ splitExpenseZeroDistance: 'Bitte geben Sie eine gültige Entfernung ein, bevor Sie fortfahren.',
+ splitExpenseZeroTotalAmountNotEditable: (amount: string) => `Der Betrag kann nicht bearbeitet werden, wenn die ursprüngliche Summe ${amount} ist.`,
splitExpenseOneMoreSplit: 'Keine Aufteilungen hinzugefügt. Füge mindestens eine hinzu, um zu speichern.',
splitExpenseEditTitle: (amount: string, merchant: string) => `${amount} für ${merchant} bearbeiten`,
removeSplit: 'Aufteilung entfernen',
diff --git a/src/languages/el.ts b/src/languages/el.ts
index 8923c26e..c18175b7 100644
--- a/src/languages/el.ts
+++ b/src/languages/el.ts
@@ -244,7 +244,7 @@ const translations: TranslationDeepObject<typeof en> = {
pleaseCompleteForm: 'Συμπληρώστε τη φόρμα παραπάνω για να συνεχίσετε',
pleaseSelectOne: 'Παρακαλώ επιλέξτε μια από τις παραπάνω επιλογές',
invalidRateError: 'Παρακαλούμε εισαγάγετε έγκυρο συντελεστή',
- lowRateError: 'Το ποσοστό δεν μπορεί να είναι αρνητικό',
+ lowRateError: 'Το επιτόκιο δεν μπορεί να είναι αρνητικό',
email: 'Παρακαλούμε εισαγάγετε μια έγκυρη διεύθυνση email',
login: 'Προέκυψε σφάλμα κατά τη σύνδεση. Παρακαλούμε δοκιμάστε ξανά.',
},
@@ -1439,8 +1439,8 @@ const translations: TranslationDeepObject<typeof en> = {
totalAmountGreaterThanOriginal: (amount: string) => `Το συνολικό ποσό είναι κατά ${amount} μεγαλύτερο από την αρχική δαπάνη.`,
totalAmountLessThanOriginal: (amount: string) => `Το συνολικό ποσό είναι κατά ${amount} λιγότερο από την αρχική δαπάνη.`,
splitExpenseZeroAmount: 'Παρακαλώ εισαγάγετε ένα έγκυρο ποσό πριν συνεχίσετε.',
- splitExpenseZeroDistance: 'Παρακαλώ εισαγάγετε μια έγκυρη απόσταση πριν συνεχίσετε.',
- splitExpenseZeroTotalAmountNotEditable: (amount: string) => `Το ποσό δεν μπορεί να επεξεργαστεί όταν το αρχικό σύνολο είναι ${amount}.`,
+ splitExpenseZeroDistance: 'Παρακαλούμε εισαγάγετε μια έγκυρη απόσταση πριν συνεχίσετε.',
+ splitExpenseZeroTotalAmountNotEditable: (amount: string) => `Δεν είναι δυνατή η επεξεργασία του ποσού όταν το αρχικό σύνολο είναι ${amount}.`,
splitExpenseOneMoreSplit: 'Δεν προστέθηκαν κατανομές. Προσθέστε τουλάχιστον μία για να αποθηκεύσετε.',
splitExpenseEditTitle: (amount: string, merchant: string) => `Επεξεργαστείτε ${amount} για ${merchant}`,
removeSplit: 'Αφαίρεση διαχωρισμού',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index c71121ab..6220c0bf 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -231,7 +231,7 @@ const translations: TranslationDeepObject<typeof en> = {
pleaseCompleteForm: 'Por favor complete el formulario de arriba para continuar',
pleaseSelectOne: 'Seleccione una de las opciones',
invalidRateError: 'Por favor, introduce una tarifa válida',
- lowRateError: 'La tarifa no puede ser negativa',
+ lowRateError: 'La tasa no puede ser negativa',
email: 'Por favor, introduzca una dirección de correo electrónico válida',
login: 'Se produjo un error al iniciar sesión. Por favor intente nuevamente.',
},
@@ -1383,7 +1383,7 @@ const translations: TranslationDeepObject<typeof en> = {
totalAmountLessThanOriginal: (amount) => `El importe total es ${amount} menor que el gasto original.`,
splitExpenseZeroAmount: 'Por favor, introduce un importe válido antes de continuar.',
splitExpenseZeroDistance: 'Por favor, introduce una distancia válida antes de continuar.',
- splitExpenseZeroTotalAmountNotEditable: (amount) => `El importe no se puede editar cuando el total original es ${amount}.`,
+ splitExpenseZeroTotalAmountNotEditable: (amount: string) => `No se puede editar el importe cuando el total original es ${amount}.`,
splitExpenseOneMoreSplit: 'No se han añadido divisiones. Añade al menos una para guardar.',
splitExpenseEditTitle: (amount, merchant) => `Editar ${amount} para ${merchant}`,
removeSplit: 'Eliminar división',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 1b16cd2c..37caf28c 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -239,7 +239,7 @@ const translations: TranslationDeepObject<typeof en> = {
pleaseCompleteForm: '続行するには上のフォームに入力してください',
pleaseSelectOne: '上からオプションを選択してください',
invalidRateError: '有効なレートを入力してください',
- lowRateError: 'レートを負の値にすることはできません',
+ lowRateError: 'レートをマイナスにすることはできません',
email: '有効なメールアドレスを入力してください',
login: 'ログイン中にエラーが発生しました。もう一度お試しください。',
},
@@ -1376,7 +1376,7 @@ const translations: TranslationDeepObject<typeof en> = {
totalAmountLessThanOriginal: (amount: string) => `合計金額は元の経費より${amount}少なくなっています。`,
splitExpenseZeroAmount: '続行する前に有効な金額を入力してください。',
splitExpenseZeroDistance: '続行する前に有効な距離を入力してください。',
- splitExpenseZeroTotalAmountNotEditable: (amount: string) => `元の合計が${amount}の場合、金額は編集できません。`,
+ splitExpenseZeroTotalAmountNotEditable: (amount: string) => `元の合計金額が ${amount} の場合、金額は編集できません。`,
splitExpenseOneMoreSplit: '分割が追加されていません。保存するには少なくとも 1 件追加してください。',
splitExpenseEditTitle: (amount: string, merchant: string) => `${merchant} の ${amount} を編集`,
removeSplit: '分割を削除',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 565b25de..e4d4b66e 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -1391,7 +1391,7 @@ const translations: TranslationDeepObject<typeof en> = {
totalAmountLessThanOriginal: (amount: string) => `Het totale bedrag is ${amount} minder dan de oorspronkelijke uitgave.`,
splitExpenseZeroAmount: 'Voer een geldig bedrag in voordat je verdergaat.',
splitExpenseZeroDistance: 'Voer een geldige afstand in voordat je verdergaat.',
- splitExpenseZeroTotalAmountNotEditable: (amount: string) => `Het bedrag kan niet worden bewerkt als het oorspronkelijke totaal ${amount} is.`,
+ splitExpenseZeroTotalAmountNotEditable: (amount: string) => `Bedrag kan niet worden bewerkt wanneer het oorspronkelijke totaal ${amount} is.`,
splitExpenseOneMoreSplit: 'Geen splitsingen toegevoegd. Voeg er minimaal één toe om op te slaan.',
splitExpenseEditTitle: (amount: string, merchant: string) => `${amount} voor ${merchant} bewerken`,
removeSplit: 'Splits verwijderen',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 1f608775..e6cf56a2 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -1424,7 +1424,7 @@ const translations: TranslationDeepObject<typeof en> = {
totalAmountGreaterThanOriginal: (amount: string) => `Kwota łączna jest o ${amount} wyższa niż pierwotny wydatek.`,
totalAmountLessThanOriginal: (amount: string) => `Kwota całkowita jest o ${amount} mniejsza niż pierwotny wydatek.`,
splitExpenseZeroAmount: 'Wprowadź prawidłową kwotę przed kontynuowaniem.',
- splitExpenseZeroDistance: 'Wprowadź prawidłową odległość przed kontynuowaniem.',
+ splitExpenseZeroDistance: 'Wpisz prawidłową odległość, zanim przejdziesz dalej.',
splitExpenseZeroTotalAmountNotEditable: (amount: string) => `Kwoty nie można edytować, gdy pierwotna suma wynosi ${amount}.`,
splitExpenseOneMoreSplit: 'Nie dodano podziałów. Dodaj co najmniej jeden, aby zapisać.',
splitExpenseEditTitle: (amount: string, merchant: string) => `Edytuj ${amount} dla ${merchant}`,
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 179ea368..06b8bdc1 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -1337,8 +1337,8 @@ const translations: TranslationDeepObject<typeof en> = {
totalAmountGreaterThanOriginal: (amount: string) => `总金额比原始报销多出 ${amount}。`,
totalAmountLessThanOriginal: (amount: string) => `总金额比原始报销少 ${amount}。`,
splitExpenseZeroAmount: '请在继续之前输入有效金额。',
- splitExpenseZeroDistance: '请在继续之前输入有效距离。',
- splitExpenseZeroTotalAmountNotEditable: (amount: string) => `原始总额为 ${amount} 时,无法编辑金额。`,
+ splitExpenseZeroDistance: '请先输入有效的距离,然后再继续。',
+ splitExpenseZeroTotalAmountNotEditable: (amount: string) => `当原始总额为 ${amount} 时,无法编辑金额。`,
splitExpenseOneMoreSplit: '尚未添加拆分。请至少添加一项以保存。',
splitExpenseEditTitle: (amount: string, merchant: string) => `编辑 ${merchant} 的 ${amount}`,
removeSplit: '移除拆分',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
@marufsharifi how you looking on the re-review? |
BUG#1:Steps
Expected Behavior Actual Behavior RF-1.mov |
BUG#2Steps
Expected Behavior Actual Behavior RF-2.mov |
BUG#3Steps
Expected Behavior Actual Behavior RF-3.mp4 |
marufsharifi
left a comment
There was a problem hiding this comment.
Just two minor suggestions.
| if (isDistanceRequest && splitExpense.customUnit) { | ||
| const {unit, rate} = resolveSplitItemRate({customUnit: splitExpense.customUnit, fallbackMileageRate: mileageRate, policy, policies}); | ||
| if (rate && rate > 0) { | ||
| if (rate === 0) { |
There was a problem hiding this comment.
No automated test covers this "split by date" $0-rate branch.
| if (isEditingSplit && transaction) { | ||
| // A split keeps the readings of the whole trip while its distance is only its share of it, so readings | ||
| // saved unchanged must not replace the split's distance with the whole trip's distance | ||
| const hasReadingChanges = splitDraftTransaction?.comment?.odometerStart !== start || splitDraftTransaction?.comment?.odometerEnd !== end; |
There was a problem hiding this comment.
This guard has no test
|
@NicolasBonet, all done on my end. Over to you. Thanks! |
|
closing in favor of #104756 |
Explanation of Change
Lets workspaces use a $0 distance rate, for mileage that's tracked but not reimbursed.
validateRateValueaccepts 0 and only rejects negative rates.lowRateErrornow reads "Rate can't be negative".!ratebecomesrate === undefined). This removes the "Pending..." merchant, the false "Rate out of policy" error and the disabled rate field. A distance of 0 stays blocked.Saving a $0 split also needs the Auth change in https://github.com/Expensify/Auth/pull/25159. Without it,
SplitTransactionreturns402 Distance rate missing from policy.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/685239
PROPOSAL: N/A
Tests
0.-1...0.and saves as $0.00.0.Offline tests
QA Steps
Same as tests. Step 24 requires https://github.com/Expensify/Auth/pull/25159 to be deployed.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Brave (Chromium), local dev environment.
Steps 1-3: the $0 rate saves
Steps 4-5: the minus sign is stripped, so -1 saves as 1
Steps 6-7: "." becomes "0." and saves as $0.00
Steps 8-10: $0.00 amount and the rate name, with no Pending or out-of-policy error
Steps 11-12: the saved $0 expense
Steps 15-16: Split is available
Steps 17-18: two $0 splits of 6 mi each
Step 19: the amounts are read-only, with the info message
Step 20: the percentages are read-only, with the same message
Step 21: third split added (6 / 6 / 0 mi)
Step 22: made even (4 / 4 / 4 mi), all $0
Steps 23-24: the $0 split is saved as three 4 mi splits at $0.00 (with https://github.com/Expensify/Auth/pull/25159 running locally)
Steps 25-26: $0.76 split, $3.04 is 4 mi and $6.08 is 8 mi
Steps 25-26: the $0.76 split is saved
Step 26: at $0.76 the amounts are still editable and no message shows
Offline: the $0 expense shows as pending
Offline: saved once back online