fix: merge expense inconsistency in mobile app (#2882)

* fix: merge expense inconsistency in mobile app * minor * minor * major * console remove * test: merge expense test fix (#2889) * test: merge expense test fix * major * minor
fylein · Apr 26, 2024 · 01609b4 · 01609b4
1 parent 7258837
commit 01609b4
Show file tree

Hide file tree

Showing 7 changed files with 75 additions and 24 deletions.
diff --git a/src/app/core/models/generic-fields-form-values.model.ts b/src/app/core/models/generic-fields-form-values.model.ts
@@ -1,3 +1,4 @@
+// TODO - Use snake_case instead of camelCase
 export interface GenericFieldsFormValues {
   amount: string;
   dateOfSpend: Date;
@@ -10,4 +11,5 @@ export interface GenericFieldsFormValues {
   tax_amount: number;
   costCenter: number;
   purpose: string;
+  receipts_from: string;
 }
diff --git a/src/app/fyle/merge-expense/generic-fields-form/generic-fields-form.component.html b/src/app/fyle/merge-expense/generic-fields-form/generic-fields-form.component.html
@@ -92,15 +92,15 @@
   <div *ngIf="receiptOptions?.length !== 0" class="merge-expenses--internal-block">
     <app-fy-select
       [enableSearch]="false"
-      [label]="'Select receipt'"
+      [label]="'Receipt'"
       [disabled]="disableFormElements || receiptOptions?.length === 0"
       [mandatory]="false"
       [nullOption]="false"
       [options]="receiptOptions"
       [selectModalHeader]="'Select receipt'"
-      formControlName="receipt_ids"
-      [touchedInParent]="genericFieldsFormGroup.controls.receipt_ids.touched"
-      [validInParent]="genericFieldsFormGroup.controls.receipt_ids.valid"
+      formControlName="receipts_from"
+      [touchedInParent]="genericFieldsFormGroup.controls.receipts_from.touched"
+      [validInParent]="genericFieldsFormGroup.controls.receipts_from.valid"
     >
     </app-fy-select>
   </div>

diff --git a/src/app/fyle/merge-expense/generic-fields-form/generic-fields-form.component.spec.ts b/src/app/fyle/merge-expense/generic-fields-form/generic-fields-form.component.spec.ts
@@ -66,7 +66,7 @@ describe('GenericFieldsFormComponent', () => {
     it('should emit receiptChanged event if receipt option changes', () => {
       spyOn(component.receiptChanged, 'emit');
       component.ngOnInit();
-      component.genericFieldsFormGroup.controls.receipt_ids.setValue('txErhlkzewZF');
+      component.genericFieldsFormGroup.controls.receipts_from.setValue('txErhlkzewZF');
       expect(component.receiptChanged.emit).toHaveBeenCalledOnceWith('txErhlkzewZF');
     });
 

diff --git a/src/app/fyle/merge-expense/generic-fields-form/generic-fields-form.component.ts b/src/app/fyle/merge-expense/generic-fields-form/generic-fields-form.component.ts
@@ -87,7 +87,7 @@ export class GenericFieldsFormComponent implements OnInit, ControlValueAccessor,
   ngOnInit(): void {
     this.genericFieldsFormGroup = this.formBuilder.group({
       amount: [, Validators.required],
-      receipt_ids: [],
+      receipts_from: [],
       dateOfSpend: [],
       paymentMode: [, Validators.required],
       project: [],
@@ -120,8 +120,8 @@ export class GenericFieldsFormComponent implements OnInit, ControlValueAccessor,
       this.paymentModeChanged.emit(paymentMode);
     });
 
-    this.genericFieldsFormGroup.controls.receipt_ids.valueChanges.subscribe((receiptIds: string) => {
-      this.receiptChanged.emit(receiptIds);
+    this.genericFieldsFormGroup.controls.receipts_from.valueChanges.subscribe((receiptsFrom: string) => {
+      this.receiptChanged.emit(receiptsFrom);
     });
 
     this.genericFieldsFormGroup.valueChanges.subscribe((formControlNames: FormGroup) => {

diff --git a/src/app/fyle/merge-expense/merge-expense-1.page.spec.ts b/src/app/fyle/merge-expense/merge-expense-1.page.spec.ts
@@ -149,6 +149,7 @@ export function TestCases1(getTestBed) {
         spyOn(component, 'loadCategoryDependentFields');
         spyOn(component, 'subscribeExpenseChange');
         spyOn(component, 'generateCustomInputOptions').and.returnValue(combinedOptionsData1);
+        spyOn(component, 'setupDefaultReceipts');
       });
 
       it('should setup class observables', () => {
@@ -300,13 +301,33 @@ export function TestCases1(getTestBed) {
           expect(res).toEqual(optionsData21);
         });
 
+        expect(component.setupDefaultReceipts).toHaveBeenCalledTimes(1);
+
         expect(mergeExpensesService.generateDistanceOptions).toHaveBeenCalledOnceWith(transformedPlatformedExpense1);
         expect(mergeExpensesService.generateDistanceUnitOptions).toHaveBeenCalledOnceWith(
           transformedPlatformedExpense1
         );
       });
     });
 
+    describe('setupDefaultReceipts():', () => {
+      beforeEach(() => {
+        component.expenses = cloneDeep(transformedPlatformedExpense1);
+        component.genericFieldsOptions$ = of(cloneDeep(combinedOptionsData1));
+      });
+
+      it('should set receipts_from as transaction ID if any of the merged expense has receipt', () => {
+        component.expenses[1].tx_file_ids = ['fi2xk29232qwr'];
+        component.setupDefaultReceipts(component.expenses);
+        expect(component.fg.controls.genericFields.value.receipts_from).toEqual('txZA0Oj6TV9c');
+      });
+
+      it('should set receipts_from as null if none of the merged expense has receipt', () => {
+        component.setupDefaultReceipts(component.expenses);
+        expect(component.fg.controls.genericFields.value.receipts_from).toEqual(null);
+      });
+    });
+
     describe('loadGenericFieldsOptions(): ', () => {
       beforeEach(() => {
         component.expenses = expenseList2;
@@ -344,6 +365,7 @@ export function TestCases1(getTestBed) {
           tax_amount: 0.01,
           costCenter: null,
           purpose: null,
+          receipts_from: undefined,
         });
         expect(mergeExpensesService.getFieldValue).toHaveBeenCalledTimes(11);
       });

diff --git a/src/app/fyle/merge-expense/merge-expense-2.page.spec.ts b/src/app/fyle/merge-expense/merge-expense-2.page.spec.ts
@@ -126,33 +126,40 @@ export function TestCases2(getTestBed) {
         expect(mergeExpensesService.getFieldValueOnChange).toHaveBeenCalledTimes(11);
       });
 
-      it('should update receipt_ids to null if expense is undefined', () => {
+      it('should update receipts_from to undefined if expense is undefined', () => {
         component.onExpenseChanged(-1);
-        expect(component.fg.controls.genericFields.value.receipt_ids).toEqual(null);
+        expect(component.fg.controls.genericFields.value.receipts_from).toBeUndefined();
       });
 
-      it('should update receipt_ids to null if tx_file_ids is undefined', () => {
+      it('should update receipts_from to undefined if tx_file_ids is undefined', () => {
         component.onExpenseChanged(1);
-        expect(component.fg.controls.genericFields.value.receipt_ids).toEqual(null);
+        expect(component.fg.controls.genericFields.value.receipts_from).toBeUndefined();
       });
 
-      it('should update receipt_ids to null if tx_file_ids is empty', () => {
+      it('should update receipts_from to undefined if tx_file_ids is empty', () => {
         component.expenses[1].tx_file_ids = [];
         component.onExpenseChanged(1);
-        expect(component.fg.controls.genericFields.value.receipt_ids).toEqual(null);
+        expect(component.fg.controls.genericFields.value.receipts_from).toBeUndefined();
       });
 
-      it('should set receipt_ids to tx_split_group_id if tx_file_ids is not empty and touchedGenericFields is undefined', () => {
+      it('should set receipts_from to null if touchedGenericFields includes receipts_from', () => {
+        component.touchedGenericFields = ['receipts_from'];
         component.expenses[1].tx_file_ids = ['fiGLwwPtYD8X'];
         component.onExpenseChanged(1);
-        expect(component.fg.controls.genericFields.value.receipt_ids).toEqual('tx3nHShG60zq');
+        expect(component.fg.controls.genericFields.value.receipts_from).toBeNull();
       });
 
-      it('should set receipt_ids to tx_split_group_id if tx_file_ids is not empty and receipt_ids field is not touched in the form', () => {
+      it('should set receipts_from to tx_id if tx_file_ids is not empty and touchedGenericFields is undefined', () => {
+        component.expenses[1].tx_file_ids = ['fiGLwwPtYD8X'];
+        component.onExpenseChanged(1);
+        expect(component.fg.controls.genericFields.value.receipts_from).toEqual('tx3nHShG60zq');
+      });
+
+      it('should set receipts_from to tx_id if tx_file_ids is not empty and receipts_from field is not touched in the form', () => {
         component.expenses[1].tx_file_ids = ['fiGLwwPtYD8X'];
         component.touchedGenericFields = ['dateOfSpend', 'paymentMode'];
         component.onExpenseChanged(1);
-        expect(component.fg.controls.genericFields.value.receipt_ids).toEqual('tx3nHShG60zq');
+        expect(component.fg.controls.genericFields.value.receipts_from).toEqual('tx3nHShG60zq');
       });
 
       it('should call getFieldValueOnChange for updating amount with correct arguments', () => {

diff --git a/src/app/fyle/merge-expense/merge-expense.page.ts b/src/app/fyle/merge-expense/merge-expense.page.ts
@@ -325,11 +325,24 @@ export class MergeExpensePage implements OnInit, AfterViewChecked {
         this.receiptOptions = receiptOptions;
       });
 
-    expenses$.subscribe((expenses) => (this.expenses = expenses));
+    expenses$.subscribe((expenses) => {
+      this.expenses = expenses;
+      // Set receipts from expenses if the merge form is having one or more expenses without receipts
+      this.setupDefaultReceipts(expenses);
+    });
 
     this.combinedCustomProperties = this.generateCustomInputOptions(customProperties as Partial<CustomInput>[][]);
   }
 
+  setupDefaultReceipts(expenses: Partial<Expense>[]): void {
+    const expenseWithReceipt = expenses.find((expense) => expense.tx_file_ids.length > 0);
+    this.fg.patchValue({
+      genericFields: {
+        receipts_from: expenseWithReceipt?.tx_id || null,
+      },
+    });
+  }
+
   loadGenericFieldsOptions(): void {
     this.genericFieldsOptions$.subscribe(
       ({
@@ -358,6 +371,7 @@ export class MergeExpensePage implements OnInit, AfterViewChecked {
             tax_amount: this.mergeExpensesService.getFieldValue(taxAmountOptionsData),
             costCenter: this.mergeExpensesService.getFieldValue(constCenterOptionsData),
             purpose: this.mergeExpensesService.getFieldValue(purposeOptionsData),
+            receipts_from: this.genericFieldsFormValues?.receipts_from,
           },
         });
         const expensesInfo = this.mergeExpensesService.setDefaultExpenseToKeep(this.expenses);
@@ -392,11 +406,6 @@ export class MergeExpensePage implements OnInit, AfterViewChecked {
       }) => {
         this.fg.patchValue({
           genericFields: {
-            receipt_ids:
-              this.expenses[selectedIndex]?.tx_file_ids?.length > 0 &&
-              !this.touchedGenericFields?.includes('receipt_ids')
-                ? this.expenses[selectedIndex].tx_split_group_id
-                : null,
             amount: this.mergeExpensesService.getFieldValueOnChange(
               amountOptionsData,
               this.touchedGenericFields?.includes('amount'),
@@ -465,6 +474,17 @@ export class MergeExpensePage implements OnInit, AfterViewChecked {
             ),
           },
         });
+
+        // Change receipt form field only if the selected expense has receipts
+        if (this.expenses[selectedIndex]?.tx_file_ids?.length > 0) {
+          this.fg.patchValue({
+            genericFields: {
+              receipts_from: !this.touchedGenericFields?.includes('receipts_from')
+                ? this.expenses[selectedIndex].tx_id
+                : null,
+            },
+          });
+        }
       }
     );
   }