[CP prod] fix: The Violation column can be selected without the necessary filters - #101104
Conversation
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.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Thanks @nkdengineer for the PR. I'll review it today, please resolve conflict |
I've done. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb Chromeandroid-mweb.moviOS: HybridAppios.moviOS: mWeb Safariios-mwev.movMacOS: Chrome / Safariweb-resize.mp4 |
|
@nkdengineer, the code changes look good overall, with just a couple of minor comments. |
neil-marcellini
left a comment
neil-marcellini
left a comment
There was a problem hiding this comment.
Thanks for the fix. It's looking good to me. Some NAB comments that could be done in a follow up or before merging. Tom reported this issue Investigate blank Violations column on regular expense search and the fixes here should resolve it too, so I added it to the PR description. We should CP this to production.
Here are screenshots from main and this branch showing that I can reproduce the problem for an expense with an approved violation, and the problem is then solved on this branch.
Main:
This branch:
| isPolicyTaxEnabled?: boolean; | ||
| fallbackPolicyID?: string; | ||
| sortBy?: SearchSortBy; | ||
| shouldShowViolationsColumn?: boolean; |
There was a problem hiding this comment.
NAB: It looks like we compute this boolean at every call site, so it might be easier to pass in the queryJSON and compute this within the function, but not a big deal either way.
neil-marcellini
changed the title
|
@neil-marcellini Yeah, the same issue wwith Tom's issue was reported in Slack yesterday then I fixed it here |
|
Can you pls DM me a link to that Slack report? Or otherwise comment there and make sure we don't have duplicate issues? |
|
✋ 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! 🧪🧪
|
[CP prod] fix: The Violation column can be selected without the necessary filters (cherry picked from commit 15012f0) (cherry-picked to staging by luacmartins)
|
🚀 Cherry-picked to staging by https://github.com/luacmartins in version: 9.4.78-3 🚀
|
|
Yes — help site changes were required, and I opened a draft PR: Docs updates for E/A#101104 Violations column requires a violation filter. Why the docs needed updating: this PR makes the Violations column conditional — it's only listed in Display > Edit columns when What changed (3 articles, docs-only):
UI label verification and authoring complianceI drove dev NewDot web to confirm labels and current behavior:
The writing follows Per the @nkdengineer, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.78-3 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Cherry-picked to staging by https://github.com/luacmartins in version: 9.4.80-0 🚀
|
|
Yes — help site changes were required. Draft PR: Docs updates for E/A#101104 Violations column requires violation filter ( Why the docs needed updating: this PR makes the Violations column conditional. It no longer appears just because the returned expenses carry violation data — it now requires the Submitted violation or Approved violation option under the Has filter, and while one of those is applied the column is always shown and can't be deselected. Three articles described the old behavior.
How the copy was verifiedUI labels were read from the live web UI (accessibility snapshot, not inferred from screenshots) and cross-checked against Two things worth knowing about that check:
One check I could not complete locally: @nkdengineer, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.80-0 🚀
Bundle Size Analysis (Sentry): |
5 participants
Explanation of Change
fix: The Violation column can be selected without the necessary filters
Fixed Issues
$ #100877
$ #101212
PROPOSAL: #100877 (comment)
Tests
Violationscolumn isn't displayed in the tableViolationsoption is not displayed in the listFilter>Has> SelectSubmitted violation/Approved violationViolationscolumn is displayed in the tableViolationsoption is displayed in the list and can't be deselectedOffline tests
QA Steps
Same as test
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
Screen.Recording.2026-09-14.at.22.04.41.mov
Android: mWeb Chrome
Screen.Recording.2026-09-14.at.22.02.20.mov
iOS: Native
Screen.Recording.2026-09-14.at.22.08.52.mov
iOS: mWeb Safari
Screen.Recording.2026-09-14.at.22.03.52.mov
MacOS: Chrome / Safari
Screen.Recording.2026-09-14.at.21.59.04.mov