-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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'; | ||||||||||||||||||||||||||||||||||
|
@@ -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', | ||||||||||||||||||||||||||||||||||
|
@@ -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[]; | ||||||||||||||||||||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
save(): void { | ||||||||||||||||||||||||||||||||||
const advancedSettingPayload = XeroAdvancedSettingModel.constructPayload(this.advancedSettingForm); | ||||||||||||||||||||||||||||||||||
this.isSaveInProgress = true; | ||||||||||||||||||||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix potential memory leak in subscription The subscription to +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();
+ }
+}
|
||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private setupPage() { | ||||||||||||||||||||||||||||||||||
this.isOnboarding = this.router.url.includes('onboarding'); | ||||||||||||||||||||||||||||||||||
|
@@ -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; | ||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
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.📝 Committable suggestion