Conversation
|
Please ignore the typescript error for now as it's not really connected. I'll fix it later(I hope that it will be fixed by some other PR and I will just merge main 😃) |
src/pages/workspace/accounting/intacct/EnterSageIntacctCredentialsPage.tsx
Outdated
Show resolved
Hide resolved
| key: policy.id, | ||
| icon: policy.avatarURL ? policy.avatarURL : ReportUtils.getDefaultWorkspaceAvatar(policy.name), | ||
| iconType: policy.avatarURL ? CONST.ICON_TYPE_AVATAR : CONST.ICON_TYPE_WORKSPACE, | ||
| description: date ? `Sage Intacct - Last synced ${date}` : 'Sage Intacct', |
src/languages/en.ts
Outdated
| if (!integrationToConnect) { | ||
| return `Are you sure you want to disconnect ${currentIntegration || 'this integration'}?`; | ||
| } | ||
| return `Are you sure you want to disconnect ${currentIntegration || 'this integration'} to set up ${integrationToConnect}`; |
There was a problem hiding this comment.
we don't have an alternative for the integrationToConnect (in case it is undefined)
src/languages/es.ts
Outdated
| if (!integrationToConnect) { | ||
| return `¿Estás seguro de que quieres desconectar ${currentIntegration || 'integración'}?`; | ||
| } | ||
| return `¿Estás seguro de que quieres desconectar ${currentIntegration || 'this integration'} para configurar ${integrationToConnect}`; |
There was a problem hiding this comment.
why do we need to change this file 🤔 Are there any important changes?
There was a problem hiding this comment.
It's fix for typescript error that also happens on main. When this type error will be fixed on main this changes will not be present on this PR.
There was a problem hiding this comment.
this one seems unrelated to the PR (like the one above)
|
@hungvu193 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] |
src/pages/workspace/accounting/intacct/EnterSageIntacctCredentialsPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/intacct/EnterSageIntacctCredentialsPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/intacct/EnterSageIntacctCredentialsPage.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
This page will be empty if there's no existing connection, I think we will need a placeholder or smth here 🤔
Or Should we disable Reuse existing connection from three dot menu?
cc @yuwenmemon
There was a problem hiding this comment.
If there is no existing connection, then this page will never show up. User will be navigated directly to IntacctPrerequisitesPage
There was a problem hiding this comment.
This's what I'm seeing, I still can open it from three dot menu
Screen.Recording.2024-06-19.at.09.10.09.mov
There was a problem hiding this comment.
That's on me, now with new condition this ThreeDotsMenu won't open when there's no existing Sage Intacct connection
src/pages/workspace/accounting/intacct/IntacctPrerequisitesPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/intacct/IntacctPrerequisitesPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/intacct/EnterSageIntacctCredentialsPage.tsx
Outdated
Show resolved
Hide resolved
src/pages/workspace/accounting/intacct/ExistingConnectionsPage.tsx
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
|
I got a loop while going back from Screen.Recording.2024-06-28.at.17.00.31.mov |
|
Fixed all comments and completed author checklist, I think this is ready for merge. |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid.movAndroid: mWeb ChromeiOS: Nativeios.moviOS: mWeb SafarimSafari.movMacOS: Chrome / Safarichrome.movchrome.another.movMacOS: Desktopdesk.mov |
| SAGE_INTACCT_SYNC_IMPORT_DATA: 'intacctImportData', | ||
| SAGE_INTACCT_SYNC_IMPORT_DIMENSIONS: 'intacctImportDimensions', | ||
| SAGE_INTACCT_SYNC_IMPORT_EMPLOYEES: 'intacctImportEmployees', | ||
| SAGE_INTACCT_SYNC_IMPORT_DIMENSIONS: 'intacctImportDimensions', |
There was a problem hiding this comment.
NAB: Why move this it was in alphabetical order already 🤔
|
✋ 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/yuwenmemon in version: 9.0.4-0 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/tgolen in version: 9.0.4-5 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.5-13 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.0.6-8 🚀
|
| const lastSuccessfulSyncDate = policy.connections?.intacct.lastSync?.successfulDate; | ||
| const date = lastSuccessfulSyncDate ? datetimeToRelative(lastSuccessfulSyncDate) : undefined; | ||
| return { | ||
| title: policy.name, |
There was a problem hiding this comment.
There was a regression #45848, we have to add avatarID as policy.id so has same avatar color as the workspace.
| const policyID = policy?.id ?? '-1'; | ||
| const successfulDate = policy?.connections?.quickbooksOnline?.lastSync?.successfulDate; | ||
| const formattedDate = useMemo(() => (successfulDate ? new Date(successfulDate) : new Date()), [successfulDate]); | ||
| const successfulDate = getIntegrationLastSuccessfulDate(connectedIntegration ? policy?.connections?.[connectedIntegration] : undefined); |
There was a problem hiding this comment.
This PR caused a regression here - #45780
We should get the last successful date of the integration. Then, if connectionSyncProgress is the same integration displayed and the state is 'jobDone', get the more recent update time of the two.
| onBackButtonPress={() => Navigation.goBack(ROUTES.WORKSPACE_ACCOUNTING.getRoute(policyID))} | ||
| /> | ||
| <View style={[styles.flex1]}> | ||
| <MenuItemList |
There was a problem hiding this comment.
From this issue #59514, We forgot to add a ScrollView for the Existing connections list. If the list gets long enough to exceed the page length, it can’t be scrolled. context: #59514 (comment)
Details
Fixed Issues
$ #43532
PROPOSAL: N/A
Tests
Verify that Enter Credentials page looks like this:
Connect to Sage Intacct // right now we can connect on Old Dot
Create another workspace and try connecting it to Sage Intacct
Verify that you pop up/modal with “Create new connection” and “Reuse existing connection” options pops up
Offline tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
Screen.Recording.2024-06-28.at.13.44.06.mov
Android: mWeb Chrome
anotherandroid.mov
iOS: Native
ios.mov
iOS: mWeb Safari
mwebIOS1.mov
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov