[$250] Netsuite - Able to connect Netsuite with incorrect tokens

[lanitochka17]

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

---

Version Number: 9.0.20-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4859063&group_by=cases:section_id&group_order=asc&group_id=314239
Email or phone of affected tester (no customers): applausetester+gm1094@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

Precondition:

1. Login with an account that is an admin of a control workspace (or create one in OD if needed)
2. Enable accounting in workspace settings
   Steps
3. Navigate to Accounting
4. Click on the connect button next to NetSuite
5. Reach the credentials step
6. Enter incorrect the NetSuite tokens

Expected Result:

When credentials are entered incorrectly a sync error should be displayed

Actual Result:

User is able to connect Netsuite with incorrect tokens. No error message is displayed

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6572733_1723709133681.Recording__3717.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f1ac27b5bef67232
• Upwork Job ID: 1831077669368516851
• Last Price Increase: 2024-09-03
• Automatic offers:
• shubham1206agra | Contributor | 103803673

Issue Owner

Current Issue Owner: @rushatgabhane

[melvin-bot]

Triggered auto assignment to @sakluger (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.

[lanitochka17]

@sakluger FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #wave-control

[sakluger]

Hey @yuwenmemon, any idea why we're not throwing a connection error here?

[yuwenmemon]

Weird. Will take a look.

[sakluger]

@yuwenmemon any ideas?

[yuwenmemon]

No update yet, other issues have been taking precedence but will look soon!

[yuwenmemon]

So... we never really built a way to display error messages from NetSuite other than the generic sync error message (cc @mananjadhav @shubham1206agra) - we kind of kicked the can down the road on that one. But I'm thinking now might be a good time to get this going and we can use the traditional lastSync object that all the other integrations use.

If anything, we can do this just for the error message for now, and then maybe move NetSuite over to using it just like the rest of the integrations, at least for NewDot.

[mananjadhav]

Agreed. We started this discussion here but didn't prioritize. One of the recommendations was lastSync.wrongCredentials = true, so that we can differentiate between a general error or auth error. We could also probably change it to enum and handle it accordingly. wdyt?

[yuwenmemon]

@mananjadhav We actually have an isAuthentiationError property now on the lastSync object that can help with that too. I can make sure we start adding it for NetSuite.

[mananjadhav]

@mananjadhav We actually have an isAuthentiationError property now on the lastSync object that can help with that too. I can make sure we start adding it for NetSuite.

Yeah that would help. I am assuming isAuthenticationError is correct property name.

[yuwenmemon]

Yes 😄

[yuwenmemon]

Okay I've put in a PR above to update the NetSuite connection to start using the lastSync object like all the other integrations. We can flip this to be external once that's deployed. @mananjadhav @SzymczakJ or @war-in Would any of you be interested in working on that?

[mananjadhav]

I can pick this.

[shubham1206agra]

I am here for review if needed.

[melvin-bot]

@yuwenmemon @sakluger this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[sakluger]

@yuwenmemon - does your PR use the same methods that are being used in the QBO Export & Error Handling project?

[melvin-bot]

📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[yuwenmemon]

Apologies for the melvin madness @rushatgabhane @shubham1206agra @mananjadhav @sakluger

As per the conversation above, @mananjadhav will take on the implementation here and @shubham1206agra will do the review.

[mananjadhav]

Thanks for assigning. I'll have the PR Draft ready by tomorrow. @yuwenmemon just confirming once again based on the isAuthenticationError property is set, we'll show an invalid auth message, right? do we have any text/copy that we have finalized/used?

[yuwenmemon]

Let's follow the model used by Intacct. An error message will be stored in the lastSync object itself as well:

[mananjadhav]

Okay thanks for confirming.

[shubham1206agra]

@mananjadhav Bump here for PR

[mananjadhav]

Working on it, will be ready by tomorrow. I wasn't available on the weekend.

[greg-schroeder]

Thanks for the update @mananjadhav

[mananjadhav]

SageIntacct adds an error message and a Learn more link which leads to the help.expensify page. For NetSuite should we use this link https://help.expensify.com/articles/expensify-classic/connections/netsuite/Netsuite-Troubleshooting#expensierror-ns0109-failed-to-login-to-netsuite-please-verify-your-credentials ?

[yuwenmemon]

Yeah that would be a nice addition @mananjadhav

[melvin-bot]

This issue has not been updated in over 15 days. @mananjadhav, @yuwenmemon, @sakluger, @shubham1206agra 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!

[sakluger]

Summarizing payment on this issue:

Contributor: @mananjadhav $250, please request on Newdot
Contributor+: @shubham1206agra $250, sent offer via Upwork (https://www.upwork.com/nx/wm/offer/104323832)

@shubham1206agra I had to send a new offer - please let me know once you've accepted it. Thanks!

[shubham1206agra]

@sakluger Accepted offer

[garrettmknight]

$250 approved for @mananjadhav

[sakluger]

Thanks everyone! I completed payment to @shubham1206agra so we're good to close this one out.