Skip to content
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.

Sign up for GitHub

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

feat: Single credit line for Journal Entry #1017

Merged
merged 5 commits into from
Oct 16, 2024
Merged

feat: Single credit line for Journal Entry #1017

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9d25adf
feat: Single credit line for Journal Entry
ashwin1111 Oct 11, 2024
35887c9
Merge branch 'master' into je-single-credit-ns
ashwin1111 Oct 11, 2024
7a7acbe
new org case
ashwin1111 Oct 14, 2024
08a2bdb
fix new case
ashwin1111 Oct 14, 2024
a7d3989
Merge branch 'master' into je-single-credit-ns
ashwin1111 Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/app/branding/c1-contents-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ export const c1Contents = {
preferenceLabel: 'Other preferences',
preferenceSubLabel: 'Create new records in NetSuite if no vendor record is found or the accounting period is closed.',
previewDescriptionFieldLabel: 'Preview of the description field',
autoCreateMerchantsLabel: 'Auto create merchant on NetSuite for credit card charge'
autoCreateMerchantsLabel: 'Auto create merchant on NetSuite for credit card charge',
singleCreditLineJELabel: 'Create a single itemized offset credit entry for Journal',
singleCreditLineJESubLabel: 'Merge all Credits in a Journal to create a single entry.'
}
}
},
Expand Down
4 changes: 3 additions & 1 deletion src/app/branding/fyle-contents-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ export const fyleContents = {
preferenceLabel: 'Other Preferences',
preferenceSubLabel: 'Based on your preference, you can choose whether you want to create any new records in NetSuite from Fyle. (when there is no employee record found, or when the accounting period is closed).',
previewDescriptionFieldLabel: 'Preview of the Description Field',
autoCreateMerchantsLabel: 'Auto Create Merchant on NetSuite for Credit Card Charge'
autoCreateMerchantsLabel: 'Auto Create Merchant on NetSuite for Credit Card Charge',
singleCreditLineJELabel: 'Create a single itemized offset credit entry for Journal',
singleCreditLineJESubLabel: 'Merge all Credits in a Journal to create a single entry.'
Comment on lines +88 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding similar properties to Xero integration

The new properties added to the NetSuite integration (singleCreditLineJELabel and singleCreditLineJESubLabel) are also present in the QuickBooks Online integration. However, these properties are missing from the Xero integration. For consistency across integrations, consider:

  1. Adding these properties to the Xero integration if the functionality is applicable.
  2. If not applicable to Xero, document the reason for the difference to maintain clarity for future development.

To check the current state of the Xero integration, run the following script:

#!/bin/bash
# Description: Check Xero integration for similar properties

echo "Searching for single credit line properties in Xero integration:"
rg --type typescript 'xero.*singleCreditLineJE'

echo "Searching for auto-create merchants properties in Xero integration:"
rg --type typescript 'xero.*autoCreateMerchants'

}
}
},
Expand Down
2 changes: 2 additions & 0 deletions src/app/core/models/branding/content-configuration.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ export type ContentConfiguration = {
scheduleSubLabel: string;
previewDescriptionFieldLabel: string;
autoCreateMerchantsLabel: string;
singleCreditLineJELabel: string;
singleCreditLineJESubLabel: string;
}
},
},
Expand Down
7 changes: 5 additions & 2 deletions src/app/core/models/netsuite/netsuite-configuration/netsuite-advanced-settings.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export type NetsuiteAdvancedSettingConfiguration = {
auto_create_destination_entity: boolean,
auto_create_merchants: boolean,
change_accounting_period: boolean,
memo_structure: string[]
memo_structure: string[],
je_single_credit_line: boolean
}

export type NetsuiteAdvancedSettingGeneralMapping = {
Expand Down Expand Up @@ -138,6 +139,7 @@ export class NetsuiteAdvancedSettingModel extends HelperUtility {
useEmployeeClass: new FormControl(advancedSettings?.general_mappings.use_employee_class ? advancedSettings?.general_mappings.use_employee_class : false),
changeAccountingPeriod: new FormControl(advancedSettings?.configuration.change_accounting_period),
autoCreateVendors: new FormControl(advancedSettings?.configuration.auto_create_destination_entity),
singleCreditLineJE: new FormControl(advancedSettings?.configuration.je_single_credit_line),
exportSchedule: new FormControl(advancedSettings?.workspace_schedules?.enabled ? true : false),
exportScheduleFrequency: new FormControl(advancedSettings?.workspace_schedules?.enabled ? advancedSettings?.workspace_schedules.interval_hours : 1),
memoStructure: new FormControl(advancedSettings?.configuration.memo_structure),
Expand All @@ -160,7 +162,8 @@ export class NetsuiteAdvancedSettingModel extends HelperUtility {
auto_create_destination_entity: advancedSettingsForm.get('autoCreateVendors')?.value,
change_accounting_period: advancedSettingsForm.get('changeAccountingPeriod')?.value,
memo_structure: advancedSettingsForm.get('memoStructure')?.value,
auto_create_merchants: advancedSettingsForm.get('autoCreateMerchants')?.value
auto_create_merchants: advancedSettingsForm.get('autoCreateMerchants')?.value,
je_single_credit_line: advancedSettingsForm.get('singleCreditLineJE')?.value || false
},
general_mappings: {
vendor_payment_account: advancedSettingsForm.get('paymentAccount')?.value ? advancedSettingsForm.get('paymentAccount')?.value : emptyDestinationAttribute,
Expand Down
26 changes: 16 additions & 10 deletions ...uite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,24 +87,30 @@
[formControllerName]="'changeAccountingPeriod'">
</app-configuration-toggle-field>

<div class="autoCreateVendor" *ngIf="isAutoCreateVendorsFieldVisible() && brandingFeatureConfig.featureFlags.advancedSettings.autoCreateVendors" [ngClass]="{'tw-mb-24-px' : !isAutoCreateMerchantsFieldVisible()}">
<app-configuration-toggle-field
[form]="advancedSettingForm"
[iconPath]="'user-plus'"
[label]="brandingContent.autoCreateVendorsLabel + getCreateVendorLabel() "
[subLabel]="'While exporting reimbursable expenses from ' + brandingConfig.brandName + ', the integration will automatically create a ' + getCreateVendorLabel().toLowerCase() + ' if a match does not exist in NetSuite already.'"
[formControllerName]="'autoCreateVendors'">
</app-configuration-toggle-field>
</div>
<app-configuration-toggle-field *ngIf="isAutoCreateVendorsFieldVisible() && brandingFeatureConfig.featureFlags.advancedSettings.autoCreateVendors"
[form]="advancedSettingForm"
[iconPath]="'user-plus'"
[label]="brandingContent.autoCreateVendorsLabel + getCreateVendorLabel() "
[subLabel]="'While exporting reimbursable expenses from ' + brandingConfig.brandName + ', the integration will automatically create a ' + getCreateVendorLabel().toLowerCase() + ' if a match does not exist in NetSuite already.'"
[formControllerName]="'autoCreateVendors'">
</app-configuration-toggle-field>

<app-configuration-toggle-field *ngIf="isAutoCreateMerchantsFieldVisible() && brandingFeatureConfig.featureFlags.advancedSettings.autoCreateMerchants"
<app-configuration-toggle-field *ngIf="isAutoCreateMerchantsFieldVisible() && brandingFeatureConfig.featureFlags.advancedSettings.autoCreateMerchants"
[form]="advancedSettingForm"
[iconPath]="'user-plus'"
[label]="brandingContent.autoCreateVendorsLabel + getCreateMerchantLabel()"
[subLabel]="'While exporting credit card expenses from ' + brandingConfig.brandName + ', the integration will automatically create a merchant if a match does not exist in NetSuite already.'"
[formControllerName]="'autoCreateMerchants'">
</app-configuration-toggle-field>

<app-configuration-toggle-field class="tw-pt-0" *ngIf="isSingleCreditLineJEFieldVisible() && brandingFeatureConfig.featureFlags.advancedSettings.singleCreditLineJE"
[form]="advancedSettingForm"
[iconPath]="'list'"
[label]="brandingContent.singleCreditLineJELabel"
[subLabel]="brandingContent.singleCreditLineJESubLabel"
[formControllerName]="'singleCreditLineJE'">
</app-configuration-toggle-field>

</div>

<div class="tw-mb-16-px">
Expand Down
4 changes: 4 additions & 0 deletions ...tsuite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ export class NetsuiteAdvancedSettingsComponent implements OnInit {
return this.workspaceGeneralSettings.reimbursable_expenses_object && this.workspaceGeneralSettings.reimbursable_expenses_object !== NetsuiteReimbursableExpensesObject.JOURNAL_ENTRY;
}

isSingleCreditLineJEFieldVisible(): boolean {
return this.workspaceGeneralSettings.reimbursable_expenses_object === NetsuiteReimbursableExpensesObject.JOURNAL_ENTRY || this.workspaceGeneralSettings.corporate_credit_card_expenses_object === NetSuiteCorporateCreditCardExpensesObject.JOURNAL_ENTRY;
}

onMultiSelectChange() {
const memo = this.advancedSettingForm.controls.memoStructure.value;
const changedMemo = AdvancedSettingsModel.formatMemoPreview(memo, this.defaultMemoOptions)[1];
Expand Down
54 changes: 54 additions & 0 deletions ...s/netsuite/netsuite-shared/netsuite-export-settings/netsuite-export-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ export class NetsuiteExportSettingsComponent implements OnInit {
if (this.isOnboarding) {
this.workspaceService.setOnboardingState(NetsuiteOnboardingState.IMPORT_SETTINGS);
this.router.navigate([`/integrations/netsuite/onboarding/import_settings`]);
} else if (this.isAdvancedSettingAffected()) {
this.router.navigate(['/integrations/netsuite/main/configuration/advanced_settings']);
Comment on lines +209 to +210
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reconsider Navigation Logic After Saving Settings

In the constructPayloadAndSave method, when isAdvancedSettingAffected() returns true, the user is automatically redirected to the advanced settings page after saving. This abrupt navigation might confuse users who are not expecting it. Consider providing a prompt or notification to inform the user about the impact on advanced settings, allowing them to choose whether to navigate there.

}
}, () => {
this.isSaveInProgress = false;
Expand All @@ -218,7 +220,59 @@ export class NetsuiteExportSettingsComponent implements OnInit {
this.router.navigate([`/integrations/netsuite/onboarding/connector`]);
}

private isAdvancedSettingAffected(): boolean {
return (this.exportSettings?.configuration?.reimbursable_expenses_object !== NetsuiteReimbursableExpensesObject.JOURNAL_ENTRY && this.exportSettingForm.value.reimbursableExportType === NetsuiteReimbursableExpensesObject.JOURNAL_ENTRY) || (this.exportSettings?.configuration?.corporate_credit_card_expenses_object !== NetSuiteCorporateCreditCardExpensesObject.JOURNAL_ENTRY && this.exportSettingForm.value.creditCardExportType === NetSuiteCorporateCreditCardExpensesObject.JOURNAL_ENTRY);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider refactoring for improved readability.

The isAdvancedSettingAffected method correctly checks for changes that would affect advanced settings. However, the single-line return statement with multiple conditions might be hard to read and maintain.

Consider refactoring the method for improved readability:

private isAdvancedSettingAffected(): boolean {
  const isReimbursableChanged = 
    this.exportSettings?.configuration?.reimbursable_expenses_object !== NetsuiteReimbursableExpensesObject.JOURNAL_ENTRY &&
    this.exportSettingForm.value.reimbursableExportType === NetsuiteReimbursableExpensesObject.JOURNAL_ENTRY;

  const isCorporateCardChanged = 
    this.exportSettings?.configuration?.corporate_credit_card_expenses_object !== NetSuiteCorporateCreditCardExpensesObject.JOURNAL_ENTRY &&
    this.exportSettingForm.value.creditCardExportType === NetSuiteCorporateCreditCardExpensesObject.JOURNAL_ENTRY;

  return isReimbursableChanged || isCorporateCardChanged;
}

This refactored version separates the conditions for reimbursable and corporate card changes, making the code more readable and easier to maintain.


private replaceContentBasedOnConfiguration(updatedConfiguration: string, existingConfiguration: string | undefined | null, exportType: string): string {
const configurationUpdate = `You have changed the export type of $exportType expense from <b>$existingExportType</b> to <b>$updatedExportType</b>,
which would impact a few configurations in the <b>Advanced settings</b>. <br><br>Please revisit the <b>Advanced settings</b> to check and enable the
features that could help customize and automate your integration workflows.`;

let content = '';
// If both are not none and it is an update case else for the new addition case
if (updatedConfiguration && existingConfiguration) {
content = configurationUpdate.replace('$exportType', exportType).replace('$existingExportType', existingConfiguration.toLowerCase().replace(/^\w/, (c: string) => c.toUpperCase())).replace('$updatedExportType', updatedConfiguration.toLowerCase().replace(/^\w/, (c: string) => c.toUpperCase()));
}

return content;
}

Comment on lines +227 to +240
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve string manipulation and extract repeated logic.

The replaceContentBasedOnConfiguration method effectively generates a customized warning message. However, there are opportunities to improve code clarity and reusability.

Consider the following improvements:

  1. Use template literals for easier string interpolation.
  2. Extract the capitalization logic into a separate method.
  3. Simplify the content generation logic.

Here's a refactored version:

private capitalizeFirstLetter(str: string): string {
  return str.charAt(0).toUpperCase() + str.slice(1).toLowerCase();
}

private replaceContentBasedOnConfiguration(updatedConfiguration: string, existingConfiguration: string | undefined | null, exportType: string): string {
  if (updatedConfiguration && existingConfiguration) {
    const existingExportTypeFormatted = this.capitalizeFirstLetter(existingConfiguration);
    const updatedExportTypeFormatted = this.capitalizeFirstLetter(updatedConfiguration);

    return `You have changed the export type of ${exportType} expense from <b>${existingExportTypeFormatted}</b> to <b>${updatedExportTypeFormatted}</b>,
      which would impact a few configurations in the <b>Advanced settings</b>.<br><br>Please revisit the <b>Advanced settings</b> to check and enable the
      features that could help customize and automate your integration workflows.`;
  }
  return '';
}

This refactored version improves readability and reduces code duplication.

private constructWarningMessage(): string {
let content: string = '';
const existingReimbursableExportType = this.exportSettings?.configuration?.reimbursable_expenses_object;
const existingCorporateCardExportType = this.exportSettings?.configuration?.corporate_credit_card_expenses_object;

const updatedReimbursableExportType = this.exportSettingForm.value.reimbursableExportType ? this.exportSettingForm.value.reimbursableExportType : null;
const updatedCorporateCardExportType = this.exportSettingForm.value.creditCardExportType ? this.exportSettingForm.value.creditCardExportType : null;

let updatedExportType;
let existingExportType;
let exportType;

if (existingReimbursableExportType !== updatedReimbursableExportType) {
updatedExportType = updatedReimbursableExportType;
existingExportType = existingReimbursableExportType;
exportType = 'reimbursable';
} else if (existingCorporateCardExportType !== updatedCorporateCardExportType) {
updatedExportType = updatedCorporateCardExportType;
existingExportType = existingCorporateCardExportType;
exportType = 'credit card';
}

if (this.isAdvancedSettingAffected() && exportType) {
content = this.replaceContentBasedOnConfiguration(updatedExportType, existingExportType, exportType);
}

return content;
}

Comment on lines +241 to +269
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Simplify logic and handle multiple changes.

The constructWarningMessage method effectively determines export type changes and constructs a warning message. However, there are opportunities for improvement.

The current implementation only handles one change at a time. If both reimbursable and corporate card export types change, only the first change will be reported in the warning message.

Consider refactoring the method to handle multiple changes and simplify the logic:

private constructWarningMessage(): string {
  const changes = [
    {
      type: 'reimbursable',
      existing: this.exportSettings?.configuration?.reimbursable_expenses_object,
      updated: this.exportSettingForm.value.reimbursableExportType
    },
    {
      type: 'credit card',
      existing: this.exportSettings?.configuration?.corporate_credit_card_expenses_object,
      updated: this.exportSettingForm.value.creditCardExportType
    }
  ];

  return changes
    .filter(change => change.existing !== change.updated && this.isAdvancedSettingAffected())
    .map(change => this.replaceContentBasedOnConfiguration(change.updated, change.existing, change.type))
    .join('<br><br>');
}

This refactored version handles multiple changes, reduces redundancy, and improves readability.

save(): void {
if (this.isAdvancedSettingAffected() && this.exportSettings.configuration) {
this.warningDialogText = this.constructWarningMessage();
this.isConfirmationDialogVisible = true;
return;
}
this.constructPayloadAndSave({hasAccepted: true, event: ConfigurationWarningEvent.NETSUITE_EXPORT_SETTINGS});
}

Expand Down
Loading