Fix: Allow updating NetSuite tokens without resetting connection configuration#83434
Conversation
…h error When users update their NetSuite token credentials due to an authentication error, the app previously called ConnectPolicyToNetSuite which triggers initConnection and resets all existing configuration. This change adds a new UpdateNetSuiteTokens command that preserves existing config and only updates the credentials, then conditionally calls it when isAuthenticationError is true on the NetSuite connection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@yuwenmemon 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] |
| API.write(WRITE_COMMANDS.CONNECT_POLICY_TO_NETSUITE, parameters, {optimisticData}); | ||
| } | ||
|
|
||
| function updateNetSuiteTokens(policyID: string, credentials: Omit<ConnectPolicyToNetSuiteParams, 'policyID'>) { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The updateNetSuiteTokens function body is an exact duplicate of connectPolicyToNetSuite (lines 21-37) -- identical signature, identical optimistic data construction, identical parameter assembly. The only difference is the write command (UPDATE_NETSUITE_TOKENS vs CONNECT_POLICY_TO_NETSUITE).
Extract the shared logic into a helper that accepts the command as a parameter:
function writeNetSuiteCredentials(
command: typeof WRITE_COMMANDS.CONNECT_POLICY_TO_NETSUITE | typeof WRITE_COMMANDS.UPDATE_NETSUITE_TOKENS,
policyID: string,
credentials: Omit<ConnectPolicyToNetSuiteParams, 'policyID'>,
) {
const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.POLICY_CONNECTION_SYNC_PROGRESS>> = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.POLICY_CONNECTION_SYNC_PROGRESS}${policyID}`,
value: {
stageInProgress: CONST.POLICY.CONNECTIONS.SYNC_STAGE_NAME.NETSUITE_SYNC_CONNECTION,
connectionName: CONST.POLICY.CONNECTIONS.NAME.NETSUITE,
timestamp: new Date().toISOString(),
},
},
];
const parameters: ConnectPolicyToNetSuiteParams = {
policyID,
...credentials,
};
API.write(command, parameters, {optimisticData});
}
function connectPolicyToNetSuite(policyID: string, credentials: Omit<ConnectPolicyToNetSuiteParams, 'policyID'>) {
writeNetSuiteCredentials(WRITE_COMMANDS.CONNECT_POLICY_TO_NETSUITE, policyID, credentials);
}
function updateNetSuiteTokens(policyID: string, credentials: Omit<ConnectPolicyToNetSuiteParams, 'policyID'>) {
writeNetSuiteCredentials(WRITE_COMMANDS.UPDATE_NETSUITE_TOKENS, policyID, credentials);
}Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
@imgyf Could you explain about it, such as why you 👎?
There was a problem hiding this comment.
@truph01 after looking through the comment again, it's actually valid so I've fixed it in the latest commit.
|
Would you mind adding a video of the flow for desktop web? I mostly just want to see the placement of the option in the menu. |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Product review not required on this one.
|
Hi @dannymcclain , here's the video as requested. 83434-video.mov |
|
Nice! Thank you 👍 |
|
@imgyf Please update your PR author checklist to include videos, not only screenshots. |
|
@truph01 I have updated the PR author checklist to include videos for all 5 platforms, please review. |
|
@imgyf Could you merge main here? |
|
@truph01 merged main as requested. |
|
@truph01 not sure why PR Author Checklist check is failing when I already have all checkboxes checked? |
|
@truph01 please review again. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-03-10.at.12.09.28.movAndroid: mWeb ChromeScreen.Recording.2026-03-10.at.12.08.44.moviOS: HybridAppScreen.Recording.2026-03-10.at.12.02.33.moviOS: mWeb SafariScreen.Recording.2026-03-10.at.12.05.08.movMacOS: Chrome / SafariScreen.Recording.2026-03-10.at.11.47.57.mov |
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🚧 @yuwenmemon has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/yuwenmemon in version: 9.3.37-0 🚀
|
|
Deploy Blocker #85196 was identified to be related to this PR. |
|
@jponikarchuk do you want me to revert the PR first? |
|
@imgyf @yuwenmemon @truph01 Can this one please be tested internally, not able to |
|
@yuwenmemon any way to test this internally? |
Regression NoteThis PR caused a deploy blocker: #85196 What went wrong: The conditional in Fix: PR #85200 added an Takeaway: When branching on connection error state, always consider the connection lifecycle stage (unverified vs verified), an error flag alone doesn't tell you whether the connection was ever successfully initialized. |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.37-10 🚀
|
Explanation of Change
When a user needs to update their NetSuite token credentials (due to an authentication error, token rotation, etc.), the app previously called
ConnectPolicyToNetSuite, which triggersinitConnectionon the backend and resets all user configuration (subsidiary, export destinations, mappings, custom segments, etc.).This PR adds support for the backend's
UpdateNetSuiteTokenscommand, which validates and updates credentials then triggers a sync — all while preserving existing config.Changes:
src/libs/API/types.ts— AddedUPDATE_NETSUITE_TOKENSwrite command mapped to existingConnectPolicyToNetSuiteParamssrc/libs/actions/connections/NetSuiteCommands.ts— AddedupdateNetSuiteTokens()action mirroringconnectPolicyToNetSuite()but dispatching the new commandsrc/pages/.../NetSuiteTokenInputForm.tsx— Updated form submission to callupdateNetSuiteTokenswhenisAuthenticationErroris true, otherwiseconnectPolicyToNetSuiteas beforeFixed Issues
$ #82339
PROPOSAL: #82339 (comment)
Tests
isAuthenticationError: trueon the connection'slastSyncin OnyxUpdateNetSuiteTokens(notConnectPolicyToNetSuite)Fresh connection scenario:
ConnectPolicyToNetSuiteas beforeFailure scenarios:
Offline tests
UpdateNetSuiteTokensrequest is sentQA Steps
ConnectPolicyToNetSuitePR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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
83434-android-native-vid.mov
Android: mWeb Chrome
83434-android-mweb-vid.mov
iOS: Native
83434-ios-native-vid.mov
iOS: mWeb Safari
83434-ios-mweb-vid.mov
MacOS: Chrome / Safari
83434-macos-safari-vid.mov