[#Wave-Control - Add Sage Intacct] Add connection sync progress and error handling.

[hungvu193]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Tracking GH: https://github.com/Expensify/Expensify/issues/388780

Design doc section:
https://docs.google.com/document/d/1k3ZFw8KB55yPUSCG6KYZlwpwEtmRt3eUshwxs7bZq5I/edit#heading=h.8m1ckdwmo1e5

A follow up of #43532

Issue Owner

Current Issue Owner: @miljakljajic

[melvin-bot]

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[hungvu193]

cc @yuwenmemon @SzymczakJ

[SzymczakJ]

Hey! I’m Jakub Szymczak from Software Mansion, an expert agency, and I’d like to work on this issue!

[yuwenmemon]

Thanks @hungvu193 !

[melvin-bot]

Current assignee @miljakljajic is eligible for the NewFeature assigner, not assigning anyone new.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[melvin-bot]

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

[SzymczakJ]

reminding myself to implement "choose Entity" functionality in this PR

[SzymczakJ]

Couple questions on how thing on BE are going:

1. What about sending Sage Intacct sync progress from BE like we do for QBO/Xero? (edit: connectionSyncProgress key is not present in onyx during the Sage Intacct Sync)
2. Is UpdateSageIntacctEntity endpoint ready? When connecting to Sage Intacct for the first time, there's no config.entity field. Shouldn't we provide connection with some initial setting like we do on Xero with Xero Organization(one of organizations is chosen arbitrarily)?
3. Bumping my question that I made on Slack
   cc @yuwenmemon @NikkiWines

[NikkiWines]

What about sending Sage Intacct sync progress from BE like we do for QBO/Xero?

Pretty sure @yuwenmemon added this already (https://github.com/Expensify/Integration-Server/pull/7974, right, @yuwenmemon?)

Is UpdateSageIntacctEntity endpoint ready? When connecting to Sage Intacct for the first time, there's no config.entity field. Shouldn't we provide connection with some initial setting like we do on Xero with Xero Organization(one of organizations is chosen arbitrarily)?

Yep, the UpdateSageIntacctEntity endpoint is on prod already. If the user is using the Top level entity we store an empty string, otherwise we store the ID associated with the entity. I believe by default we use the Top level (if available) which might be why you're not seeing an initial value

Bumping my question that I made on Slack
cc @yuwenmemon @NikkiWines

Looking at this now, will answer in the thread

[SzymczakJ]

Another two question since I forgot to ask this:

1. What about an endpoint for connecting by reusing other policy's credentials? I remember that at some point we talked about it, but it's not mentioned in doc.
2. What about SyncPolicyToSageIntacct API command, is it already up?

[NikkiWines]

What about an endpoint for connecting by reusing other policy's credentials? I remember that at some point we talked about it, but it's not mentioned in doc.

I'm not sure about this! I'll take a look at what we do for Expensify Classic now.

What about SyncPolicyToSageIntacct API command, is it already up?

Yes, this is already on prod!

[SzymczakJ]

Another batch of questions:

1. SyncPolicyToSageIntacct is working, but it's not sending sync progress, connectionSyncProgress is not updated by BE. The same happens when we connect to Sage Intacct for the first time. Is there any estimate on when this will be ready?
2. What about an endpoint for connecting by reusing other policy's credentials? Or maybe we're able to reuse some existing endpoint?
3. What about error handling in case when user gives wrong credentials? Right now any errors that happen with Sage Intacct sync are put into policy.connections object, but when user gives wrong credentials we're not going to connect him to Sage Intacct and therefore there will be no policy.connections object and no error. Related [slack thread].(https://swmansion.slack.com/archives/C06ML6X0W9L/p1720179157722519?thread_ts=1719271507.184899&cid=C06ML6X0W9L)

4. Found this comment related to tax sync in the design doc:

Tax will only be an option if the customer is syncing at an entity with tax groups set up in Sage Intacct OR if they are a multi-entity set up with tax groups set up, syncing at the top level.

https://docs.google.com/document/d/1k3ZFw8KB55yPUSCG6KYZlwpwEtmRt3eUshwxs7bZq5I/edit#bookmark=id.myrsjl26u0g7

How do I know that the entity has a tax group set up? Maybe we should put this in connections.intacct.data.entities?

Do you know answers to these questions? @JmillsExpensify @yuwenmemon @NikkiWines

[NikkiWines]

SyncPolicyToSageIntacct is working, but it's not sending sync progress, connectionSyncProgress is not updated by BE. The same happens when we connect to Sage Intacct for the first time. Is there any estimate on when this will be ready?

@yuwenmemon can you check this out since you added some sync progress logic here?

What about an endpoint for connecting by reusing other policy's credentials? Or maybe we're able to reuse some existing endpoint?

I think this is more of a polish than a core requirement, so let's make this a follow-up issue to complete. What do you think @yuwenmemon?

What about error handling in case when user gives wrong credentials? Right now any errors that happen with Sage Intacct sync are put into policy.connections object, but when user gives wrong credentials we're not going to connect him to Sage Intacct and therefore there will be no policy.connections object and no error. Related [slack thread].(https://swmansion.slack.com/archives/C06ML6X0W9L/p1720179157722519?thread_ts=1719271507.184899&cid=C06ML6X0W9L)

It doesn't appear like we currently store anything related to the error when the sync fails - we just display the error on Expensify Classic and clear out the connection data. I can update this so we store the error in the policy.connections object

How do I know that the entity has a tax group set up? Maybe we should put this in connections.intacct.data.entities?

I think the best way to confirm this is to check that data.entities is not empty and also that config.tax.syncTax is present in the connection data.

[SzymczakJ]

What about error handling in case when user gives wrong credentials? Right now any errors that happen with Sage Intacct sync are put into policy.connections object, but when user gives wrong credentials we're not going to connect him to Sage Intacct and therefore there will be no policy.connections object and no error. Related [slack thread].(https://swmansion.slack.com/archives/C06ML6X0W9L/p1720179157722519?thread_ts=1719271507.184899&cid=C06ML6X0W9L)

It doesn't appear like we currently store anything related to the error when the sync fails - we just display the error on Expensify Classic and clear out the connection data. I can update this so we store the error in the policy.connections object

I ask because we clearly have such pattern in Sage Intacct design doc: 1. User wants to connect to Sage Intacct but gives wrong credentials. 2. We indicate that and allow user to Enter credentials again(to achieve these two things we need connections data)

[miljakljajic]

If no payment is needed I'll unassign myself, thank you! Add me back if you need anything from BZ team member.

[war-in]

Hey 👋 The blocking PR has been merged and we are ready to start working on this task 🎉

But there is still one thing to discuss. Are there any designs for the wrong credentials error handling?
In QBO and Xero we show the Enter credentials option in the popup but here (and in NetSuite) we could also have an existing connection which we might want to reuse.

@yuwenmemon Could you please help me with getting those designs? 🙏

[yuwenmemon]

@war-in Is the question whether we'd want to show the "Reuse Existing Connection" option in addition to "Enter credentials"?

I'd say No - let's just show Enter credentials and "Disconnect". If one connection is broken, most likely all of them are broken, so reusing existing credentials wouldn't really help in that case.

[war-in]

Thanks for the answer! It'll be handled by @SzymczakJ when he is back in the office on Monday 🤞

[SzymczakJ]

I'm back and I'm starting to work on this one!

[SzymczakJ]

I've put out the draft PR, but it's gonna work only if we fix one thing on BE.
When giving wrong credentials to Sage Intacct we get a connections object with lasSync that looks like that:

we have an errorMessage that says "Authentication error" and isAuthenticationError set to false. Shouldn't the isAuthenticationError be set to true? When we set it to true we can handle this enter credentials flow the same way we do it in Xero and other integrations. @yuwenmemon

[SzymczakJ]

Another question: right now all error handling is handled like that:


which is not coherent with designs of Sage Intacct

I know that there is a design doc about error handling in integrations and that's why I'm wondering, if I should go with the Sage Intacct design doc version of error, or do it the same every other integration does this. Which version is the correct one?

[yuwenmemon]

@SzymczakJ I can make sure isAuthenticationError is set correctly for you!

For the error message, let's go with the Design Doc version, please. If anything, the other integrations should look more like the Intacct error, but we don't have to worry about fixing that just yet if it's too cumbersome.

[SzymczakJ]

Making error match Design Doc version shouldn't be too hard, I'll change it in this PR.

[SzymczakJ]

Is BE change already deployed @yuwenmemon?

[yuwenmemon]

Yep! Sorry it just went out yesterday

[yuwenmemon]

Let me know if you run into any issues

[SzymczakJ]

Works as expected, I'll make PR ready for review as a first thing on Monday morning.

[hungvu193]

Any update with the PR @SzymczakJ 😄

[SzymczakJ]

Sorry, I got ravaged by some other task that came up to be much more troublesome than I thought. I'll work on it at night 😬

[SzymczakJ]

Ready for review @hungvu193!

[melvin-bot]

This issue has not been updated in over 15 days. @yuwenmemon, @dannymcclain, @hungvu193, @war-in, @SzymczakJ eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[hungvu193]

The PR reached production a while ago, so I think we can close this issue.

@yuwenmemon Can you please help me create a payment issue for this project? Thank you 😄