-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HOLD for payment 2024-11-26] [$250] New Feature: "When to export" selector for auto-sync for NetSuite #51512
Comments
Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif ( |
Triggered auto assignment to @mallenexpensify ( |
|
Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify ( |
Removing |
ProposalPlease 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 :
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: 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. 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 We will add translate and spanish traslations too.
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. What alternative solutions did you explore? (Optional) |
@twilight2294 thanks for the proposal. Can you please share demo video? |
@aimane-chnaif here's the demo video: Screen.Recording.2024-10-29.at.12.19.07.PM.mov |
@twilight2294's proposal looks good. |
Current assignee @yuwenmemon is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
|
All new connections will be created with the |
Ah ok. Thanks @yuwenmemon for clarifying. |
Yes correct. |
|
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: If no regressions arise, payment will be issued on 2024-11-26. 🎊 For reference, here are some details about the assignees on this issue:
|
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:
|
Payment Summary
BugZero Checklist (@mallenexpensify)
|
@rojiphil @twilight2294 can you please accept the job and reply here once you have? |
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:
***@***.***>
|
Contributor: @twilight2294 paid $250 via Upwork |
@mallenexpensify Accepted the offer. Thanks. |
@rojiphil paid, payment post updated above.
@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. |
Meanwhile, here is the test case to this issue for addition: Precondition: Netsuite Accounting must be setup in a workspace
|
Test Case, thx @rojiphil
@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. |
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. |
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)
Please use the constants defined in Expensify Common though: Expensify/expensify-common#817
The selection should be saved under the property
accountingMethod
in the NetSuiteoptions.config
object.If there is no
accountingMethod
property set in the NetSuite config, the selector should default toCASH
Linked Expensify Issue: https://github.com/Expensify/Expensify/issues/422402
Issue Owner
Current Issue Owner: @Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @mallenexpensifyThe text was updated successfully, but these errors were encountered: