[HOLD for payment 2024-11-26] [$250] New Feature: "When to export" selector for auto-sync for NetSuite

[yuwenmemon]

Hello! We need to build the following selector in the NetSuite configuration options:

The "When to export" selector in the above mock-up is what we're adding.

The two options should have the following values (copied from the same feature in OldDot)

const accountingMethodOptions = [
    {
        value: 'ACCRUAL',
        text: 'Accrual',
        description: 'Out of pocket expenses will export when final approved',
    },
    {
        value: 'CASH',
        text: 'Cash',
        description: 'Out of pocket expenses will export when paid',
    },
];

Please use the constants defined in Expensify Common though: Expensify/expensify-common#817

The selection should be saved under the property accountingMethod in the NetSuite options.config object.

If there is no accountingMethod property set in the NetSuite config, the selector should default to CASH

Linked Expensify Issue: https://github.com/Expensify/Expensify/issues/422402

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021852432563378960977
• Upwork Job ID: 1852432563378960977
• Last Price Increase: 2024-11-01

Issue Owner

Current Issue Owner: @mallenexpensify

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @mallenexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[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 - @dubielzyk-expensify (NewFeature)

[yuwenmemon]

Removing NewFeature label since this does not fall under a project, and design has already reviewed.

[twilight2294]

Proposal

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

"When to export" selector for auto-sync for NetSuite

What is the root cause of that problem?

Feature request

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

We need to update NetSuiteAdvancedPage :

1. Update the toggle with a menuitem and navigate to new Auto-sync page:

    const menuItems: Array<MenuItemWithSubscribedSettings | ToggleItem | DividerLineItem> = [
        {
            type: 'menuitem',
            title: autoSyncConfig?.autoSync.enabled ? "Enabled" : "Disabled",
            description: translate('workspace.accounting.autoSync'),
            onPress: () => Navigation.navigate(ROUTES.POLICY_ACCOUNTING_NETSUITE_AUTO_SYNC.getRoute(policyID)),
            hintText: (() => {
                if (!autoSyncConfig?.autoSync.enabled) return undefined;
                return accountingMehtod === CONST.NETSUITE_ACCOUNTING_METHODS.CASH
                    ? 'Out of pocket expenses will export when paid'
                    : 'Out of pocket expenses will export when final approved';
            })(),  
        },

The above text will be converted to spanish as well and will be displayed using translate.

We would need to create a new route and a new screen for the AutoSync page:
Routes:

    POLICY_ACCOUNTING_NETSUITE_AUTO_SYNC: {
        route: 'settings/workspaces/:policyID/connections/netsuite/advanced/autosync',
        getRoute: (policyID: string) => `settings/workspaces/${policyID}/connections/netsuite/advanced/autosync` as const,
    },
    POLICY_ACCOUNTING_NETSUITE_ACCOUNTING_METHOD: {
        route: 'settings/workspaces/:policyID/connections/netsuite/advanced/autosync/accounting-method',
        getRoute: (policyID: string) => `settings/workspaces/${policyID}/connections/netsuite/advanced/autosync/accounting-method` as const,
    },

SCREENS:

            NETSUITE_AUTO_SYNC: 'Policy_Accounting_NetSuite_Auto_Sync',
            NETSUITE_ACCOUNTING_METHOD: 'Policy_Accounting_NetSuite_Accounting_Methog',

We also need to update linking config and modal stack navigator to attach the new pages to the routes.
2. Create a new Page to display the Auto-Sync options:

function NetSuiteAutoSyncPage({policy, route}: NetSuiteAutoSyncPage) {
    const styles = useThemeStyles();
    const {translate} = useLocalize();
    const autoSyncConfig = policy?.connections?.netsuite?.config;
    const policyID = route.params.policyID ?? '-1';
    const backTo = route.params.backTo;
    const isQuickSettingsFlow = backTo;
    return (
        <AccessOrNotFoundWrapper
            policyID={policyID}
            accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
            featureName={CONST.POLICY.MORE_FEATURES.ARE_CATEGORIES_ENABLED}
        >
            <ScreenWrapper
                includeSafeAreaPaddingBottom={false}
                style={[styles.defaultModalContainer]}
                testID={NetSuiteAutoSyncPage.displayName}
            >
                <HeaderWithBackButton
                    title={translate('common.settings')}
                    onBackButtonPress={() => Navigation.goBack(isQuickSettingsFlow ? ROUTES.SETTINGS_CATEGORIES_ROOT.getRoute(policyID, backTo) : undefined)}
                />
                <ToggleSettingOptionRow
                    title={translate('workspace.accounting.autoSync')}
                    // wil be converted to translate and spanish translation too
                    subtitle={'Sync NetSuite and Expensify automatically, every day. Export finalized report in realtime'}
                    isActive={!!autoSyncConfig?.autoSync.enabled}
                    wrapperStyle={[styles.pv2, styles.mh5]}
                    switchAccessibilityLabel={translate('workspace.netsuite.advancedConfig.autoSyncDescription')}
                    shouldPlaceSubtitleBelowSwitch
                    onCloseError={() => Policy.clearNetSuiteAutoSyncErrorField(policyID)}
                    onToggle={(isEnabled) => Connections.updateNetSuiteAutoSync(policyID, isEnabled)}
                    pendingAction={settingsPendingAction([CONST.NETSUITE_CONFIG.AUTO_SYNC], autoSyncConfig?.pendingFields)}
                    errors={ErrorUtils.getLatestErrorField(autoSyncConfig, CONST.NETSUITE_CONFIG.AUTO_SYNC)}
                />
                {!!autoSyncConfig?.autoSync.enabled && (
                    <MenuItemWithTopDescription
                    title={autoSyncConfig.accountingMethod === CONST.NETSUITE_ACCOUNTING_METHODS.ACCRUAL ? 'Accural' : 'Cash'}
                    description='When to export'
                    shouldShowRightIcon
                    onPress={() => Navigation.navigate(ROUTES.POLICY_ACCOUNTING_NETSUITE_ACCOUNTING_METHOD.getRoute(policyID))}
                                        />
                )}

            </ScreenWrapper>
        </AccessOrNotFoundWrapper>
    );
}

NetSuiteAutoSyncPage.displayName = 'NetSuiteAutoSyncPage';

export default withPolicyConnections(NetSuiteAutoSyncPage);

We will always default to CASH. Also whenever we enable the toggle either initial value should be blank or should be set to cash that can be decided during PR phase.

We will add translate and spanish traslations too.

3. Create a accounting method Selecting page:

function NetSuiteAccountingMethodPage({policy}: NetSuiteAccountingMethodPage) {
    const {translate} = useLocalize();
    const styles = useThemeStyles();

    const autoSyncConfig = policy?.connections?.netsuite?.config;
    const [typeSelected, setTypeSelected] = useState(autoSyncConfig?.accountingMethod ?? CONST.NETSUITE_ACCOUNTING_METHODS.ACCRUAL);
    const policyID = policy?.id ?? '0';


    const data = useMemo(() => {
        const accountingMethodOptions = [
            {
                value: 'ACCRUAL',
                text: 'Accrual',
                alternateText: 'Out of pocket expenses will export when final approved',
                keyForList: CONST.NETSUITE_ACCOUNTING_METHODS.ACCRUAL,
                isSelected: typeSelected === CONST.NETSUITE_ACCOUNTING_METHODS.ACCRUAL,

            },
            {
                value: 'CASH',
                text: 'Cash',
                alternateText: 'Out of pocket expenses will export when paid',
                keyForList: CONST.NETSUITE_ACCOUNTING_METHODS.CASH,
                isSelected: typeSelected === CONST.NETSUITE_ACCOUNTING_METHODS.CASH,
            },

        ];

        return accountingMethodOptions;
    }, [translate]);

    const updateAccountingMethod = useCallback(
        (value) => {
          
                Connections.updateNetSuiteReimbursementAccountID(policyID, value, autoSyncConfig?.accountingMethod);
 
            Navigation.goBack(ROUTES.POLICY_ACCOUNTING_NETSUITE_ADVANCED.getRoute(policyID));
        },
        [policyID, autoSyncConfig?.reimbursementAccountID],
    );

    return (
        <ScreenWrapper
            testID={NetSuiteAccountingMethodPage.displayName}
            includeSafeAreaPaddingBottom={false}
            shouldEnablePickerAvoiding={false}
            shouldEnableMaxHeight
        >
            <HeaderWithBackButton
                title={translate('workspace.card.issueCard')}
            />
            <Text style={[ styles.ph5, styles.mv3]}>{'Choose when to export the expenses:'}</Text>
            <SelectionList
                ListItem={RadioListItem}
                onSelectRow={({value}) => {
                    setTypeSelected(value)
    
                        Connections.updateNetSuiteReimbursementAccountID(policyID, value, config?.reimbursementAccountID);

                    Navigation.goBack(ROUTES.POLICY_ACCOUNTING_NETSUITE_AUTO_SYNC.getRoute(policyID));


                }}
                sections={[{data}]}
                shouldSingleExecuteRowSelect
                initiallyFocusedOptionKey={typeSelected}
                shouldUpdateFocusedIndex
                isAlternateTextMultilineSupported
            />
        </ScreenWrapper>
    );
}

Here too, we would need spanish translations which will be done in the PR phase, we also need BE changes to make a new API endpoint which will update the value of AccountingMethod. updateNetSuiteReimbursementAccountID will be implemented on the frontend to set the data optimistically and also call the API

What alternative solutions did you explore? (Optional)

[aimane-chnaif]

@twilight2294 thanks for the proposal. Can you please share demo video?

[twilight2294]

@aimane-chnaif here's the demo video:

Screen.Recording.2024-10-29.at.12.19.07.PM.mov

[aimane-chnaif]

@twilight2294's proposal looks good.
Please make sure to follow instructions in OP.
🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @yuwenmemon is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

[yuwenmemon]

In this case for the first time, we should update the accountingMethod in options.config to CASH right, I can do that in the FE but for the Backend we need to set a default value for the first time toggle right ?

All new connections will be created with the accountingMethod already set to ACCRUAL so the default value should just follow what is set in the connection object. if there is no accountingMethod, then go with CASH as the default.

[rojiphil]

All new connections will be created with the accountingMethod already set to ACCRUAL so the default value should just follow what is set in the connection object. if there is no accountingMethod, then go with CASH as the default.

Ah ok. Thanks @yuwenmemon for clarifying.
Also, is the understanding correct that enabling or disabling the auto sync should not impact the configured accounting method i.e the method would remain the same?

[yuwenmemon]

Yes correct.

[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.63-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• New Feature: "When to export" selector for auto-sync for NetSuite #51949

If no regressions arise, payment will be issued on 2024-11-26. 🎊

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

• @rojiphil requires payment (Needs manual offer from BZ)
• @twilight2294 requires payment (Needs manual offer from BZ)

[melvin-bot]

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

• [@rojiphil] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @rojiphil paid $(AMOUNT) via Upwork (LINK)
• ROLE: @twilight2294 paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@mallenexpensify)

• 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/1852432563378960977/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[mallenexpensify]

@rojiphil @twilight2294 can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~021852432563378960977

[twilight2294]

Accepted the job offer

…

On Wed, Nov 27, 2024 at 5:56 AM Matt Allen ***@***.***> wrote: @rojiphil <https://github.com/rojiphil> @twilight2294 <https://github.com/twilight2294> can you please accept the job and reply here once you have? https://www.upwork.com/jobs/~021852432563378960977 — Reply to this email directly, view it on GitHub <#51512 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BLXFUO3CFEBIFXRZMZACHOT2CUGUVAVCNFSM6AAAAABQUEIJPCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBSGMZTIOJTGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mallenexpensify]

Contributor: @twilight2294 paid $250 via Upwork
Contributor+: @rojiphil paid $250 via Upwork

[rojiphil]

can you please accept the job and reply here once you have?

@mallenexpensify Accepted the offer. Thanks.

[mallenexpensify]

@rojiphil paid, payment post updated above.

[@rojiphil] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

@rojiphil , do we want a test case for this new feature? If not, why not? Thx

[rojiphil]

do we want a test case for this new feature? If not, why not? Thx

@mallenexpensify Thanks for the payment. Regarding new features, I think it would be good if the QA process implicitly involved mandatorily including the test case directly from the PR in their database. Maybe a checklist is also needed for QA.
The reasoning is that, for C+, the existing BZ checklist is designed for bugs/blockers and not for new features, while adding test cases for new features is a mandatory step for QA. What do you think?

[rojiphil]

Meanwhile, here is the test case to this issue for addition:

Precondition: Netsuite Accounting must be setup in a workspace

1. Navigate to Workspace —> Accounting
2. Click on Advanced
3. Enable Auto-sync toggle if disabled
4. Verify that a new field When to export is displayed
5. Select the field When to export
6. Verify that two options Cash and Accrual are displayed.
7. Select an unselected option.
8. Verify that we are navigated back to Autosync page and the new selected option is displayed.
9. Also verify the hint text i.e. if we select CASH as option then the hint text below auto-sync option should be: Out-of-pocket expenses will export when paid and if we select ACCURAL as option then the hind text should be: Out-of-pocket expenses will export when final approved

[mallenexpensify]

Test Case, thx @rojiphil

• https://github.com/Expensify/Expensify/issues/449285

QA process implicitly involved mandatorily including the test case directly from the PR in their database

@rojiphil can you elaborate on this? Are you saying that, for every new feature, QA should do something for all of the PRs to create new test cases without the assigned internal engineer, C+ or contributor doing anything? I do find it interesting how most test cases for bugs are just the test steps from the OP of the PR.

[rojiphil]

can you elaborate on this? Are you saying that, for every new feature, QA should do something for all of the PRs to create new test cases without the assigned internal engineer, C+ or contributor doing anything? I do find it interesting how most test cases for bugs are just the test steps from the OP of the PR.

@mallenexpensify Yeah. That’s correct. As I understand, the QA should have a database of all test cases. As such, any new feature/improvement should be added to the test case database. And I believe it is the QA team’s responsibility to write and maintain this. Not frequently though but once in a while, all these test cases can be run to ensure that the software functions as expected. Regression test cases, to me, are a subset of test cases from the database of all test cases that are required to be tested by QA to check for feature regressions. Regression tests can be run whenever code is changed for a certain feature to ensure that all is well within the feature.

[mallenexpensify]

Thx @rojiphil. For New Features, it's the responsibility of the people creating or working on the feature to propose the updated test steps then QA will add them, if needed. If the new feature comes from a design doc, an author of the doc might choose to write all test steps at once so that they're cohesive. For small features, it's likely default to have C+ create the test steps unless it's been clearly stated not to, or that someone else will add them. This is to ensure they get added. We'd rather have multiple GHs for adding new test steps and have QA close dupes or unneeded ones vs missing a test case.