Skip to content

fix: JE fixes #1093

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

Merged
merged 4 commits into from
Dec 4, 2024
Merged

fix: JE fixes #1093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c82ff8
JE fixes
DhaaraniCIT Dec 3, 2024
b322eee
JE fixes
DhaaraniCIT Dec 3, 2024
44d41c6
QA bugs fixes
DhaaraniCIT Dec 4, 2024
fb2c214
QA bugs fixes
DhaaraniCIT Dec 4, 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
2 changes: 1 addition & 1 deletion src/app/core/models/common/helper.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export class HelperUtility {
form.controls[controllerName].clearValidators();

// Temporary hack: Don't reset value for fields that are used in reimbursable + ccc section
if (controllerName !== 'accountsPayable' && controllerName !== 'defaultCCCAccount' && controllerName !== 'bankAccount' && controllerName !== 'creditCardAccount' && controllerName !== 'defaultCreditCardVendor' && controllerName !== 'nameInJournalEntry' && controllerName !== 'exportScheduleFrequency') {
if (controllerName !== 'accountsPayable' && controllerName !== 'defaultCCCAccount' && controllerName !== 'bankAccount' && controllerName !== 'creditCardAccount' && controllerName !== 'defaultCreditCardVendor' && controllerName !== 'nameInJournalEntry' && controllerName !== 'exportScheduleFrequency' && controllerName !== 'employeeMapping') {
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring the growing list of exceptions.

The current approach of listing specific field names as exceptions is becoming unwieldy and harder to maintain. The comment "Temporary hack" also suggests this needs improvement.

Consider refactoring to a more maintainable solution:

-if (controllerName !== 'accountsPayable' && controllerName !== 'defaultCCCAccount' && controllerName !== 'bankAccount' && controllerName !== 'creditCardAccount' && controllerName !== 'defaultCreditCardVendor' && controllerName !== 'nameInJournalEntry' && controllerName !== 'exportScheduleFrequency' && controllerName !== 'employeeMapping') {
+const PRESERVE_VALUE_FIELDS = new Set([
+  'accountsPayable',
+  'defaultCCCAccount',
+  'bankAccount',
+  'creditCardAccount',
+  'defaultCreditCardVendor',
+  'nameInJournalEntry',
+  'exportScheduleFrequency',
+  'employeeMapping'
+]);
+
+if (!PRESERVE_VALUE_FIELDS.has(controllerName)) {

This approach:

  1. Makes the list of exceptions more maintainable
  2. Improves readability
  3. Provides better performance for lookups
  4. Makes it easier to add/remove fields

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

form.controls[controllerName].setValue(null);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/app/core/models/db/destination-attribute.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { BusinessCentralDestinationAttributes } from "../business-central/db/business-central-destination-attribute.model";
import { QbdDirectDestinationAttribute } from "../qbd-direct/db/qbd-direct-destination-attribuite.model";
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in import path

The import path contains a spelling error: "attribuite" should be "attribute".

-import { QbdDirectDestinationAttribute } from "../qbd-direct/db/qbd-direct-destination-attribuite.model";
+import { QbdDirectDestinationAttribute } from "../qbd-direct/db/qbd-direct-destination-attribute.model";
📝 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
import { QbdDirectDestinationAttribute } from "../qbd-direct/db/qbd-direct-destination-attribuite.model";
import { QbdDirectDestinationAttribute } from "../qbd-direct/db/qbd-direct-destination-attribute.model";

import { Sage300DestinationAttributes } from "../sage300/db/sage300-destination-attribuite.model";
import { PaginatedResponse } from "./paginated-response.model";

Expand Down Expand Up @@ -36,7 +37,7 @@ export interface PaginatedDestinationAttribute extends PaginatedResponse {
}

export type GroupedDestinationAttribute = {
ACCOUNT: Sage300DestinationAttributes[] | BusinessCentralDestinationAttributes[],
ACCOUNT: Sage300DestinationAttributes[] | BusinessCentralDestinationAttributes[] | QbdDirectDestinationAttribute[],
EXPENSE_TYPE: Sage300DestinationAttributes[],
EXPENSE_PAYMENT_TYPE: Sage300DestinationAttributes[] | BusinessCentralDestinationAttributes[],
VENDOR_PAYMENT_ACCOUNT: DestinationAttribute[],
Expand Down
2 changes: 1 addition & 1 deletion src/app/core/models/db/task-log.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ export type TaskLogGetParams = {
type__in?: string[];
expense_group_ids?: number[];
task_type?: string[];
export_log__id?: number[];
export_log__id__in?: number[];
};
11 changes: 10 additions & 1 deletion ...app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-advanced-settings.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ export class QbdDirectAdvancedSettingsModel extends AdvancedSettingsModel {
return ["employee_name", "Expense/Report ID"];
}

static topMemoExpenseKeyNameConversion(keys: string[]): string[] {
keys.forEach((key: string, index: number) => {
if (key === 'expense_key') {
keys[index] = 'Expense/Report ID';
}
});
return keys;
}
Comment on lines +32 to +39
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve method implementation to follow best practices

The current implementation has potential issues:

  1. Mutates the input array directly
  2. Lacks input validation
  3. Has unclear return semantics (both mutates and returns)

Consider this safer implementation:

-    static topMemoExpenseKeyNameConversion(keys: string[]): string[] {
-        keys.forEach((key: string, index: number) => {
-            if (key === 'expense_key') {
-                keys[index] = 'Expense/Report ID';
-            }
-        });
-        return keys;
-    }
+    static topMemoExpenseKeyNameConversion(keys: string[] | null | undefined): string[] {
+        if (!keys?.length) {
+            return [];
+        }
+        return keys.map((key: string) => 
+            key === 'expense_key' ? 'Expense/Report ID' : key
+        );
+    }
📝 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
static topMemoExpenseKeyNameConversion(keys: string[]): string[] {
keys.forEach((key: string, index: number) => {
if (key === 'expense_key') {
keys[index] = 'Expense/Report ID';
}
});
return keys;
}
static topMemoExpenseKeyNameConversion(keys: string[] | null | undefined): string[] {
if (!keys?.length) {
return [];
}
return keys.map((key: string) =>
key === 'expense_key' ? 'Expense/Report ID' : key
);
}


static formatMemoStructure(memoStructure: string[], defaultMemoOptions: string[]): string[] {
const originMemo: string[] = [];
defaultMemoOptions.forEach((field, index) => {
Expand All @@ -43,7 +52,7 @@ export class QbdDirectAdvancedSettingsModel extends AdvancedSettingsModel {

return new FormGroup({
expenseMemoStructure: new FormControl(advancedSettings?.line_level_memo_structure && advancedSettings?.line_level_memo_structure.length > 0 ? this.formatMemoStructure(this.defaultMemoFields(), advancedSettings?.line_level_memo_structure) : this.defaultMemoFields(), Validators.required),
topMemoStructure: new FormControl(advancedSettings?.top_level_memo_structure && advancedSettings?.top_level_memo_structure.length > 0 ? advancedSettings?.top_level_memo_structure : this.defaultTopMemoOptions(), Validators.required),
topMemoStructure: new FormControl(advancedSettings?.top_level_memo_structure && advancedSettings?.top_level_memo_structure.length > 0 ? this.topMemoExpenseKeyNameConversion(advancedSettings?.top_level_memo_structure) : this.defaultTopMemoOptions(), Validators.required),
exportSchedule: new FormControl(advancedSettings?.schedule_is_enabled ? advancedSettings?.schedule_is_enabled : false),
email: new FormControl(advancedSettings?.emails_selected ? advancedSettings?.emails_selected : null),
exportScheduleFrequency: new FormControl(advancedSettings?.schedule_is_enabled ? advancedSettings?.interval_hours : 1),
Expand Down
10 changes: 6 additions & 4 deletions src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-export-settings.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ export class QbdDirectExportSettingModel extends ExportSettingModel {
return form.controls.creditCardExportType.value === QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY;
case 'defaultCreditCardAccountName':
return form.controls.creditCardExportType.value === QBDCorporateCreditCardExpensesObject.CREDIT_CARD_PURCHASE;
case 'employeeMapping':
return !form.get('reimbursableExportType')?.value && form.get('creditCardExportType')?.value && form.get('creditCardExportType')?.value === QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY;
Comment on lines +177 to +178
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Review the employeeMapping mandatory field logic

The current condition for making employeeMapping mandatory seems inconsistent. It only requires employeeMapping when reimbursableExportType is NOT set, but the validation rules later show that employeeMapping is required for both BILL and JOURNAL_ENTRY types. This could lead to validation inconsistencies.

Consider updating the condition to:

case 'employeeMapping':
-  return !form.get('reimbursableExportType')?.value && form.get('creditCardExportType')?.value && form.get('creditCardExportType')?.value === QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY;
+  return (form.get('reimbursableExportType')?.value === QBDReimbursableExpensesObject.BILL || 
+          form.get('reimbursableExportType')?.value === QBDReimbursableExpensesObject.JOURNAL_ENTRY) || 
+         (form.get('creditCardExportType')?.value === QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY);

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

default:
return false;
}
Expand All @@ -182,23 +184,23 @@ export class QbdDirectExportSettingModel extends ExportSettingModel {
static getValidators(): [ExportSettingValidatorRule, ExportModuleRule[]] {

const exportSettingValidatorRule: ExportSettingValidatorRule = {
reimbursableExpense: ['reimbursableExportType', 'reimbursableExportGroup', 'reimbursableExportDate', 'reimbursableExpenseState', 'employeeMapping'],
reimbursableExpense: ['reimbursableExportType', 'reimbursableExportGroup', 'reimbursableExportDate', 'reimbursableExpenseState'],
creditCardExpense: ['creditCardExportType', 'creditCardExportGroup', 'creditCardExportDate', 'creditCardExpenseState']
};

const exportModuleRule: ExportModuleRule[] = [
{
formController: 'reimbursableExportType',
requiredValue: {
[QbdDirectReimbursableExpensesObject.BILL]: [],
[QbdDirectReimbursableExpensesObject.JOURNAL_ENTRY]: ['defaultReimbursableAccountsPayableAccountName']
[QbdDirectReimbursableExpensesObject.BILL]: ['employeeMapping'],
[QbdDirectReimbursableExpensesObject.JOURNAL_ENTRY]: ['defaultReimbursableAccountsPayableAccountName', 'employeeMapping']
}
},
{
formController: 'creditCardExportType',
requiredValue: {
[QBDCorporateCreditCardExpensesObject.CREDIT_CARD_PURCHASE]: ['defaultCreditCardAccountName'],
[QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY]: ['defaultCCCAccountsPayableAccountName', 'nameInJE']
[QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY]: ['defaultCCCAccountsPayableAccountName', 'nameInJE', 'employeeMapping']
}
}
];
Expand Down
2 changes: 1 addition & 1 deletion src/app/core/services/common/dashboard.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export class DashboardService {
}

if (expenseGroupIds.length) {
const expenseKey = appName === AppName.INTACCT ? 'expense_group_ids' : appName === AppName.QBD_DIRECT ? 'export_log__id' : 'expense_group_id__in';
const expenseKey = appName === AppName.INTACCT ? 'expense_group_ids' : appName === AppName.QBD_DIRECT ? 'export_log__id__in' : 'expense_group_id__in';
apiParams[expenseKey] = expenseGroupIds;
}

Expand Down
1 change: 1 addition & 0 deletions ...tions/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
[isImportItemsEnabled]="isImportItemsEnabled"
[destinationOptionsVersion]="'v1'"
[importCodeFields]="importCodeFields"
[chartOfAccounts]="chartOfAccounts"
[errorsVersion]="'v1'">
</app-dashboard-error-section>
</div>
4 changes: 4 additions & 0 deletions ...rations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export class QbdDirectDashboardComponent implements OnInit {

importCodeFields: any;

chartOfAccounts: string[];

constructor(
private accountingExportService: AccountingExportService,
private dashboardService: DashboardService,
Expand Down Expand Up @@ -151,6 +153,8 @@ export class QbdDirectDashboardComponent implements OnInit {

this.importCodeFields = responses[5].import_settings?.import_code_fields;

this.chartOfAccounts = responses[5].import_settings.chart_of_accounts;

Comment on lines +156 to +157
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add null safety check for import_settings

The code assumes import_settings exists in the response. Add a null check to prevent potential runtime errors.

-      this.chartOfAccounts = responses[5].import_settings.chart_of_accounts;
+      this.chartOfAccounts = responses[5].import_settings?.chart_of_accounts ?? [];
📝 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
this.chartOfAccounts = responses[5].import_settings.chart_of_accounts;
this.chartOfAccounts = responses[5].import_settings?.chart_of_accounts ?? [];

const queuedTasks: QbdDirectTaskLog[] = responses[2].results.filter((task: QbdDirectTaskLog) => this.exportLogProcessingStates.includes(task.status));
this.failedExpenseGroupCount = responses[2].results.filter((task: QbdDirectTaskLog) => task.status === TaskLogState.FAILED || task.status === TaskLogState.FATAL).length;

Expand Down
6 changes: 3 additions & 3 deletions ...ct/qbd-direct-shared/qbd-direct-export-settings/qbd-direct-export-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
[showExportPreview]="false">
</app-configuration-select-field>

<div *ngIf="!exportSettingsForm.get('reimbursableExpense')?.value && exportSettingsForm.get('creditCardExportType')?.value && exportSettingsForm.get('creditCardExportType')?.value === QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY">
<div *ngIf="exportSettingsForm.get('creditCardExportType')?.value && helperService.isFieldRequired(exportSettingsForm, 'employeeMapping')">
<app-configuration-select-field
[form]="exportSettingsForm"
[isFieldMandatory]="true"
Expand Down Expand Up @@ -174,7 +174,7 @@
<div *ngIf="exportSettingsForm.get('creditCardExportType')?.value && helperService.isFieldRequired(exportSettingsForm, 'defaultCreditCardAccountName')">
<app-configuration-select-field
[form]="exportSettingsForm"
[destinationAttributes]="cccAccpuntOptions(exportSettingsForm.get('creditCardExportType')?.value)"
[destinationAttributes]="cccAccountOptions(exportSettingsForm.get('creditCardExportType')?.value)"
[destinationOptionKey]="QbdDirectExportSettingDestinationOptionKey.CREDIT_CARD_ACCOUNT"
[isOptionSearchInProgress]="isOptionSearchInProgress"
[isAdvanceSearchEnabled]="true"
Expand All @@ -194,7 +194,7 @@
<div *ngIf="helperService.isFieldRequired(exportSettingsForm, 'defaultCCCAccountsPayableAccountName')">
<app-configuration-select-field
[form]="exportSettingsForm"
[destinationAttributes]="destinationOptionsWatcher(['AccountsPayable'] , destinationAccounts)"
[destinationAttributes]="cccAccountOptions(exportSettingsForm.get('creditCardExportType')?.value)"
[destinationOptionKey]="QbdDirectExportSettingDestinationOptionKey.ACCOUNTS_PAYABLE"
[isOptionSearchInProgress]="isOptionSearchInProgress"
[isAdvanceSearchEnabled]="true"
Expand Down
4 changes: 2 additions & 2 deletions ...rect/qbd-direct-shared/qbd-direct-export-settings/qbd-direct-export-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class QbdDirectExportSettingsComponent implements OnInit{
return this.destinationOptionsWatcher(['Bank', 'AccountsPayable', 'CreditCard', 'OtherCurrentLiability', 'LongTermLiability'], this.destinationAccounts);
}

cccAccpuntOptions(cccExportType: string): DestinationAttribute[] {
cccAccountOptions(cccExportType: string): DestinationAttribute[] {
if (cccExportType === QBDCorporateCreditCardExpensesObject.CREDIT_CARD_PURCHASE) {
return this.destinationOptionsWatcher(['CreditCard'], this.destinationAccounts);
} else if (cccExportType === QBDCorporateCreditCardExpensesObject.JOURNAL_ENTRY && this.exportSettingsForm.controls.employeeMapping.value === EmployeeFieldMapping.EMPLOYEE && this.exportSettingsForm.controls.nameInJE.value === EmployeeFieldMapping.EMPLOYEE) {
Expand Down Expand Up @@ -182,7 +182,7 @@ export class QbdDirectExportSettingsComponent implements OnInit{

cccExportTypeWatcher(): void {
this.exportSettingsForm.controls.creditCardExportType.valueChanges.subscribe((creditCardExportTypeValue) => {
this.exportSettingsForm.controls.creditCardExportGroup.patchValue(this.creditCardExpenseGroupingFieldOptions[1].value);
this.exportSettingsForm.controls.creditCardExportGroup.patchValue(QbdDirectExportSettingModel.expenseGroupingFieldOptions()[1].value);
this.exportSettingsForm.controls.creditCardExportGroup.disable();
this.creditCardExpenseGroupingFieldOptions = [QbdDirectExportSettingModel.expenseGroupingFieldOptions()[1]];
});
Comment on lines 184 to 188
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 3, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Replace hardcoded index with explicit value reference

Using a hardcoded index [1] for accessing options is fragile and could break if the options array changes. Consider using a constant or finding the option by value.

  cccExportTypeWatcher(): void {
    this.exportSettingsForm.controls.creditCardExportType.valueChanges.subscribe((creditCardExportTypeValue) => {
-       this.exportSettingsForm.controls.creditCardExportGroup.patchValue(QbdDirectExportSettingModel.expenseGroupingFieldOptions()[1].value);
+       const defaultOption = QbdDirectExportSettingModel.expenseGroupingFieldOptions().find(option => 
+         option.value === QbdDirectExpenseGroupBy.EXPENSE
+       );
+       if (!defaultOption) {
+         throw new Error('Default expense grouping option not found');
+       }
+       this.exportSettingsForm.controls.creditCardExportGroup.patchValue(defaultOption.value);
        this.exportSettingsForm.controls.creditCardExportGroup.disable();
-       this.creditCardExpenseGroupingFieldOptions = [QbdDirectExportSettingModel.expenseGroupingFieldOptions()[1]];
+       this.creditCardExpenseGroupingFieldOptions = [defaultOption];
    });
  }
📝 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
this.exportSettingsForm.controls.creditCardExportType.valueChanges.subscribe((creditCardExportTypeValue) => {
this.exportSettingsForm.controls.creditCardExportGroup.patchValue(this.creditCardExpenseGroupingFieldOptions[1].value);
this.exportSettingsForm.controls.creditCardExportGroup.patchValue(QbdDirectExportSettingModel.expenseGroupingFieldOptions()[1].value);
this.exportSettingsForm.controls.creditCardExportGroup.disable();
this.creditCardExpenseGroupingFieldOptions = [QbdDirectExportSettingModel.expenseGroupingFieldOptions()[1]];
});
this.exportSettingsForm.controls.creditCardExportType.valueChanges.subscribe((creditCardExportTypeValue) => {
const defaultOption = QbdDirectExportSettingModel.expenseGroupingFieldOptions().find(option =>
option.value === QbdDirectExpenseGroupBy.EXPENSE
);
if (!defaultOption) {
throw new Error('Default expense grouping option not found');
}
this.exportSettingsForm.controls.creditCardExportGroup.patchValue(defaultOption.value);
this.exportSettingsForm.controls.creditCardExportGroup.disable();
this.creditCardExpenseGroupingFieldOptions = [defaultOption];
});

Expand Down
9 changes: 8 additions & 1 deletion .../shared/components/dashboard/dashboard-error-section/dashboard-error-section.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { MappingService } from 'src/app/core/services/common/mapping.service';
import { TrackingService } from 'src/app/core/services/integration/tracking.service';
import { WindowService } from 'src/app/core/services/common/window.service';
import { HelperService } from 'src/app/core/services/common/helper.service';
import { QbdDirectDestinationAttribute } from 'src/app/core/models/qbd-direct/db/qbd-direct-destination-attribuite.model';

@Component({
selector: 'app-dashboard-error-section',
Expand Down Expand Up @@ -51,6 +52,8 @@ export class DashboardErrorSectionComponent implements OnInit {

@Input() importCodeFields: string[];

@Input() chartOfAccounts: string[];

filteredMappings: ExtendedGenericMapping[];

destinationOptions: DestinationAttribute[];
Expand Down Expand Up @@ -134,6 +137,10 @@ export class DashboardErrorSectionComponent implements OnInit {
}, 100);
}

destinationOptionsWatcher(detailAccountType: string[], destinationOptions: QbdDirectDestinationAttribute[]): DestinationAttribute[] {
return destinationOptions.filter((account: QbdDirectDestinationAttribute) => detailAccountType.includes(account.detail.account_type));
}
Comment on lines +140 to +142
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider improving method name and robustness.

The current implementation could be enhanced for better clarity and safety:

  1. The method name destinationOptionsWatcher suggests it's watching for changes, but it's actually filtering options. Consider renaming to filterDestinationOptionsByAccountType.
  2. Add null checks to prevent runtime errors.
-  destinationOptionsWatcher(detailAccountType: string[], destinationOptions: QbdDirectDestinationAttribute[]): DestinationAttribute[] {
-    return destinationOptions.filter((account: QbdDirectDestinationAttribute) =>  detailAccountType.includes(account.detail.account_type));
+  private filterDestinationOptionsByAccountType(detailAccountType: string[], destinationOptions: QbdDirectDestinationAttribute[]): DestinationAttribute[] {
+    if (!detailAccountType?.length || !destinationOptions?.length) {
+      return [];
+    }
+    return destinationOptions.filter((account: QbdDirectDestinationAttribute) => 
+      account.detail?.account_type && detailAccountType.includes(account.detail.account_type)
+    );
📝 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
destinationOptionsWatcher(detailAccountType: string[], destinationOptions: QbdDirectDestinationAttribute[]): DestinationAttribute[] {
return destinationOptions.filter((account: QbdDirectDestinationAttribute) => detailAccountType.includes(account.detail.account_type));
}
private filterDestinationOptionsByAccountType(detailAccountType: string[], destinationOptions: QbdDirectDestinationAttribute[]): DestinationAttribute[] {
if (!detailAccountType?.length || !destinationOptions?.length) {
return [];
}
return destinationOptions.filter((account: QbdDirectDestinationAttribute) =>
account.detail?.account_type && detailAccountType.includes(account.detail.account_type)
);
}


private getDestinationOptionsV2(errorType: AccountingErrorType) {
this.mappingService.getGroupedDestinationAttributes([this.destinationField], 'v2').subscribe(groupedDestinationResponse => {
if (this.sourceField === 'EMPLOYEE') {
Expand All @@ -142,7 +149,7 @@ export class DashboardErrorSectionComponent implements OnInit {
if (this.destinationField === 'EXPENSE_TYPE') {
this.destinationOptions = groupedDestinationResponse.EXPENSE_TYPE;
} else {
this.destinationOptions = groupedDestinationResponse.ACCOUNT;
this.destinationOptions = this.appName !== AppName.QBD_DIRECT ? groupedDestinationResponse.ACCOUNT : this.destinationOptionsWatcher( this.chartOfAccounts, groupedDestinationResponse.ACCOUNT as QbdDirectDestinationAttribute[]);
}
}

Expand Down
Loading