Repository navigation
Remove legacy violations suggested searches from the Spend UI - #103143
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@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] |
| CONST.SEARCH.SEARCH_KEYS.TOP_MERCHANTS, | ||
| ] as const satisfies SearchKey[]; | ||
|
|
||
| const insightsPageMenuKeys = new Set<SearchKey>([...SPEND_INSIGHT_KEYS, CONST.SEARCH.SEARCH_KEYS.VIOLATIONS_BY_SUBMITTER]); |
There was a problem hiding this comment.
(Neil's AI agent)
NAB: insightsPageMenuKeys and insightsSearchKeys in createTypeMenuSections hold the same list. Can the Insights section loop iterate insightsPageMenuKeys instead (a Set keeps insertion order), so a future insight can't be added to one and missed in the other?
| expect(menuKeys).not.toContain(CONST.SEARCH.SEARCH_KEYS.VIOLATIONS_BY_SUBMITTER); | ||
| expect(menuKeys).toContain(CONST.SEARCH.SEARCH_KEYS.EXPENSES); | ||
| expect(menuSections.find((section) => section.translationPath === 'search.tabs.insights')).toBeUndefined(); | ||
| expect(SearchUIUtils.getSuggestedSearches(adminAccountID)[CONST.SEARCH.SEARCH_KEYS.VIOLATIONS_BY_SUBMITTER].key).toBe(CONST.SEARCH.SEARCH_KEYS.VIOLATIONS_BY_SUBMITTER); |
There was a problem hiding this comment.
(Neil's AI agent)
NAB: this only checks that the entry's key equals the constant it's defined with in getSuggestedSearches, so it doesn't test any behavior. The new useSearchKeyParam test already proves the search still resolves, so I think this line can be removed.
neil-marcellini
left a comment
There was a problem hiding this comment.
Good to go, thank you. I don't think we need a C+.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariNo C+ needed for a simple removal. Pasting this to pass the check. |
|
🚧 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.4-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.4-3 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
We're going to move the Insight to a separate tab, then we want to remove
Insightsuggested searches fromSpendpageFixed Issues
$ #102811
PROPOSAL:
Tests
Precondition:
insightsPagebeta is enabledInsightsuggested searches don't appear anymoreOffline 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-10-06.at.14.09.42.mov
Android: mWeb Chrome
Screen.Recording.2026-10-06.at.14.08.01.mov
iOS: Native
Screen.Recording.2026-10-06.at.14.08.16.mov
iOS: mWeb Safari
Screen.Recording.2026-10-06.at.14.10.07.mov
MacOS: Chrome / Safari