Skip to content

Commit

Permalink
fix: concurrency issue in mobile app where report is approved but edi…
Browse files Browse the repository at this point in the history 
…t expense page was opening up (#2763)
  • Loading branch information
@suyashpatil78
suyashpatil78 committed Feb 19, 2024
1 parent c14975d commit d6dfabb
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 73 deletions.
54 changes: 13 additions & 41 deletions src/app/fyle/my-view-report/my-view-report.page.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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({
Expand Down
68 changes: 36 additions & 32 deletions src/app/fyle/my-view-report/my-view-report.page.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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';
Expand All @@ -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<void> {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit d6dfabb

Please sign in to comment.