feat: add date bounds (startDate/endDate) to mileage rate create flow#90153
Conversation
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
…er design doc Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
![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: 6cba45c886
ℹ️ 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".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Krishna2323
force-pushed
the
krishna/89830-add-date-bounds-to-mileage-rate-create
branch
2 times, most recently
from
10bbbc0 to
f39c6e5
Compare
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a53e6cb47
ℹ️ 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".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Commented on the issue for better visibility. Details@Expensify/design The spec shows the date validation error ("Start date must occur before end date") above the Save button. However, in this PR implementation we show inline errors under each field (Name, Amount, Start date) since the form can have multiple validation errors at once — displaying only one error above Save could be confusing when there are 3+ issues to fix.PR current behavior:
Is this acceptable, or should we match the spec exactly and show only the specific date error above Save? Monosnap.screencast.2026-05-15.16-58-02.mp4
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
…e-bounds-to-mileage-rate-create
…ate-bounds-to-mileage-rate-create # Conflicts: # src/languages/es.ts
This comment has been minimized.
This comment has been minimized.
|
@Krishna2323 Conflicts and failed checks |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@thelullabyy all done |
neil-marcellini
left a comment
neil-marcellini
left a comment
There was a problem hiding this comment.
Thanks for working on this. I have a few requests and questions, but overall, it is looking good.
| enabledWhenOffline | ||
| style={[styles.flexGrow1]} | ||
| shouldHideFixErrorsAlert | ||
| shouldHideFixErrorsAlert={!isDateBoundMileageRateEnabled || !hasMultipleErrors} |
There was a problem hiding this comment.
I don't understand why we should hide this alert if there's a single error. Looks like generally in the app if the form has multiple fields then we leave this prop as its default of false. So I think it would be fine if we just set this based on the beta.
There was a problem hiding this comment.
This was suggested by @shawnborton here: #89830 (comment)
I also agree on that — I couldn't find any precedent in the codebase for conditionally showing the error alert based on error count, so I've removed it and simplified to just the beta gate. Happy to add it back if needed.
There was a problem hiding this comment.
One thing we have been doing lately is that if there is only one form error, don't show the error above the green button and only show the inline error under the field. If there are multilpe, then we can show the "Please fix all errors" message about the button. Let me know if that makes sense!
@shawnborton I want to make sure you're good with this decision. If you feel strongly about it we can make the change in a follow up, but it seems unnecessary and overcomplicates the implementation especially for a v1. We do not do this in most places in the app.
Did you mean we should hide the "fix all errors" alert based on the number of error messages present or just when the form has a single field? The former is a new code pattern, the latter already happens.
There was a problem hiding this comment.
I am saying that if you have a form with multiple inputs, if only one input has an error, just show a single error message below that input. No need to additionally show an error message above the submit button too.
We started doing that recently (I forget where exactly) to make the page feel lighter if we only have one error. Because previously, each time you had just a single error, we were showing two error messages on the page and it felt pretty unnecessarily heavy.
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Adding myself so that I can test ahead of merging |
| <View style={[styles.mh5, styles.mt4]}> | ||
| <InputWrapper | ||
| InputComponent={AmountForm} | ||
| inputID={INPUT_IDS.RATE} |
There was a problem hiding this comment.
I believe you missed this error
There was a problem hiding this comment.
Currently, it shows "Please enter a valid rate" error when there is no rate
There was a problem hiding this comment.
Nice catch! Fixed.
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@neil-marcellini @rlinoz could you please run the translation script again? |
Done https://github.com/Expensify/App/actions/runs/26179954893/job/77019998523 |
🦜 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 c59fdfdf..b7fc2c71 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -6468,6 +6468,7 @@ _Für ausführlichere Anweisungen [besuchen Sie unsere Hilfeseite](${CONST.NETSU
existingRateName: 'Ein Distanzsatz mit diesem Namen existiert bereits',
nameRequired: 'Name ist erforderlich',
startDateMustBeBeforeEndDate: 'Das Startdatum muss vor dem Enddatum liegen',
+ amountRequired: 'Betrag ist erforderlich',
},
amountPerUnit: (unit: string) => `Betrag pro ${unit}`,
startDate: 'Startdatum',
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 55a88dca..01740594 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -6399,10 +6399,11 @@ ${amount} para ${merchant} - ${date}`,
existingRateName: 'Ya existe una tasa de distancia con este nombre',
nameRequired: 'El nombre es obligatorio',
startDateMustBeBeforeEndDate: 'La fecha de inicio debe ser anterior a la fecha de fin',
+ amountRequired: 'El importe es obligatorio',
},
amountPerUnit: (unit: string) => `Importe por ${unit}`,
startDate: 'Fecha de inicio',
- endDate: 'Fecha de finalización',
+ endDate: 'Fecha de fin',
},
editor: {
nameInputLabel: 'Nombre',
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 124fa360..9f139ee0 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -6490,6 +6490,7 @@ _Pour des instructions plus détaillées, [visitez notre site d’aide](${CONST.
existingRateName: 'Un taux kilométrique portant ce nom existe déjà',
nameRequired: 'Le nom est obligatoire',
startDateMustBeBeforeEndDate: 'La date de début doit être antérieure à la date de fin',
+ amountRequired: 'Le montant est obligatoire',
},
amountPerUnit: (unit: string) => `Montant par ${unit}`,
startDate: 'Date de début',
diff --git a/src/languages/it.ts b/src/languages/it.ts
index ca51ec2f..6701d8b1 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -6456,6 +6456,7 @@ _Per istruzioni più dettagliate, [visita il nostro sito di assistenza](${CONST.
existingRateName: 'Esiste già una tariffa distanza con questo nome',
nameRequired: 'Il nome è obbligatorio',
startDateMustBeBeforeEndDate: 'La data di inizio deve essere precedente alla data di fine',
+ amountRequired: "L'importo è obbligatorio",
},
amountPerUnit: (unit: string) => `Importo per ${unit}`,
startDate: 'Data di inizio',
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index c627c73f..567f9a69 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -6384,9 +6384,10 @@ _詳しい手順については、[ヘルプサイトをご覧ください](${CO
rateNameRequired: 'レート名は必須です',
existingRateName: 'この名前の距離レートはすでに存在します',
nameRequired: '名前は必須です',
- startDateMustBeBeforeEndDate: '開始日は終了日より前である必要があります',
+ startDateMustBeBeforeEndDate: '開始日は終了日より前でなければなりません',
+ amountRequired: '金額は必須です',
},
- amountPerUnit: (unit: string) => `${unit} あたりの金額`,
+ amountPerUnit: (unit: string) => `${unit}あたりの金額`,
startDate: '開始日',
endDate: '終了日',
},
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index a46cf07d..887856a0 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -6435,6 +6435,7 @@ _Voor meer gedetailleerde instructies, [bezoek onze help-site](${CONST.NETSUITE_
existingRateName: 'Er bestaat al een afstandstarief met deze naam',
nameRequired: 'Naam is verplicht',
startDateMustBeBeforeEndDate: 'Begindatum moet vóór de einddatum liggen',
+ amountRequired: 'Bedrag is verplicht',
},
amountPerUnit: (unit: string) => `Bedrag per ${unit}`,
startDate: 'Startdatum',
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 708cb0dd..160b8915 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -6427,9 +6427,10 @@ _Aby uzyskać bardziej szczegółowe instrukcje, [odwiedź naszą stronę pomocy
existingRateName: 'Stawka za odległość o tej nazwie już istnieje',
nameRequired: 'Imię jest wymagane',
startDateMustBeBeforeEndDate: 'Data początkowa musi być wcześniejsza niż data końcowa',
+ amountRequired: 'Kwota jest wymagana',
},
amountPerUnit: (unit: string) => `Kwota za ${unit}`,
- startDate: 'Data początkowa',
+ startDate: 'Data rozpoczęcia',
endDate: 'Data zakończenia',
},
editor: {
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index d57029fe..d39e7c81 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -6435,6 +6435,7 @@ _Para instruções mais detalhadas, [visite nossa central de ajuda](${CONST.NETS
existingRateName: 'Já existe uma tarifa de distância com este nome',
nameRequired: 'Nome é obrigatório',
startDateMustBeBeforeEndDate: 'A data de início deve ser anterior à data de término',
+ amountRequired: 'O valor é obrigatório',
},
amountPerUnit: (unit: string) => `Valor por ${unit}`,
startDate: 'Data de início',
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 7bad2bfd..1e7d3281 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -6274,7 +6274,13 @@ _如需更详细的说明，请[访问我们的帮助网站](${CONST.NETSUITE_IM
one: '确定要删除此费率吗？',
other: '确定要删除这些费率吗？',
}),
- errors: {rateNameRequired: '费率名称为必填项', existingRateName: '已存在同名的里程费率', nameRequired: '姓名为必填项', startDateMustBeBeforeEndDate: '开始日期必须早于结束日期'},
+ errors: {
+ rateNameRequired: '费率名称为必填项',
+ existingRateName: '已存在同名的里程费率',
+ nameRequired: '姓名为必填项',
+ startDateMustBeBeforeEndDate: '开始日期必须早于结束日期',
+ amountRequired: '金额为必填项',
+ },
amountPerUnit: (unit: string) => `每${unit}金额`,
startDate: '开始日期',
endDate: '结束日期',
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@thelullabyy please check again. |
| currency={currency} | ||
| ref={inputCallbackRef} | ||
| /> | ||
| {isDateBoundMileageRateEnabled ? ( |
There was a problem hiding this comment.
I think it would be fine to migrate to the new model even if the beta is disabled, meaning hide only the date fields, but NAB and maybe even better not to.
neil-marcellini
left a comment
neil-marcellini
left a comment
There was a problem hiding this comment.
Thanks for the updates
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @neil-marcellini has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 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.3.79-1 🚀
Bundle Size Analysis (Sentry): |
7 participants
Explanation of Change
Fixed Issues
$ #89830
PROPOSAL:
Tests
Test 1: Beta OFF — No regression
dateBoundMileageRatebeta is NOT enabledTest 2: Beta ON — Happy path
dateBoundMileageRatebetaTest 3: Beta ON — Empty form shows all errors simultaneously
Test 4: Beta ON — Date validation
Test 5: Beta ON — Dates are optional
Offline tests
QA Steps
Same as tests
Verify that no errors appear in the JS console
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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_hybrid.mp4
Android: mWeb Chrome
android_mWeb.mp4
iOS: Native
ios_hybrid.mp4
iOS: mWeb Safari
ios_mWeb.mp4
MacOS: Chrome / Safari
web_chrome.mp4