Add text-based filter length validation#94013
Conversation
melvin-bot
Bot
requested review from
aimane-chnaif and
heyjennahay
and removed request for
a team and
heyjennahay
|
@aimane-chnaif 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![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: 0c482d10fc
ℹ️ 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".
|
@aimane-chnaif bump for review |
|
Sorry forgot this PR. Please pull main |
|
@aimane-chnaif done |
|
Should we set max width to the pill? Happening on production so not blocker. 
|
|
Please address bot review comments. |
|
Done |
|
Confirmed: Merchant: 255 |
|
I noticed different behavior in android native. On android, there's character limit natively. Max 5000 characters so we can't see error for Report ID, Report field, Keyword, Title. android.mov |
|
Added a note |
|
@luacmartins ready for you! |
luacmartins
left a comment
luacmartins
left a comment
There was a problem hiding this comment.
Looks good. Just one minor comment. We also have conflicts now
| onChange: (value: string | undefined) => void; | ||
| }; | ||
|
|
||
| function isTextInput(el: BaseTextInputRef | RNTextInput | null): el is RNTextInput { |
There was a problem hiding this comment.
Same for the other instances below
| function isTextInput(el: BaseTextInputRef | RNTextInput | null): el is RNTextInput { | |
| function isTextInput(element: BaseTextInputRef | RNTextInput | null): element is RNTextInput { |
|
@luacmartins reviewed before & all comments are addressed, so going to go ahead and merge this since its blocking #95023 |
|
🚧 JS00001 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/JS00001 in version: 9.4.29-0 🚀
|
|
🤖 I reviewed the changes in this PR against the help site files under No help site changes are required. This PR adds a max-length validation guardrail to the text-based Search filters (Merchant, Description, Title, Report ID, Keyword, Withdrawal ID) on the Spend page — when the entered value exceeds the field's character limit, an inline error is shown and the Apply/Confirm button is blocked. The only article that documents these filters, Getting Started with the Spend Page, explains how to apply filters but does not document per-field character limits, and it shouldn't need to — these limits are an internal input-validation guardrail rather than a user-facing feature. Documenting arbitrary character counts (e.g. 1000 for Description, 255 for Merchant) would add noise without helping users, and the values could drift from the code over time. No draft PR was created since there is nothing to change. @bernhardoj, please confirm you agree no help site update is needed here. If you'd like the character limits documented anyway, let me know and I'll open a draft PR. |
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.4.31-0 🚀
|
|
🤖 I reviewed the changes in this PR against Expensify's help site files under No help site changes are required. This PR adds max-length input validation to the text-based Search filters (Merchant, Description, Report ID, keyword, Title, Withdrawal ID) and report-field text filters — showing an inline error and blocking apply/confirm when a value exceeds its character limit. This is an internal UI guardrail, not a documented, user-facing feature. I checked the Search-related help articles and none of them document character/length limits for filter inputs:
The change doesn't alter any documented workflow, tab name, settings label, or button, so there's nothing to update on the help site and no draft PR is needed. @bernhardoj, if you believe this behavior (e.g. the specific max-length limits) should be documented for users, let me know and I'll draft a help site PR. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.31-0 🚀
Bundle Size Analysis (Sentry): |
6 participants
Explanation of Change
Fixed Issues
$ #84877
PROPOSAL:
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Go to Spend
Enter a very long value into the text-based filter that exceeds its max. length
Max length:
Description: 1000
Merchant: 255
Title: 10000
Others: 15000
NOTE: Android natively has a max length of 5000
Verify an error is shown
Verify pressing the apply/confirm button does nothing
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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