[HOLD for payment 2024-08-29] [$250] NetSuite - Sync error isn't displayed when connecting to NetSuite with invalid credentials

[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.16-3
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/4807616&group_by=cases:section_id&group_id=314239&group_order=asc
Issue reported by: Applause - Internal Team

Action Performed:

1. Open the app
2. Log in with a new Gmail account
3. Click on FAB - New workspace
4. Enable "Accounting" in the "More features" page.
5. Navigate to "Accounting"
6. Connect to NetSuite with invalid credentials

Expected Result:

Sync error should be displayed

Actual Result:

Sync error isn't displayed when connecting to NetSuite with invalid credentials

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

Bug6561075_1722694354704.JLKK3142.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ceab16370193c3b8
• Upwork Job ID: 1820591512049375650
• Last Price Increase: 2024-08-05

Issue Owner

Current Issue Owner: @VictoriaExpensify

[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.

[nyomanjyotisa]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Sync error isn't displayed when connecting to NetSuite with invalid credentials

What is the root cause of that problem?

We don't return true on hasSynchronizationError function if the net suite connection verified value is false, since verified is an indicator of whether the sync connection has been successful

App/src/libs/actions/connections/index.ts

Lines 366 to 372 in d0ecede

	function hasSynchronizationError(policy: OnyxEntry<Policy>, connectionName: PolicyConnectionName, isSyncInProgress: boolean): boolean {
	// NetSuite does not use the conventional lastSync object, so we need to check for lastErrorSyncDate
	if (connectionName === CONST.POLICY.CONNECTIONS.NAME.NETSUITE) {
	return !isSyncInProgress && !!policy?.connections?.[CONST.POLICY.CONNECTIONS.NAME.NETSUITE].lastErrorSyncDate;
	}
	return !isSyncInProgress && policy?.connections?.[connectionName]?.lastSync?.isSuccessful === false;
	}

App/src/types/onyx/Policy.ts

Lines 971 to 972 in d0ecede

	/** Whether the sync connection has been successful */
	verified: boolean;

What changes do you think we should make in order to solve the problem?

Update this code to the following to return true if the net suite connection verified value is false

    if (connectionName === CONST.POLICY.CONNECTIONS.NAME.NETSUITE) {
        return !isSyncInProgress && (!!policy?.connections?.[CONST.POLICY.CONNECTIONS.NAME.NETSUITE].lastErrorSyncDate || !policy?.connections?.[CONST.POLICY.CONNECTIONS.NAME.NETSUITE]?.verified);
    }

RESULT

New-Expensify.26.mp4

What alternative solutions did you explore? (Optional)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01ceab16370193c3b8

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk (External)

[sakluger]

Tagging @yuwenmemon, @mananjadhav, @shubham1206agra since you all worked on the design doc for this feature.

[shubham1206agra]

@sakluger This looks like a BE issue.

[brunovjk]

Indeed, we are not checking for when verified: false. I tested @nyomanjyotisa's proposal, and it seems to fix the issue. However, I'm wondering about the error message—should we update it to something specific about the credential error? @tgolen , can you confirm if it's only a backend issue? Thank you.

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @tgolen, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[tgolen]

Wouldn't the error message get updated here?

https://github.com/Expensify/App/blob/main/src/languages/en.ts#L3115

[brunovjk]

Exactly. should we update/add it to something specific about the invalid credentials?

[mananjadhav]

There was a discussion here but I cannot find what we concluded for the same.

[tgolen]

I think if we can be more specific about the error, then we should, yeah. It only helps the user and that makes them happier and then they like the product more. Though, I don't know that we need to hold up this proposal on changing the error message. I'm going to go ahead and assign this one with the current proposal.

[melvin-bot]

❌ There was an error making the offer to @brunovjk for the Reviewer role. The BZ member will need to manually hire the contributor.

[melvin-bot]

❌ There was an error making the offer to @nyomanjyotisa for the Contributor role. The BZ member will need to manually hire the contributor.

[tgolen]

woop, not sure what happened there. @sakluger can you take care of that, please?

[sakluger]

Sure thing! Here are the Upwork offers:

@brunovjk - https://www.upwork.com/nx/wm/offer/103437519
@nyomanjyotisa - https://www.upwork.com/nx/wm/offer/103437511

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.23-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Display sync error when connecting to NetSuite with invalid credentials #47059

If no regressions arise, payment will be issued on 2024-08-29. 🎊

For reference, here are some details about the assignees on this issue:

• @nyomanjyotisa requires payment (Needs manual offer from BZ)
• @brunovjk requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR:
• [@brunovjk] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@brunovjk] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@brunovjk] Determine if we should create a regression test for this bug.
• [@brunovjk] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[sakluger]

I'm going to be OOO until Aug 30, so I'll assign another BZ member who can manually create those Upwork offers.

[melvin-bot]

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

[brunovjk]

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: Not a regression, but an exception for NetSuite in error checking logic: commit
• [@brunovjk] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: Add handling of the NetSuite beta in NewDot #44064 (comment)
• [@brunovjk] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g., updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: No discussion needed as this was not a systemic issue but a missed check for verified === false.
• [@brunovjk] Determine if we should create a regression test for this bug. Yes
• [@brunovjk] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

• Click on FAB - New workspace
• Enable "Accounting" in the "More features" page
• Navigate to "Accounting"
• Connect to NetSuite with invalid credentials
• Verify sync error displayed
• Do we agree 👍 or 👎

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @nyomanjyotisa paid $(AMOUNT) via Upwork (LINK)
• ROLE: @brunovjk paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@VictoriaExpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1820591512049375650/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[tgolen]

@VictoriaExpensify Can you please complete the checklist here to close this one out?

[VictoriaExpensify]

Created an issue to add the regression test to TestRail - https://github.com/Expensify/Expensify/issues/425122

[VictoriaExpensify]

Payment summary:

Contributor: @nyomanjyotisa paid $250 via Upwork Job
Contributor+: @brunovjk paid $250 via Upwork Job

Thanks for your work on this issue!