QBD - fix: The workspace with a connection error is still listed as reusable#87468
Conversation
|
@ahmedGaber93 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@lakchote Do we consider this a bug? I see in the code the other accounting integrations not block reusing if it has sync error. I think "copy integration" should still work even if we have a sync error. |
|
Bug: Workspace is displayed on reuse list even it not synced yet, after synced and failed it disappear. 20260409151700262.mp4 |
@ahmedGaber93 this has been handled |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari20260409164447731.mp4 |
|
@srikarparsi 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] |
francoisl
left a comment
There was a problem hiding this comment.
Going to approve as it's fixing a blocker(ish), we can address the 2 comments as follow-up if needed.
| import ScreenWrapper from '@components/ScreenWrapper'; | ||
| import ScrollView from '@components/ScrollView'; | ||
| import Text from '@components/Text'; | ||
| import useAdminPoliciesConnectedToQBD from '@hooks/useAdminPoliciesConnectedToQBD'; |
There was a problem hiding this comment.
Do we still need that hook anywhere at all?
| function PolicyAccountingPage({policy}: PolicyAccountingPageProps) { | ||
| useWorkspaceDocumentTitle(policy?.name, 'workspace.common.accounting'); | ||
| const hasPoliciesConnectedToSageIntacct = useHasPoliciesConnectedToSageIntacct(); | ||
| const hasPoliciesConnectedToQBD = useHasPoliciesConnectedToQBD(); |
There was a problem hiding this comment.
Same thing for this hook, do we still need it anywhere?
|
^Sent you a cleanup PR Lucien #87512 |
|
🚧 @francoisl 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/francoisl in version: 9.3.58-0 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed the changes in this PR against the help site files under This PR is a bug fix that filters out QuickBooks Desktop workspaces with connection errors (and the current workspace) from the "Existing connections" reuse list. It changes internal filtering logic only — no user-facing feature names, UI labels, navigation steps, or workflows documented in the help site are affected. The relevant help article ( No help site changes are required for this PR. |
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.3.58-9 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.3.58-9 🚀
|
Explanation of Change
The QuickBooks Desktop reuse existing connections flow treated any admin workspace with a QBD connection as reusable, so a workspace could appear in the Existing connections list even when that workspace's QBD connection was in an error state.
Fixed Issues
$ #87457
PROPOSAL:
Tests
Preconditions: Have a workspace with the Accounting feature enabled with the next error connection: Can't connect to QuickBooks Desktop ("Data sync did not complete, please make sure the Web Connector is running and try again later.")
Steps to reach this condition:
Steps:
Expected Result:
Video test
Screen.Recording.2026-04-09.at.10.43.09.mov
Offline tests
NA
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as in tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)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