Support domain and workspace feed with the same name under one workspace#74356
Conversation
…kspace-cards-same-name # Conflicts: # src/pages/workspace/companyCards/BankConnection/index.native.tsx # src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx # src/pages/workspace/companyCards/assignCard/AssigneeStep.tsx # src/pages/workspace/companyCards/assignCard/InviteNewMemberStep.tsx # src/pages/workspace/companyCards/assignCard/TransactionStartDateStep.tsx
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…kspace-cards-same-name # Conflicts: # src/hooks/useIsBlockedToAddFeed.ts # src/pages/workspace/companyCards/BankConnection/index.native.tsx # src/pages/workspace/companyCards/WorkspaceCompanyCardFeedSelectorPage.tsx # tests/unit/hooks/useIsBlockedToAddFeed.test.ts
Could you share it 😅 ? A snippet of mock onyx data would be really helpful |
…kspace-cards-same-name # Conflicts: # src/libs/CardUtils.ts # src/pages/workspace/companyCards/WorkspaceCompanyCardsErrorConfirmation.tsx # src/pages/workspace/companyCards/WorkspaceCompanyCardsSettingsPage.tsx
|
Chatted with @VickyStash about the test steps. I'll complete the checklist today. @VickyStash Could you resolve the conflict in the meantime? Ty |
…kspace-cards-same-name # Conflicts: # src/pages/workspace/companyCards/WorkspaceCompanyCardsListHeaderButtons.tsx
puneetlath
left a comment
There was a problem hiding this comment.
It seems to generally make sense to me but I find the terms combinedFeed and originalFeed kind of confusing. It's not super obvious to me what either of those mean.
Can you describe them to me and maybe we can workshop some potential alternative terms?
| import type {CompanyCardFeed} from '@src/types/onyx'; | ||
|
|
||
| type CardFeedListItem = ListItem & { | ||
| /** Combined feed key */ |
There was a problem hiding this comment.
Can we explain what this is a bit more. I don't think it's obvious what "combined feed" means.
There was a problem hiding this comment.
Can you describe them to me and maybe we can workshop some potential alternative terms?
Sure!
It's a key that combined fromfeed + domainID/workspaceAccountID depending on the feed source.
Example:
original feed: oauth.chase.com
combined feed: oauth.chase.com#123456
@puneetlath What do you think?
|
✋ 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/puneetlath in version: 9.2.59-0 🚀
|
|
@VickyStash @puneetlath this is ready for Internal QA |
Revert "Merge pull request #74356 from callstack-internal/VickyStash/…
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.2.60-0 🚀
|
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.2.60-0 🚀
|
|
🚀 Deployed to staging by https://github.com/puneetlath in version: 9.2.60-0 🚀
|
|
🚀 Deployed to production by https://github.com/grgia in version: 9.2.59-5 🚀
|
|
🚀 Deployed to production by https://github.com/grgia in version: 9.2.60-2 🚀
|


Explanation of Change
This PR updates the app logic to support displaying of domain and workspace feeds even if they have the same identification.
Example:
oauth.chase.com.oauth.chase.comthat is associated with the Workspace 1.With this PR the app should show both feeds (so two Chase feeds) under Workspace 1.
Slack discussion: https://expensify.slack.com/archives/C07NMDKEFMH/p1761930535694549?thread_ts=1761257642.552319&cid=C07NMDKEFMH
Fixed Issues
$ #73380
PROPOSAL: N/A
Tests
Common steps:
Add cardsto add feed (United States -> Direct feed -> Other -> Setup feed using Plaid)6.1 Open card details
6.2 Update card name
Internal QA:
Precondition: have an account with a workspace that has:
Make sure both feeds are displayed.
Offline tests
Same, as in the Tests section
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same, as in the Tests section
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.mp4
Android: mWeb Chrome
android_web.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
ios_web.mp4
MacOS: Chrome / Safari
web.mp4
MacOS: Desktop
desktop.mp4