Fetch categories and tags on merchant rule page to fix missing category row after cache clear#83139
Conversation
When categories/tags data is missing (e.g. after cache clear), the merchant rule form hides the Category and Tag rows because the data check returns false. This adds a useEffect to fetch categories and tags when the merchant rule page mounts if the data is not yet loaded, matching the existing pattern in IOURequestStepCategory. Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
@MelvinBot Mark it as ready |
|
PR has been marked as ready for review. |
|
@ShridharGoel 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] |
melvin-bot
bot
requested review from
heyjennahay
and removed request for
a team
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| if (policy?.areTagsEnabled && policyTags.length === 0) { | ||
| openPolicyTagsPage(policyID); | ||
| } |
There was a problem hiding this comment.
But there might be no tags even though they are enabled
There was a problem hiding this comment.
Good catch — policyTags.length === 0 can't distinguish between "not loaded yet" and "loaded but no tags exist," since the getTagLists selector returns [] for both undefined and {}. This would cause an infinite refetch loop when tags are enabled but none have been created.
Fixed by adding a raw policyTagsRaw Onyx subscription (without the selector/default) and checking !policyTagsRaw instead. This correctly only fetches when the data hasn't been loaded (undefined), not when it's loaded but empty ({}).
- Use policyTagsRaw (without selector) to distinguish "not loaded" (undefined)
from "loaded but empty" ({}), preventing infinite refetch when tags are
enabled but none exist
- Simplify useEffect to depend only on policyID, matching the pattern in
IOURequestStepCategory
Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
melvin-bot
bot
requested review from
ShridharGoel and
roryabraham
and removed request for
a team
|
Addressed both review comments:
|
| useNetwork({onReconnect: fetchPolicyData}); | ||
|
|
||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| useEffect(fetchPolicyData, [policyID]); |
There was a problem hiding this comment.
| useEffect(fetchPolicyData, [policyID]); | |
| useEffect(() => { | |
| fetchPolicyData(); | |
| }, [fetchPolicyData]); |
There was a problem hiding this comment.
Applied this suggestion — now using fetchPolicyData as the useEffect dependency instead of suppressing the lint rule. Thanks for catching that.
…sable Addresses review feedback to properly list fetchPolicyData in the useEffect dependency array rather than suppressing the exhaustive-deps lint rule. The useCallback already captures all relevant dependencies. Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
| const [policyTagsRaw] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, {canBeMissing: true}); | ||
| const [policyTags = getEmptyArray<ValueOf<PolicyTagLists>>()] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, { |
There was a problem hiding this comment.
Let's prefer a single Onyx subscription
There was a problem hiding this comment.
Consolidated into a single useOnyx subscription (policyTagsRaw) and now deriving policyTags via useMemo(() => getTagLists(policyTagsRaw), [policyTagsRaw]). This gives us the raw value for the !policyTagsRaw undefined check in fetchPolicyData (to avoid the infinite refetch loop) while still producing the same processed tag lists array for rendering — all from one Onyx subscription. Also cleaned up the now-unused ValueOf, PolicyTagLists, and getEmptyArray imports.
Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
| const [policyTagsRaw] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, {canBeMissing: true}); | ||
| const policyTags = useMemo(() => getTagLists(policyTagsRaw), [policyTagsRaw]); |
There was a problem hiding this comment.
| const [policyTagsRaw] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, {canBeMissing: true}); | |
| const policyTags = useMemo(() => getTagLists(policyTagsRaw), [policyTagsRaw]); | |
| const [policyTagsFromOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, {canBeMissing: true}); | |
| const policyTags = useMemo( | |
| () => getTagLists(policyTagsFromOnyx) ?? getEmptyArray<ValueOf<PolicyTagLists>>(), | |
| [policyTagsFromOnyx], | |
| ); |
There was a problem hiding this comment.
Applied — renamed to policyTagsFromOnyx and restored the getEmptyArray fallback in the useMemo. Also restored the corresponding ValueOf, PolicyTagLists, and getEmptyArray imports.
Addresses review feedback to use a clearer variable name and restore the getEmptyArray default for the derived policyTags. Co-authored-by: Shridhar Goel <ShridharGoel@users.noreply.github.com>
|
Deleted 5 comments containing "PR Reviewer Checklist". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-02-21.at.10.49.27.PM.movAndroid: mWeb ChromeScreen.Recording.2026-02-21.at.10.48.00.PM.moviOS: HybridAppScreen.Recording.2026-02-21.at.10.44.25.PM.moviOS: mWeb SafariScreen.Recording.2026-02-21.at.10.46.01.PM.movMacOS: Chrome / SafariScreen.Recording.2026-02-21.at.10.32.19.PM.mov |
|
@MelvinBot Delete this, this and this |
|
🚧 @roryabraham 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! 🧪🧪
|
|
✋ 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/roryabraham in version: 9.3.25-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.25-13 🚀
|
4 participants
Explanation of Change
After clearing cache via Troubleshoot, the Category (and Tag) rows disappear from the merchant rule form because the Onyx data for
POLICY_CATEGORIESandPOLICY_TAGSis wiped and never re-fetched. TheMerchantRulePageBasecomponent reads these from Onyx but does not trigger a fetch when the data is missing.This adds a
useEffect+useNetworkpattern (matching the existing approach inIOURequestStepCategory) to fetch policy categories and tags on mount when they're not loaded. This ensures the Category and Tag rows appear even after a cache clear.Fixed Issues
$ #83131
PROPOSAL: #83131 (comment)
Tests
Offline tests
useNetwork({onReconnect})handler will re-fetch)QA Steps
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