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.

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: memo structure in xero app #1094

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/app/branding/c1-contents-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ export const c1Contents = {
cccExpenseBankAccountLabel: 'Which bank account should the bank transactions post to?',
cccExpenseStateSubLabel: 'You can choose to only export expenses when they\'ve been labeled approved or closed. '
},
memoStructureLabel: 'Set the line-item description field in NetSuite',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Xero

Copy link
Contributor

Choose a reason for hiding this comment

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

instead of netsuite, to make it xero

memoStructureSubLabel: 'Choose from a list of available data points that you\'d like to export to the description field in NetSuite. ',
anishfyle marked this conversation as resolved.
Show resolved Hide resolved
stepSubLabel: 'Configure how and when expenses from Expense Management can be exported to Xero.',
cccExpenseStateLabel: 'How should expenses be labeled before exporting from Expense Management?'
},
Expand Down
4 changes: 4 additions & 0 deletions src/app/branding/fyle-contents-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ export const fyleContents = {
billPaymentAccountSubLabel: ', the payment entries will be posted to the selected Payment account in ',
postEntriesCurrentPeriod: 'Post entries in the current accounting period',
contentText: 'In this section, you can customize the integration based on your accounting requirements. ',
topLevelMemoStructureLabel: 'Customize the Top-Level Memo Field',
topLevelMemoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop’s top-level memo field when exporting expenses from Fyle.',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Xero

memoStructureLabel: 'Customize the Line-Item Level Memo Field',
memoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop\’s line-item level memo field when exporting expenses from Fyle.',
Comment on lines +249 to +252
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect reference to QuickBooks Desktop in Xero configuration

The labels incorrectly reference QuickBooks Desktop instead of Xero in both the top-level and line-item level memo structure configurations.

Apply this diff to fix the references:

-                topLevelMemoStructureLabel: 'Customize the Top-Level Memo Field',
-                topLevelMemoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop's top-level memo field when exporting expenses from Fyle.',
-                memoStructureLabel: 'Customize the Line-Item Level Memo Field',
-                memoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop\'s line-item level memo field when exporting expenses from Fyle.',
+                topLevelMemoStructureLabel: 'Customize the Top-Level Memo Field',
+                topLevelMemoStructureSubLabel: 'Select the datapoints you\'d like to export to Xero\'s top-level memo field when exporting expenses from Fyle.',
+                memoStructureLabel: 'Customize the Line-Item Level Memo Field',
+                memoStructureSubLabel: 'Select the datapoints you\'d like to export to Xero\'s line-item level memo field when exporting expenses from Fyle.',
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
topLevelMemoStructureLabel: 'Customize the Top-Level Memo Field',
topLevelMemoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop’s top-level memo field when exporting expenses from Fyle.',
memoStructureLabel: 'Customize the Line-Item Level Memo Field',
memoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop\’s line-item level memo field when exporting expenses from Fyle.',
topLevelMemoStructureLabel: 'Customize the Top-Level Memo Field',
topLevelMemoStructureSubLabel: 'Select the datapoints you\'d like to export to Xero\'s top-level memo field when exporting expenses from Fyle.',
memoStructureLabel: 'Customize the Line-Item Level Memo Field',
memoStructureSubLabel: 'Select the datapoints you\'d like to export to Xero\'s line-item level memo field when exporting expenses from Fyle.',

frequencySubLabel: 'Set a frequency based on how often you want your expenses in Fyle to be exported to Xero.',
customPreferencesLabel: 'Other Preferences',
customPreferencesSubLabel: 'Based on your preference, you can choose whether you want to create any new records in Xero from ' + brandingConfig.brandName + '. (when there is no employee record found, or when the accounting period is closed)',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export type XeroAdvancedSettingWorkspaceGeneralSetting = {
sync_xero_to_fyle_payments: boolean,
auto_create_destination_entity: boolean,
change_accounting_period: boolean,
auto_create_merchant_destination_entity: boolean
auto_create_merchant_destination_entity: boolean,
memo_structure: string[]
}

export type XeroAdvancedSettingGeneralMapping = {
Expand Down Expand Up @@ -111,6 +112,7 @@ export class XeroAdvancedSettingModel extends HelperUtility{
exportSchedule: new FormControl(advancedSettings.workspace_schedules?.enabled ? true : false),
exportScheduleFrequency: new FormControl(advancedSettings.workspace_schedules?.enabled ? advancedSettings.workspace_schedules.interval_hours : 1),
autoCreateMerchantDestinationEntity: new FormControl(advancedSettings.workspace_general_settings.auto_create_merchant_destination_entity ? advancedSettings.workspace_general_settings.auto_create_merchant_destination_entity : false),
memoStructure: new FormControl(advancedSettings.workspace_general_settings.memo_structure),
search: new FormControl(),
searchOption: new FormControl(),
email: new FormControl(advancedSettings?.workspace_schedules?.emails_selected && advancedSettings?.workspace_schedules?.emails_selected?.length > 0 ? AdvancedSettingsModel.filterAdminEmails(advancedSettings?.workspace_schedules?.emails_selected, adminEmails) : []),
Expand All @@ -126,7 +128,8 @@ export class XeroAdvancedSettingModel extends HelperUtility{
sync_xero_to_fyle_payments: advancedSettingsForm.get('paymentSync')?.value && advancedSettingsForm.get('paymentSync')?.value === PaymentSyncDirection.XERO_TO_FYLE ? true : false,
auto_create_destination_entity: advancedSettingsForm.get('autoCreateVendors')?.value,
change_accounting_period: advancedSettingsForm.get('changeAccountingPeriod')?.value,
auto_create_merchant_destination_entity: advancedSettingsForm.get('autoCreateMerchantDestinationEntity')?.value
auto_create_merchant_destination_entity: advancedSettingsForm.get('autoCreateMerchantDestinationEntity')?.value,
memo_structure: advancedSettingsForm.get('memoStructure')?.value
Comment on lines +131 to +132
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

Add default value handling for memo_structure

The payload construction doesn't handle the case where memoStructure control value is undefined or null. Consider adding a default empty array.

-memo_structure: advancedSettingsForm.get('memoStructure')?.value
+memo_structure: advancedSettingsForm.get('memoStructure')?.value || []
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
auto_create_merchant_destination_entity: advancedSettingsForm.get('autoCreateMerchantDestinationEntity')?.value,
memo_structure: advancedSettingsForm.get('memoStructure')?.value
auto_create_merchant_destination_entity: advancedSettingsForm.get('autoCreateMerchantDestinationEntity')?.value,
memo_structure: advancedSettingsForm.get('memoStructure')?.value || []

},
general_mappings: {
payment_account: advancedSettingsForm.get('billPaymentAccount')?.value ? ExportSettingModel.formatGeneralMappingPayload(advancedSettingsForm.get('billPaymentAccount')?.value) : emptyDestinationAttribute
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,32 @@
[formControllerName]="'autoCreateMerchantDestinationEntity'">
</app-configuration-toggle-field>
</div>
<div class="tw-mb-16-px">
<app-configuration-step-sub-header
[label]="'Customization'"
[subLabel]="'In this section, you can customize the data that you\'d like to export from ' + brandingConfig.brandName + ' to Xero. You can choose what data points need to be exported and what shouldn\'t be.'">
</app-configuration-step-sub-header>
</div>
<div class="tw-rounded-12-px tw-border-separator tw-border tw-bg-white tw-mb-16-px">
<app-configuration-multi-select
[form]="advancedSettingForm"
[isFieldMandatory]="false"
[mandatoryErrorListName]="'Item level description'"
[label]="'Set the line item-level Description Field in Xero'"
[subLabel]="'You can choose from a list of available data points that you\'d like to export to the description field in Xero.'"
[options]="defaultMemoFields"
[iconPath]="'list'"
[placeholder]="'Set description'"
[formControllerName]="'memoStructure'"
(changeInMultiSelect)="onMultiSelectChange()">
</app-configuration-multi-select>
<div class="preview-text">
<h4 class="tw-text-form-label-text-color tw-mb-12-px">Preview of the Description Field</h4>
<div class="preview-box">
{{memoPreviewText}}
</div>
</div>
</div>
</div>
<app-configuration-step-footer
[ctaText] = "!isSaveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { FormGroup } from '@angular/forms';
import { Router } from '@angular/router';
import { forkJoin } from 'rxjs';
import { brandingConfig, brandingContent, brandingFeatureConfig, brandingKbArticles } from 'src/app/branding/branding-config';
import { ConditionField, ExpenseFilterResponse } from 'src/app/core/models/common/advanced-settings.model';
import { environment } from 'src/environments/environment';
import { EmailOption, SelectFormOption } from 'src/app/core/models/common/select-form-option.model';
import { DestinationAttribute } from 'src/app/core/models/db/destination-attribute.model';
import { AppName, ConfigurationCta, ToastSeverity, XeroFyleField, XeroOnboardingState } from 'src/app/core/models/enum/enum.model';
Expand All @@ -17,6 +17,7 @@ import { WorkspaceService } from 'src/app/core/services/common/workspace.service
import { OrgService } from 'src/app/core/services/org/org.service';
import { XeroAdvancedSettingsService } from 'src/app/core/services/xero/xero-configuration/xero-advanced-settings.service';
import { XeroHelperService } from 'src/app/core/services/xero/xero-core/xero-helper.service';
import { AdvancedSettingsModel } from 'src/app/core/models/common/advanced-settings.model';

@Component({
selector: 'app-xero-advanced-settings',
Expand All @@ -35,6 +36,10 @@ export class XeroAdvancedSettingsComponent implements OnInit {

advancedSettings: XeroAdvancedSettingGet;

memoPreviewText: string;

defaultMemoFields: string[] = ['employee_email', 'merchant', 'purpose', 'category', 'spent_on', 'report_number', 'expense_link'];

workspaceGeneralSettings: XeroWorkspaceGeneralSetting;

billPaymentAccounts: DestinationAttribute[];
Expand Down Expand Up @@ -82,6 +87,12 @@ export class XeroAdvancedSettingsComponent implements OnInit {
this.router.navigate([`/integrations/xero/onboarding/import_settings`]);
}

onMultiSelectChange() {
const memo = this.advancedSettingForm.controls.memoStructure.value;
const changedMemo = AdvancedSettingsModel.formatMemoPreview(memo, this.defaultMemoFields)[1];
this.advancedSettingForm.controls.memoStructure.patchValue(changedMemo);
}
Comment on lines +90 to +94
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

Add error handling for multi-select changes

The method directly accesses form control values without proper error handling. Consider adding null checks and error handling.

 onMultiSelectChange() {
-  const memo = this.advancedSettingForm.controls.memoStructure.value;
-  const changedMemo = AdvancedSettingsModel.formatMemoPreview(memo, this.defaultMemoFields)[1];
-  this.advancedSettingForm.controls.memoStructure.patchValue(changedMemo);
+  try {
+    const memo = this.advancedSettingForm.get('memoStructure')?.value;
+    if (memo) {
+      const changedMemo = AdvancedSettingsModel.formatMemoPreview(memo, this.defaultMemoFields)[1];
+      this.advancedSettingForm.get('memoStructure')?.patchValue(changedMemo);
+    }
+  } catch (error) {
+    console.error('Error updating memo structure:', error);
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onMultiSelectChange() {
const memo = this.advancedSettingForm.controls.memoStructure.value;
const changedMemo = AdvancedSettingsModel.formatMemoPreview(memo, this.defaultMemoFields)[1];
this.advancedSettingForm.controls.memoStructure.patchValue(changedMemo);
}
onMultiSelectChange() {
try {
const memo = this.advancedSettingForm.get('memoStructure')?.value;
if (memo) {
const changedMemo = AdvancedSettingsModel.formatMemoPreview(memo, this.defaultMemoFields)[1];
this.advancedSettingForm.get('memoStructure')?.patchValue(changedMemo);
}
} catch (error) {
console.error('Error updating memo structure:', error);
}
}


save(): void {
const advancedSettingPayload = XeroAdvancedSettingModel.constructPayload(this.advancedSettingForm);
this.isSaveInProgress = true;
Expand All @@ -108,6 +119,44 @@ export class XeroAdvancedSettingsComponent implements OnInit {
XeroAdvancedSettingModel.setConfigurationSettingValidatorsAndWatchers(this.advancedSettingForm);
}

private formatMemoPreview(): void {
const time = Date.now();
const today = new Date(time);

const previewValues: { [key: string]: string } = {
employee_email: '[email protected]',
category: 'Meals and Entertainment',
purpose: 'Client Meeting',
merchant: 'Pizza Hut',
report_number: 'C/2021/12/R/1',
spent_on: today.toLocaleDateString(),
expense_link: `${environment.fyle_app_url}/app/main/#/enterprise/view_expense/`
};

this.memoPreviewText = '';
const memo: string[] = [];
this.memoStructure.forEach((field, index) => {
if (field in previewValues) {
const defaultIndex = this.defaultMemoFields.indexOf(this.memoStructure[index]);
memo[defaultIndex] = previewValues[field];
}
});
memo.forEach((field, index) => {
this.memoPreviewText += field;
if (index + 1 !== memo.length) {
this.memoPreviewText = this.memoPreviewText + ' - ';
}
});
}

private createMemoStructureWatcher(): void {
this.memoStructure = this.advancedSettingForm.get('memoStructure')?.value;
this.formatMemoPreview();
this.advancedSettingForm.controls.memoStructure.valueChanges.subscribe((memoChanges) => {
this.memoStructure = memoChanges;
this.formatMemoPreview();
});
}
Comment on lines +152 to +159
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential memory leak in subscription

The subscription to valueChanges is not being unsubscribed, which could lead to memory leaks. Consider implementing OnDestroy and properly managing the subscription.

+private memoStructureSubscription: Subscription;

 private createMemoStructureWatcher(): void {
   this.memoStructure = this.advancedSettingForm.get('memoStructure')?.value;
   this.formatMemoPreview();
-  this.advancedSettingForm.controls.memoStructure.valueChanges.subscribe((memoChanges) => {
+  this.memoStructureSubscription = this.advancedSettingForm.get('memoStructure')?.valueChanges.subscribe((memoChanges) => {
     this.memoStructure = memoChanges;
     this.formatMemoPreview();
   });
 }

+ngOnDestroy(): void {
+  if (this.memoStructureSubscription) {
+    this.memoStructureSubscription.unsubscribe();
+  }
+}

Committable suggestion skipped: line range outside the PR's diff.


private setupPage() {
this.isOnboarding = this.router.url.includes('onboarding');
Expand All @@ -122,8 +171,8 @@ export class XeroAdvancedSettingsComponent implements OnInit {
this.workspaceGeneralSettings = response[2];
this.adminEmails = this.advancedSettings.workspace_schedules?.additional_email_options ? this.advancedSettings.workspace_schedules?.additional_email_options.concat(response[3]).flat() : response[3];
this.advancedSettingForm = XeroAdvancedSettingModel.mapAPIResponseToFormGroup(this.advancedSettings, this.adminEmails, this.billPaymentAccounts, this.helperService.shouldAutoEnableAccountingPeriod(this.org.created_at));

this.setupFormWatchers();
this.createMemoStructureWatcher();
this.isLoading = false;
});
}
Expand Down
Loading