Repository navigation
Show nights and nightly rate under Category for multi-night reservation expenses - #104310
Conversation
…on expenses Co-authored-by: thelullabyy <[email protected]>
Co-authored-by: thelullabyy <[email protected]>
Co-authored-by: thelullabyy <[email protected]>
melvin-bot
Bot
added
Melvin-Test-Android
Melvin-Test-Web
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
🦜 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/el.ts b/src/languages/el.ts
index 02768e9a..5688e78f 100644
--- a/src/languages/el.ts
+++ b/src/languages/el.ts
@@ -144,7 +144,7 @@ const translations: TranslationDeepObject<typeof en> = {
perPerson: 'ανά άτομο',
reservationNightsWithNightlyRate: ({count, nightlyRate}: {count: number; nightlyRate: string}) => ({
one: `1 διανυκτέρευση • ${nightlyRate} ανά διανυκτέρευση`,
- other: `${count} διανυκτερεύσεις • ${nightlyRate} ανά διανυκτέρευση`,
+ other: `${count} νύχτες • ${nightlyRate} ανά νύχτα`,
}),
phone: 'Τηλέφωνο',
phoneNumber: 'Αριθμός τηλεφώνου',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 350acfc3..0e2e7035 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -140,7 +140,7 @@ const translations: TranslationDeepObject<typeof en> = {
addCardTermsOfService: 'Conditions d’utilisation d’Expensify',
perPerson: 'par personne',
reservationNightsWithNightlyRate: ({count, nightlyRate}: {count: number; nightlyRate: string}) => ({
- one: `${count} nuit • ${nightlyRate} par nuit`,
+ one: `1 nuit • ${nightlyRate} par nuit`,
other: `${count} nuits • ${nightlyRate} par nuit`,
}),
phone: 'Téléphone',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index f59993e8..ae410a8b 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -140,8 +140,8 @@ const translations: TranslationDeepObject<typeof en> = {
addCardTermsOfService: 'Expensify 利用規約',
perPerson: '1人あたり',
reservationNightsWithNightlyRate: ({count, nightlyRate}: {count: number; nightlyRate: string}) => ({
- one: `1 泊 • 1泊あたり ${nightlyRate}`,
- other: `${count} 泊 • 1泊あたり ${nightlyRate}`,
+ one: `1泊 • 1泊あたり${nightlyRate}`,
+ other: `${count}泊・1泊あたり${nightlyRate}`,
}),
phone: '電話',
phoneNumber: '電話番号',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index f51a2345..9c619fe8 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -145,8 +145,6 @@ const translations: TranslationDeepObject<typeof en> = {
perPerson: 'na osobę',
reservationNightsWithNightlyRate: ({count, nightlyRate}: {count: number; nightlyRate: string}) => ({
one: `1 noc • ${nightlyRate} za noc`,
- few: `${count} noce • ${nightlyRate} za noc`,
- many: `${count} nocy • ${nightlyRate} za noc`,
other: `${count} nocy • ${nightlyRate} za noc`,
}),
phone: 'Telefon',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index 45469451..b65a2e0c 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -140,7 +140,7 @@ const translations: TranslationDeepObject<typeof en> = {
addCardTermsOfService: 'Termos de Serviço da Expensify',
perPerson: 'por pessoa',
reservationNightsWithNightlyRate: ({count, nightlyRate}: {count: number; nightlyRate: string}) => ({
- one: `${count} noite • ${nightlyRate} por noite`,
+ one: `1 noite • ${nightlyRate} por noite`,
other: `${count} noites • ${nightlyRate} por noite`,
}),
phone: 'Telefone',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
🤖 Android test steps verification results: Step 1 passed: a workspace expense with a category shows the Category row. Steps 2 and 3 were not tested. The app froze before the tester could add the reservation dates, so these results say nothing about whether the hint works. Rerun these steps in a fresh session.
|
|
🤖 Web test steps verification results: Step 1 passed: a workspace expense with a category shows the Category row with no hint. Steps 2 and 3 were not tested. The automated tester can't run JS in the page, so it couldn't add the reservation dates. These results say nothing about whether the hint works, so a person needs to run the
|
TestsSetup
How to inject the reservation dates
Steps
Offline tests
QA Steps
|
|
@MelvinBot Please add the test steps to your PR description #104310 (comment) Besides, replace all |
|
@thelullabyy 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] |
|
Done. I made three changes to the PR:
The Screenshots/Videos sections are still empty. You may want to add your recordings there before review. |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e2b31d157
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const formattedPerAttendeeAmount = | ||
| shouldDisplayTransactionAmount && actualAmount !== undefined ? convertToDisplayString(actualAmount / (transactionAttendees?.length ?? 1), actualCurrency) : ''; | ||
| // Explains the per-night category limit check, so it uses the same reservation nights. Single-night stays show nothing. | ||
| const reservationNights = getReservationNights(updatedTransaction ?? transaction); |
There was a problem hiding this comment.
When a user replaces a SmartScanned hotel receipt, replaceReceipt() merges receiptOptimistic into the transaction without clearing hotelReservationStartDate or hotelReservationEndDate (src/libs/actions/IOU/Receipt.ts, lines 257-304). Because Onyx deep-merges the nested receipt, this call continues using the previous receipt's dates and displays its night count and nightly rate for the replacement receipt, while violations are recomputed from newTransaction, whose receipt has no dates. Clear those fields for a genuine replacement (!isSameReceipt) or suppress the hint until the replacement's metadata arrives.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. The finding is valid, and I pushed a fix in 9236c8e.
Before the fix, the optimistic MERGE in replaceReceipt() kept the old receipt's hotel dates. The hint and later limit checks then used the old stay for the new receipt.
The fix:
replaceReceipt()now setshotelReservationStartDateandhotelReservationEndDatetonullwhen the receipt is replaced with a new one (!isSameReceipt). A crop or rotate keeps the dates. If the replacement fails, the failure data still restores the old receipt, including its dates.- The two
Receiptfields now allownullwhile a replacement is pending, the same waypageCountdoes. - Tests check that a new receipt clears the dates and a crop or rotate keeps them.
Checks: typecheck, lint, and spell-check passed. ReceiptTest, MoneyRequestViewTest, and ViolationUtilsTest passed (293 of 293).
|
@MelvinBot Please check this comment #104310 (comment) and add fix if needed |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari
Screen.Recording.2026-10-22.at.21.20.29.mov |
A replacement receipt kept the old receipt's hotel dates because the optimistic merge left them in place, so the hint and per-night limit check used stale dates. A crop or rotate keeps them. Co-authored-by: thelullabyy <[email protected]>
|
@neil-marcellini 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] |
neil-marcellini
left a comment
neil-marcellini
left a comment
There was a problem hiding this comment.
Looks great thanks!
|
🚧 neil-marcellini has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/neil-marcellini in version: 9.5.20-0 🚀
|
|
No help site update needed. This PR adds a small explanatory hint under Category (for example, "3 nights • $233.33 per night") and doesn't change how limits work, so no existing sentence in |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.5.20-5 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
When SmartScan finds hotel reservation dates on a receipt, category and workspace limits are checked against the per-night average instead of the total. Nothing in the expense view explained this, so a $2,800 hotel stay with a $500 category limit showed no violation and no reason why.
This PR adds a muted hint under the Category row of the expense view, for example
3 nights • $233.33 per night.MoneyRequestViewcalls the samegetReservationNightshelper that the limit check inViolationsUtilsuses, so the hint and the limit always agree on the night count. The nightly rate is the displayed amount divided by the nights, formatted withconvertToDisplayString. This follows the existing per-attendee amount pattern.MenuItem.HelpTextafter any category error, the same way the Distance row shows its hint.common.reservationNightsWithNightlyRate({count, nightlyRate}), in all 11 locales. The whole phrase is one key so each language can set its own word order (Japanese and Chinese put the amount after "per night"). Polish usesone,few, andmanyforms.MoneyRequestViewTest.tsx: a 3-night stay shows the hint, and a 1-night stay doesn't.AI Tests (run by MelvinBot on this branch):
npm run typecheck: passednpm run linton changed files: passedoxfmt(npm run fmt): no changesnpm run spell-changed: 0 issuesnpm run react-compiler-compliance-check check src/components/ReportActionItem/MoneyRequestView.tsx: passednpm test -- tests/ui/MoneyRequestViewTest.tsx: 35 of 35 passed, including the 2 new testsnpm testsuite and the Storybook smoke test. CI covers them.To test in dev, open a workspace expense that has a category, then run this in the browser console and confirm the hint appears under Category:
Onyx.merge('transactions_<transactionID>', {receipt: {hotelReservationStartDate: '2026-10-01', hotelReservationEndDate: '2026-10-04'}})Change the end date to
2026-10-02and confirm the hint disappears.Fixed Issues
$ #102558
PROPOSAL: #102558 (comment)
Tests
Setup
transactionID.How to inject the reservation dates
receiptand save:Steps
3 nights • $233.33 per nightshows under Category. It should be styled like the Distance row hint, not as an error.3 nights • $300.00 per night.2026-10-03. Verify2 nights • $450.00 per night.2026-10-02(a single night). Verify no hint shows.2026-10-01, then to2026-09-30. Verify no hint shows in either case.3 noches • $233,33 por noche. Switch to Polish and verify thenoceform (2–4 nights) and thenocyform (5 or more, e.g. 10-01 → 10-06).Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)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