-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[Policy Change Logs] Fix: Rules - Unexpected defaults for max expense amount, receipt required amount, and max expense age #77639
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
[Policy Change Logs] Fix: Rules - Unexpected defaults for max expense amount, receipt required amount, and max expense age #77639
Conversation
…nt, receipt required amount, and max expense age
|
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 |
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.
|
|
Added automated tests |
|
@QichenZhu 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] |
|
Updated based on this review comment: https://github.com/Expensify/Auth/pull/18751#pullrequestreview-3618118639 |
|
resolved the merge conflicts |
|
@madmax330 - Could you confirm whether this PR needs a C+ review? |
|
Thanks! Happy to help if needed. |
Yes it does. @QichenZhu Could you help with carrying out complete testing and review of this PR? Thanks! |
|
@rayane-d, could you number each action (including verifications) so it's easier to refer to a specific one? Currently there are only 3 numbered steps but the test actually contains 105 actions. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppTest Steps 1Test Steps 1.1A changelog entry appears in the system messages thread in the #admins roomMessage format:
|
src/languages/es.ts
Outdated
| setReceiptRequiredAmount: ({newValue}) => `estableció el monto requerido de recibo en "${newValue}"`, | ||
| changedReceiptRequiredAmount: ({oldValue, newValue}) => `cambió el monto requerido de recibo a "${newValue}" (anteriormente "${oldValue}")`, | ||
| removedReceiptRequiredAmount: ({oldValue}) => `eliminó el monto requerido de recibo (anteriormente "${oldValue}")`, | ||
|
|
||
| setMaxExpenseAmount: ({newValue}) => `estableció el monto máximo de gasto en "${newValue}"`, | ||
| changedMaxExpenseAmount: ({oldValue, newValue}) => `cambió el monto máximo de gasto a "${newValue}" (anteriormente "${oldValue}")`, | ||
| removedMaxExpenseAmount: ({oldValue}) => `eliminó el monto máximo de gasto (anteriormente "${oldValue}")`, | ||
|
|
||
| setMaxExpenseAge: ({newValue}) => `estableció la antigüedad máxima del gasto en "${newValue}" días`, | ||
| changedMaxExpenseAge: ({oldValue, newValue}) => `cambió la antigüedad máxima del gasto a "${newValue}" días (anteriormente "${oldValue}")`, | ||
| removedMaxExpenseAge: ({oldValue}) => `eliminó la antigüedad máxima del gasto (anteriormente "${oldValue}" días)`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Translation confirmation: https://expensify.slack.com/archives/C01GTK53T8Q/p1768397354807899
I've fixed these issues: facd9cd |
Done |
|
🎯 @QichenZhu, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #79674. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/inimaga in version: 9.3.3-0 🚀
|
|
@rayane-d This PR is failing because of a regression issue Changelog messages about Rules are not translated to Italian, Polski, Portuguese, etc The issue is reproducible in: Web, Android, iOS Bug7050417_1768504190906.Not_translated.mp4 |
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.3.3-8 🚀
|



















































































































































































































Explanation of Change
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/576535
PROPOSAL:
Tests
Prerequisites
Test Steps 1
Steps:
Navigate to Settings > Workspaces > [Your Workspace] > More features
Enable Rules
Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Receipt required amount field
Add an amount (e.g. $100)
Save it
Navigate to the #admins room
Open the system messages thread
Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format:
set receipt required amount to "$100.00"13, Verify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Receipt required amount field
Edit the amount (e.g. change it from $100 to $200)
Save it
Navigate to the #admins room
Open the system messages thread
Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format:
changed receipt required amount to "$200.00" (previously "$100.00")Verify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Receipt required amount field
Remove the amount
Save it
Navigate to the #admins room
Open the system messages thread
Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format:
removed receipt required amount (previously "$200.00")Verify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Test Steps 2
Steps:
Expected Results:
set max expense amount to "$100.00"Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Receipt required amount field
Edit the amount (e.g. change it from $100 to $200)
Save it
Navigate to the #admins room
Open the system messages thread
Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format: It should be
changed max expense amount to "$200.00" (previously "$100.00")Verify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Receipt required amount field
Remove the amount
Save it
Navigate to the #admins room
Open the system messages thread
Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format:
removed max expense amount (previously "$200.00")Verify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Test Steps 3
Steps:
Navigate to Settings > Workspaces > [Your Workspace] > More features
Enable Rules
Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Max expense age field
Add a value (e.g. 15 days)
Save it
Navigate to the #admins room
Open the system messages thread
Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format:
set max expense age to "15" daysVerify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Max expense age field
Edit the amount (e.g. change it from 15 days to 30 days)
Save it
Navigate to the #admins room
Open the system messages thread7. Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format:
changed max expense age to "30" days (previously "15")Verify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Navigate to Settings > Workspaces > [Your Workspace] > Rules
Click on the Max expense age field
Remove the value
Save it
Navigate to the #admins room
Open the system messages thread
Verify that: A changelog entry appears in the system messages thread in the #admins room
Verify that: Message format:
removed max expense age (previously "30" days)Verify that: The LHN sidebar preview shows the correct changelog message text
Verify that: Right-click on the changelog entry, copy the message, and verify that the copied text matches the displayed message exactly
Verify that: Change app language, and verify that the changelog message is translated as expected
Verify that: Create a thread under the changelog comment, and verify that the thread report title displays the changelog message correctly
Offline tests
QA Steps
Same as tests
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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari