Add useCardFeeds regression tests for empty linkedPolicyIDs#88359
Add useCardFeeds regression tests for empty linkedPolicyIDs#88359
Conversation
Pin the corrected behavior of the include predicate in useCardFeeds: domain feeds whose `linkedPolicyIDs` is `[]` or `['']` (a legacy/invalid shape persisted by older clients and by DomainAPI's default when `preferredPolicy` was empty) should still match via `preferredPolicy` rather than being silently dropped. The predicate fix itself shipped in PR #88136; this commit adds the regression coverage that PR did not. Made-with: Cursor
|
@ChavdaSachin 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] |
Made-with: Cursor
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
There was a problem hiding this comment.
Pull request overview
Adds regression coverage to ensure useCardFeeds correctly includes/excludes domain feeds based on linkedPolicyIDs edge-cases (notably [] and ['']) and multi-policy linkage, without changing production code.
Changes:
- Add 4 new unit tests covering
linkedPolicyIDs: [''],linkedPolicyIDs: [], non-matching non-emptylinkedPolicyIDs, and multi-entrylinkedPolicyIDsincluding the current policy. - Group the new cases under a dedicated
linkedPolicyIDs predicate filteringdescribe block.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review feedback on PR #88359: gate the four new linkedPolicyIDs tests on `result.current[1].status === 'loaded'` (the ResultMetadata from useOnyx) before running the assertion, so that negative checks don't pass vacuously while Onyx is still loading. Verified locally that all 7 tests pass on main, and that reverting the predicate to its pre-#88136 form still correctly fails the `linkedPolicyIDs: ['']` and `linkedPolicyIDs: []` regression tests. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…eed helper Addresses Copilot's DRY suggestion on PR #88359 by extracting a single setupDomainFeed helper at the top of the linkedPolicyIDs predicate filtering describe block. Each of the four tests now reads as a single line of intent: the setup payload and the .toBe(true|false) expectation. Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review comment on PR #88359 (#88359 (comment)): the four new tests previously matched feeds via key.includes(oauthFeed) and gated on result.current[1].status === 'loaded' (an Onyx implementation detail). Now compute the exact combined feed key with getCardFeedWithDomainID and put toHaveProperty([combinedFeedKey]) inside waitFor for the three positive cases, so the wait resolves on the actual expectation. The negative case keeps the loaded-status gate before asserting absence to avoid a vacuous pass while Onyx is still hydrating. Verified locally: - All 7 tests pass on main. - Reverting the predicate to its pre-#88136 form still fails the two linkedPolicyIDs: [''] / [] regression tests. Made-with: Cursor
|
🚀 Deployed to staging by https://github.com/nkuoch in version: 9.3.61-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
PR #88136 (merged earlier today) fixed the
useCardFeedsinclude predicate so domain feeds whoselinkedPolicyIDsis[]or['']no longer get silently dropped, falling back topreferredPolicy/domainIDmatching as intended. That PR didn't add coverage inuseCardFeeds.test.ts, so this PR pins the corrected behavior with regression tests against the exact onyx shape that triggered the customer issue.linkedPolicyIDs: ['']with matchingpreferredPolicy-> feed is included.linkedPolicyIDs: []with matchingpreferredPolicy-> feed is included.linkedPolicyIDs: ['OTHER_POLICY'], currentpolicyIDnot in the list -> feed is excluded (regression guard for non-emptylinkedPolicyIDssemantics).linkedPolicyIDs: ['OTHER_POLICY', policyID]-> feed is included (multi-policy linkage).I verified locally that all 4 new tests fail when the predicate is reverted to the pre-#88136 short-circuit version, and pass against
main.No production code is changed.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/548636
PROPOSAL:
Tests
```ts
if (combinedCardFeed?.linkedPolicyIDs) {
return combinedCardFeed.linkedPolicyIDs.includes(policyID);
}
```
and confirm the two new `includes domain feeds when linkedPolicyIDs ...` tests fail.
Offline tests
N/A - tests-only change.
QA Steps
N/A - tests-only change, no production code modified.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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