Xero authentication flow #40277

SzymczakJ · 2024-04-16T09:51:34Z

Details

This PR is part of the Xero integration project where we supports integration with Xero, an accounting software, on New Expensify.

This PR adds a button that the user can click to start the authentication flow with the Xero, similarily to QBO authentication flow. On both web and native platforms, this button opens up a new web page where the user can authenticate with the Xero. After the authentication is complete, the user's authentication token is stored in the backend and the web page closes.

On a web browser, the web page is opened on a separate browser window. On iOS and Android, a temporary browser window is opened as a modal within the app to display the web page.

This PR also adds support to 'Other integrations' section on PolicyAccountingPage. When one of accounting integration is successfully connected, then the rest of integrations is put into 'Other integrations' section.

Fixed Issues

$ #39725
$ #39955
PROPOSAL:

Tests

To test this you need to add .env file in root of the project with following content
ENVIRONMENT=staging

Create a Collect type workspace that has areConnectionsEnabled = true prop(you can set it on OldDot by writing commands in console like on screen-shot)
On workspace page go to the "More features" and enable "Accounting"
Go to the new "Accounting" menu entry and try setting up connection with Xero.
Try to set up Xero connection.
Try to open up 'Other integrations' and set up QBO(modal with disconnecting from QBO should pop up)

Try connecting to Xero from 'Other integrations' the same way you connected to QBO.
Try disconnecting from Xero by clicking three dots and then 'Disconnect'

Offline tests

QA Steps

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

melvin-bot · 2024-04-19T16:15:41Z

@mollfpr 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]

filip-solecki

Add also ES translations

src/components/ConnectToQuickbooksOnlineButton/index.native.tsx

src/components/ConnectToXeroButton/index.native.tsx

src/pages/workspace/accounting/PolicyAccountingPage.tsx

filip-solecki · 2024-04-22T08:56:19Z

src/pages/workspace/accounting/PolicyAccountingPage.tsx

+                const {title, icon, setupConnectionButton} = accountingIntegrationData(integration, policyID, translate, styles);
+                return {
+                    icon,
+                    iconType: 'avatar',


use CONST.ICON_TYPE_AVATAR

src/pages/workspace/accounting/PolicyAccountingPage.tsx

SzymczakJ · 2024-04-22T15:49:57Z

bump @mollfpr 😃

filip-solecki · 2024-04-22T20:57:15Z

src/libs/actions/connections/ConnectToXero.ts

+
+const getXeroSetupLink = (policyID: string) => {
+    const params = new URLSearchParams({policyID});
+    const commandURL = getCommandURL({command: 'ConnectPolicyToXero', shouldSkipWebProxy: true});


is it the final version and we are sure we want to skip web proxy? 😄

Right now proxy causes 407 Proxy Authentication Required error. When we adjust backend to work with proxy then we can remove this, but let's not allow this to block us

src/types/onyx/Policy.ts

mananjadhav · 2024-04-23T14:59:59Z

I'll be back on the review here in an hour.

rushatgabhane · 2024-04-28T14:37:01Z

Did you create a new account?

Yes

Did you setup any data in that? All I did is create a new Xero account and a basic company with no details/data

Yes same.

lakchote · 2024-04-29T14:14:39Z

A PR to fix the sync progress not being displayed is under review, and will be merged as soon as possible.

lakchote · 2024-04-29T15:03:29Z

@cdOut you'll need to update the translations with this copy:

English:

Disconnect Xero
Are you sure you want to disconnect Xero?

Spanish:

Desconectar Xero
¿Estás seguro de que quieres desconectar Xero?

English:

Disconnect Xero
Are you sure you want to disconnect Xero to set up QuickBooks Online?

Spanish:

Desconectar Xero
¿Estás seguro de que quieres desconectar Xero para configurar QuickBooks Online?

…ton component

cdOut · 2024-04-29T16:44:34Z

@mananjadhav I've pushed fixes to the recently added suggestions and also updated the translations. PR should be ready to be re-tested, let me know if I've missed any issues.

mananjadhav · 2024-04-29T18:33:05Z

Overall the auth change works, but disconnect doesn't.

Clicking on Disconnect doesn't work for Xero.
Clicking on Cancel doesn't land me back to the App.
On Desktop after successful connection it didn't redirect back to the Desktop app.
There's a slight delay after successful connection, and you end up seeing the setup clickable button. In extension to this, should we be having a loader state when the connection is being authentication?
When I click on Sync now for Xero, it shows Importing your Quickbooks Online data.

Screen.Recording.2024-04-29.at.11.57.57.PM.mov

mananjadhav · 2024-04-29T19:05:53Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android: Native
- Android: mWeb Chrome
- iOS: Native
- iOS: mWeb Safari
- MacOS: Chrome / Safari
- MacOS: Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

android-xero-authentication_J1ZqJV0D.mp4

Android: mWeb Chrome

mweb-chrome-xero-authentication.mov

iOS: Native

iOS: mWeb Safari

mweb-safari-xero-authentication.mov

MacOS: Chrome / Safari

web-xero-authentication.mov

MacOS: Desktop

desktop-xero-authentication.mov

mountiny

@mananjadhav Thanks! I think we can go ahead with this now and we can fix the Xero disconnect issue as a quick follow up. Getting this to staging will help us get more testing on this flow

lakchote · 2024-04-29T20:49:33Z

Overall the auth change works, but disconnect doesn't.

Clicking on Disconnect doesn't work for Xero.

Clicking on Cancel doesn't land me back to the App.

On Desktop after successful connection it didn't redirect back to the Desktop app.

There's a slight delay after successful connection, and you end up seeing the setup clickable button. In extension to this, should we be having a loader state when the connection is being authentication?

When I click on Sync now for Xero, it shows Importing your Quickbooks Online data.

Thank you for this recap on what needs to be fixed.

Regarding 4. the slight delay with the Set upclickable button, there's an issue for this that has been created for QBO. You can take a look here. We should take a moment to think on how to handle this (a new fullSync param/logic was added to QBO job).

Concerning the loading states, we should have them, it'll be fixed when the PR to fix push notifications will be merged.

For the reasons mentioned above by @mountiny, I'm going to proceed with the merge so we can identify and fix as soon as possible eventual new issues that'll arise on staging.

OSBotify · 2024-04-29T21:00:28Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-04-29T23:33:51Z

🚀 Cherry-picked to staging by https://github.com/francoisl in version: 1.4.67-7 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

OSBotify · 2024-04-30T01:13:48Z

🚀 Deployed to staging by https://github.com/lakchote in version: 1.4.68-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2024-04-30T02:34:36Z

🚀 Cherry-picked to staging by https://github.com/francoisl in version: 1.4.67-7 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

OSBotify · 2024-05-01T02:37:58Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.4.68-3 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

Santhosh-Sellavel · 2024-05-21T17:30:58Z

src/components/ConnectToQuickbooksOnlineButton/index.native.tsx

                        setIsDisconnectModalOpen(true);
                        return;
                    }
                    setWebViewOpen(true);
                }}
                text={translate('workspace.accounting.setup')}
-                style={styles.justifyContentCenter}


We deleted this style caused a regression here

rushatgabhane · 2024-06-26T00:19:54Z

src/components/ConnectToXeroButton/index.native.tsx

+                onClose={() => setWebViewOpen(false)}
+                fullscreen
+                isVisible={isWebViewOpen}
+                type={CONST.MODAL.MODAL_TYPE.CENTERED}


This modal type has a bug for android. More details here: #40978 (comment)

SzymczakJ added 11 commits April 11, 2024 16:53

add xero to accounting page

cda7dd0

add other integrations section

6bfcf11

add types

6d5aee1

Merge branch 'main' into @szymczak/xero-auth-flow

5c04043

make connect to integrations work

74260e0

fix connections empty object bug

ed73632

make accounting integrations more generative

b8a7641

add disconnection logic before connecting to another integration

693bd94

move ConnectToQBO to components

664b96b

Merge branch 'main' into @szymczak/xero-auth-flow

9a55673

refactor PolicyAccountingPage

a5a0fea

SzymczakJ changed the title ~~xero auth flow~~ Xero authentication flow Apr 19, 2024

SzymczakJ marked this pull request as ready for review April 19, 2024 16:15

SzymczakJ requested a review from a team as a code owner April 19, 2024 16:15

melvin-bot bot requested review from mollfpr and removed request for a team April 19, 2024 16:15

filip-solecki reviewed Apr 22, 2024

View reviewed changes

SzymczakJ added 2 commits April 22, 2024 13:45

fix PR comments

c94db81

fix type errors

deed5c5

mountiny assigned SzymczakJ Apr 22, 2024

mountiny requested review from mountiny and mananjadhav and removed request for mollfpr April 22, 2024 15:58

filip-solecki reviewed Apr 22, 2024

View reviewed changes

src/types/onyx/Policy.ts Show resolved Hide resolved

SzymczakJ added 2 commits April 23, 2024 11:34

remove unneccessary props

9d7f085

Merge branch 'main' into @szymczak/xero-auth-flow

64a3ab9

cdOut added 2 commits April 29, 2024 17:23

apply fix for QBO set up crash

8d24dc0

apply corrected translations

2ad1e72

cdOut dismissed mountiny’s stale review via 2ad1e72 April 29, 2024 16:13

cdOut added 2 commits April 29, 2024 18:29

Merge branch 'main' into @szymczak/xero-auth-flow

7ceb7c6

add the displayName property back to the ConnectToQuickbooksOnlineBut…

04df83f

…ton component

mananjadhav approved these changes Apr 29, 2024

View reviewed changes

mountiny approved these changes Apr 29, 2024

View reviewed changes

lakchote approved these changes Apr 29, 2024

View reviewed changes

lakchote merged commit 49db149 into Expensify:main Apr 29, 2024
17 checks passed

github-actions bot mentioned this pull request Apr 29, 2024

Deploy Checklist: New Expensify 2024-04-29 #41266

Closed

74 tasks

melvin-bot bot mentioned this pull request May 1, 2024

[HOLD for payment 2024-05-08] [Wave Collect] [Xero] Add Other Integrations and Xero section in the WorkspaceAccountingPage #39955

Closed

lakchote mentioned this pull request May 6, 2024

[Wave Collect] [Xero] Fix padding issue in ConnectionLayout #41561

Merged

50 tasks

mananjadhav mentioned this pull request May 9, 2024

[Xero] [Export Flow] Create the Bank account select page #41916

Merged

50 tasks

neonbhai mentioned this pull request May 16, 2024

[HOLD for payment 2024-05-22] [$250] [QBO] Accounting - "Set up" button is top aligned in respective to "QBO" wording #41301

Closed

6 tasks

Santhosh-Sellavel reviewed May 21, 2024

View reviewed changes

c3024 mentioned this pull request Jun 20, 2024

[Wave Collect][Xero] Enforce 2FA for xero #44059

Merged

50 tasks

rushatgabhane reviewed Jun 26, 2024

View reviewed changes

rushatgabhane mentioned this pull request Jun 26, 2024

[HOLD for payment 2024-06-28] [HOLD for payment 2024-06-24] Accounting - Not able to set up Quickbooks, Intuit page freezes #40978

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Xero authentication flow #40277

Xero authentication flow #40277

SzymczakJ commented Apr 16, 2024 •

edited by mountiny

Loading

melvin-bot bot commented Apr 19, 2024

filip-solecki left a comment

filip-solecki Apr 22, 2024

SzymczakJ commented Apr 22, 2024

filip-solecki Apr 22, 2024

SzymczakJ Apr 23, 2024

mananjadhav commented Apr 23, 2024

rushatgabhane commented Apr 28, 2024

lakchote commented Apr 29, 2024

lakchote commented Apr 29, 2024 •

edited

Loading

cdOut commented Apr 29, 2024

mananjadhav commented Apr 29, 2024 •

edited

Loading

mananjadhav commented Apr 29, 2024

mountiny left a comment

lakchote commented Apr 29, 2024 •

edited

Loading

OSBotify commented Apr 29, 2024

OSBotify commented Apr 29, 2024

OSBotify commented Apr 30, 2024

OSBotify commented Apr 30, 2024

OSBotify commented May 1, 2024

Santhosh-Sellavel May 21, 2024

rushatgabhane Jun 26, 2024 •

edited

Loading

Xero authentication flow #40277

Xero authentication flow #40277

Conversation

SzymczakJ commented Apr 16, 2024 • edited by mountiny Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented Apr 19, 2024

filip-solecki left a comment

Choose a reason for hiding this comment

filip-solecki Apr 22, 2024

Choose a reason for hiding this comment

SzymczakJ commented Apr 22, 2024

filip-solecki Apr 22, 2024

Choose a reason for hiding this comment

SzymczakJ Apr 23, 2024

Choose a reason for hiding this comment

mananjadhav commented Apr 23, 2024

rushatgabhane commented Apr 28, 2024

lakchote commented Apr 29, 2024

lakchote commented Apr 29, 2024 • edited Loading

cdOut commented Apr 29, 2024

mananjadhav commented Apr 29, 2024 • edited Loading

mananjadhav commented Apr 29, 2024

Reviewer Checklist

Screenshots/Videos

mountiny left a comment

Choose a reason for hiding this comment

lakchote commented Apr 29, 2024 • edited Loading

OSBotify commented Apr 29, 2024

OSBotify commented Apr 29, 2024

OSBotify commented Apr 30, 2024

OSBotify commented Apr 30, 2024

OSBotify commented May 1, 2024

Santhosh-Sellavel May 21, 2024

Choose a reason for hiding this comment

rushatgabhane Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

SzymczakJ commented Apr 16, 2024 •

edited by mountiny

Loading

lakchote commented Apr 29, 2024 •

edited

Loading

mananjadhav commented Apr 29, 2024 •

edited

Loading

lakchote commented Apr 29, 2024 •

edited

Loading

rushatgabhane Jun 26, 2024 •

edited

Loading