From d6dfabbce3a73775d6a4ce531a4eed1d3ea9d519 Mon Sep 17 00:00:00 2001 From: Suyash Patil <127177049+suyashpatil78@users.noreply.github.com> Date: Mon, 19 Feb 2024 19:14:26 +0530 Subject: [PATCH] fix: concurrency issue in mobile app where report is approved but edit expense page was opening up (#2763) --- .../my-view-report.page.spec.ts | 54 ++++----------- .../my-view-report/my-view-report.page.ts | 68 ++++++++++--------- 2 files changed, 49 insertions(+), 73 deletions(-) diff --git a/src/app/fyle/my-view-report/my-view-report.page.spec.ts b/src/app/fyle/my-view-report/my-view-report.page.spec.ts index 765f10adf3..2c87427bee 100644 --- a/src/app/fyle/my-view-report/my-view-report.page.spec.ts +++ b/src/app/fyle/my-view-report/my-view-report.page.spec.ts @@ -664,8 +664,11 @@ describe('MyViewReportPage', () => { }); describe('goToTransaction():', () => { - it('should go to view expense page', () => { - spyOn(component, 'canEditExpense').and.returnValue(false); + beforeEach(() => { + component.canEdit$ = of(false); + }); + + it('should go to view expense page if canEdit is false', () => { component.goToTransaction({ expense: expenseData, expenseIndex: 0, @@ -686,12 +689,11 @@ describe('MyViewReportPage', () => { view: ExpenseView.individual, }, ]); - expect(component.canEditExpense).toHaveBeenCalledOnceWith(expenseData.state); }); - it('should go to view edit expense page', () => { + it('should go to edit expense page if canEdit is true', () => { + component.canEdit$ = of(true); component.erpt$ = of(expectedAllReports[0]); - component.canEditExpense = () => true; fixture.detectChanges(); component.goToTransaction({ expense: expenseData, @@ -710,8 +712,7 @@ describe('MyViewReportPage', () => { ]); }); - it('should go to view mileage page', () => { - spyOn(component, 'canEditExpense').and.returnValue(false); + it('should go to view mileage page if category is mileage and canEdit is false', () => { component.goToTransaction({ expense: mileageExpense, expenseIndex: 0, @@ -732,13 +733,12 @@ describe('MyViewReportPage', () => { view: ExpenseView.individual, }, ]); - expect(component.canEditExpense).toHaveBeenCalledOnceWith(mileageExpense.state); }); - it('should go to edit mileage page', () => { + it('should go to edit mileage page if category is mileage and canEdit is true', () => { + component.canEdit$ = of(true); component.erpt$ = of(expectedAllReports[0]); - component.canEditExpense = () => true; fixture.detectChanges(); component.goToTransaction({ expense: mileageExpense, @@ -757,8 +757,7 @@ describe('MyViewReportPage', () => { ]); }); - it('should go to view per diem page', () => { - spyOn(component, 'canEditExpense').and.returnValue(false); + it('should go to view per diem page if category is per diem and canEdit is false', () => { component.goToTransaction({ expense: perDiemExpense, expenseIndex: 0, @@ -779,13 +778,12 @@ describe('MyViewReportPage', () => { view: ExpenseView.individual, }, ]); - expect(component.canEditExpense).toHaveBeenCalledOnceWith(perDiemExpense.state); }); - it('should go to edit per diem page', () => { + it('should go to edit per diem page if category is per diem and canEdit is true', () => { + component.canEdit$ = of(true); component.erpt$ = of(expectedAllReports[0]); - component.canEditExpense = () => true; fixture.detectChanges(); component.goToTransaction({ expense: perDiemExpense, @@ -872,32 +870,6 @@ describe('MyViewReportPage', () => { expect(trackingService.clickViewReportInfo).toHaveBeenCalledOnceWith({ view: ExpenseView.individual }); }); - describe('canEditTxn():', () => { - it('should show whether the user can edit txn', () => { - component.canEdit$ = of(true); - fixture.detectChanges(); - - const result = component.canEditExpense(ExpenseState.DRAFT); - expect(result).toBeTrue(); - }); - - it('should show that the user cannot edit the txn', () => { - component.canEdit$ = of(false); - fixture.detectChanges(); - - const result = component.canEditExpense(ExpenseState.APPROVED); - expect(result).toBeFalse(); - }); - - it('should show that the user cannot edit the txn if state does not match', () => { - component.canEdit$ = of(true); - fixture.detectChanges(); - - const result = component.canEditExpense(ExpenseState.APPROVED); - expect(result).toBeFalse(); - }); - }); - describe('segmentChanged():', () => { it('should change segment value', () => { component.segmentChanged({ diff --git a/src/app/fyle/my-view-report/my-view-report.page.ts b/src/app/fyle/my-view-report/my-view-report.page.ts index f81732274a..dd96f56fa5 100644 --- a/src/app/fyle/my-view-report/my-view-report.page.ts +++ b/src/app/fyle/my-view-report/my-view-report.page.ts @@ -29,7 +29,6 @@ import { OrgSettings } from 'src/app/core/models/org-settings.model'; import { Approver } from 'src/app/core/models/v1/approver.model'; import { ExtendedOrgUser } from 'src/app/core/models/extended-org-user.model'; import { ExpensesService } from 'src/app/core/services/platform/v1/spender/expenses.service'; -import { ExpenseState } from 'src/app/core/models/expense-state.enum'; import { AddExpensesToReportComponent } from './add-expenses-to-report/add-expenses-to-report.component'; import { EditReportNamePopoverComponent } from './edit-report-name-popover/edit-report-name-popover.component'; import { ShareReportComponent } from './share-report/share-report.component'; @@ -246,6 +245,7 @@ export class MyViewReportPage { const actions$ = this.reportService.actions(this.reportId).pipe(shareReplay(1)); this.canEdit$ = actions$.pipe(map((actions) => actions.can_edit)); + this.canDelete$ = actions$.pipe(map((actions) => actions.can_delete)); this.canResubmitReport$ = actions$.pipe(map((actions) => actions.can_resubmit)); @@ -413,15 +413,8 @@ export class MyViewReportPage { }); } - goToTransaction({ expense, expenseIndex }: { expense: Expense; expenseIndex: number }): void { - const canEdit = this.canEditExpense(expense.state); - let category: string; - - if (expense.category) { - category = expense.category.name && expense.category.name.toLowerCase(); - } - - let route: string; + getTransactionRoute(category: string, canEdit: boolean): string { + let route = ''; if (category === 'mileage') { route = '/enterprise/view_mileage'; @@ -439,29 +432,44 @@ export class MyViewReportPage { route = '/enterprise/add_edit_expense'; } } - if (canEdit) { - this.erpt$.pipe(take(1)).subscribe((erpt) => + + return route; + } + + goToTransaction({ expense, expenseIndex }: { expense: Expense; expenseIndex: number }): void { + this.canEdit$.subscribe((canEdit) => { + let category: string; + + if (expense.category) { + category = expense.category.name && expense.category.name.toLowerCase(); + } + + const route = this.getTransactionRoute(category, canEdit); + + if (canEdit) { + this.erpt$.pipe(take(1)).subscribe((erpt) => + this.router.navigate([ + route, + { + id: expense.id, + navigate_back: true, + remove_from_report: erpt.rp_num_transactions > 1, + }, + ]) + ); + } else { + this.trackingService.viewExpenseClicked({ view: ExpenseView.individual, category }); this.router.navigate([ route, { id: expense.id, - navigate_back: true, - remove_from_report: erpt.rp_num_transactions > 1, + txnIds: JSON.stringify(this.reportExpenseIds), + activeIndex: expenseIndex, + view: ExpenseView.individual, }, - ]) - ); - } else { - this.trackingService.viewExpenseClicked({ view: ExpenseView.individual, category }); - this.router.navigate([ - route, - { - id: expense.id, - txnIds: JSON.stringify(this.reportExpenseIds), - activeIndex: expenseIndex, - view: ExpenseView.individual, - }, - ]); - } + ]); + } + }); } async shareReport(): Promise { @@ -511,10 +519,6 @@ export class MyViewReportPage { this.trackingService.clickViewReportInfo({ view: ExpenseView.individual }); } - canEditExpense(expenseState: ExpenseState): boolean { - return this.canEdit$ && ['DRAFT', 'COMPLETE', 'APPROVER_PENDING'].indexOf(expenseState) > -1; - } - segmentChanged(event: SegmentCustomEvent): void { if (event?.detail?.value) { this.segmentValue = parseInt(event.detail.value, 10);