Skip to content
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

test: removing unnecessary optionals and increasing branch cov - 1 #2503

Merged
merged 7 commits into from
Oct 16, 2023
Merged

test: removing unnecessary optionals and increasing branch cov - 1 #2503

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7291672
test: removing unnecessary optionals and increasing branch cov - 1
suyashpatil78 Oct 10, 2023
881e2a6
Merge branch 'master' of github.com:fylein/fyle-mobile-app into rem-b…
suyashpatil78 Oct 10, 2023
264b62d
Merge branch 'master' of github.com:fylein/fyle-mobile-app into rem-b…
suyashpatil78 Oct 12, 2023
68d86ac
pr comments
suyashpatil78 Oct 12, 2023
d2dc941
Merge branch 'master' of github.com:fylein/fyle-mobile-app into rem-b…
suyashpatil78 Oct 13, 2023
27e57ea
test: removing unnecessary optionals and increasing branch cov - 2 (#…
suyashpatil78 Oct 16, 2023
eb4c7f1
Merge branch 'rem-branches-test' of github.com:fylein/fyle-mobile-app…
suyashpatil78 Oct 16, 2023
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
126 changes: 99 additions & 27 deletions src/app/fyle/add-edit-expense/add-edit-expense-1.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { actionSheetOptionsData } from 'src/app/core/mock-data/action-sheet-opti
import { expectedECccResponse } from 'src/app/core/mock-data/corporate-card-expense-unflattened.data';
import { costCenterApiRes1, expectedCCdata } from 'src/app/core/mock-data/cost-centers.data';
import { customFieldData1 } from 'src/app/core/mock-data/custom-field.data';
import { expenseFieldObjData } from 'src/app/core/mock-data/expense-field-obj.data';
import { expenseFieldObjData, txnFieldData } from 'src/app/core/mock-data/expense-field-obj.data';
import { txnFieldsMap2 } from 'src/app/core/mock-data/expense-fields-map.data';
import { apiExpenseRes, expenseData1 } from 'src/app/core/mock-data/expense.data';
import { categorieListRes } from 'src/app/core/mock-data/org-category-list-item.data';
Expand Down Expand Up @@ -144,11 +144,11 @@ export function TestCases1(getTestBed) {
popupService = TestBed.inject(PopupService) as jasmine.SpyObj<PopupService>;
navController = TestBed.inject(NavController) as jasmine.SpyObj<NavController>;
corporateCreditCardExpenseService = TestBed.inject(
CorporateCreditCardExpenseService,
CorporateCreditCardExpenseService
) as jasmine.SpyObj<CorporateCreditCardExpenseService>;
trackingService = TestBed.inject(TrackingService) as jasmine.SpyObj<TrackingService>;
recentLocalStorageItemsService = TestBed.inject(
RecentLocalStorageItemsService,
RecentLocalStorageItemsService
) as jasmine.SpyObj<RecentLocalStorageItemsService>;
recentlyUsedItemsService = TestBed.inject(RecentlyUsedItemsService) as jasmine.SpyObj<RecentlyUsedItemsService>;
tokenService = TestBed.inject(TokenService) as jasmine.SpyObj<TokenService>;
Expand Down Expand Up @@ -586,7 +586,7 @@ export function TestCases1(getTestBed) {
expect(transactionService.delete).toHaveBeenCalledOnceWith(expenseData1.tx_id);
expect(trackingService.deleteExpense).toHaveBeenCalledOnceWith({ Type: 'Marked Personal' });
expect(corporateCreditCardExpenseService.markPersonal).toHaveBeenCalledOnceWith(
component.corporateCreditCardExpenseGroupId,
component.corporateCreditCardExpenseGroupId
);
done();
});
Expand Down Expand Up @@ -615,7 +615,7 @@ export function TestCases1(getTestBed) {
expect(transactionService.delete).toHaveBeenCalledOnceWith(expenseData1.tx_id);
expect(trackingService.deleteExpense).toHaveBeenCalledOnceWith({ Type: 'Dismiss as Card Payment' });
expect(corporateCreditCardExpenseService.dismissCreditTransaction).toHaveBeenCalledOnceWith(
expenseData1.tx_corporate_credit_card_expense_group_id,
expenseData1.tx_corporate_credit_card_expense_group_id
);
done();
});
Expand Down Expand Up @@ -645,7 +645,7 @@ export function TestCases1(getTestBed) {
result.componentProps.deleteMethod().subscribe((res) => {
expect(res).toEqual(UndoMergeData2);
expect(transactionService.removeCorporateCardExpense).toHaveBeenCalledOnceWith(
activatedRoute.snapshot.params.id,
activatedRoute.snapshot.params.id
);
done();
});
Expand Down Expand Up @@ -675,20 +675,38 @@ export function TestCases1(getTestBed) {
expect(transactionService.getRemoveCardExpenseDialogBody).toHaveBeenCalledTimes(1);
expect(component.getRemoveCCCExpModalParams).toHaveBeenCalledOnceWith(header, body, ctaText, ctaLoadingText);
expect(popoverController.create).toHaveBeenCalledOnceWith(
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText),
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText)
);
expect(trackingService.unlinkCorporateCardExpense).toHaveBeenCalledTimes(1),
expect(component.goBack).toHaveBeenCalledOnceWith();
expect(component.showSnackBarToast).toHaveBeenCalledOnceWith(
{ message: 'Successfully removed the card details from the expense.' },
'information',
['msb-info'],
['msb-info']
);
expect(trackingService.showToastMessage).toHaveBeenCalledOnceWith({
ToastContent: 'Successfully removed the card details from the expense.',
});
}));

it('should go back to my expenses page if etxn is undefined', fakeAsync(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changes here

component.etxn$ = of(undefined);
spyOn(component, 'goBack');
transactionService.getRemoveCardExpenseDialogBody.and.returnValue('removed');
spyOn(component, 'getRemoveCCCExpModalParams');
spyOn(component, 'showSnackBarToast');

const deletePopoverSpy = jasmine.createSpyObj('deletePopover', ['present', 'onDidDismiss']);
deletePopoverSpy.onDidDismiss.and.resolveTo({ data: { status: 'success' } });

popoverController.create.and.resolveTo(deletePopoverSpy);

component.removeCorporateCardExpense();
tick(500);

expect(component.goBack).toHaveBeenCalledTimes(1);
}));

it('navigate back to report if redirected from report after removing txn', fakeAsync(() => {
const txn = { ...unflattenedTxn, tx: { ...unflattenedTxn.tx, report_id: 'rpFE5X1Pqi9P' } };
component.etxn$ = of(txn);
Expand All @@ -712,7 +730,7 @@ export function TestCases1(getTestBed) {
expect(transactionService.getRemoveCardExpenseDialogBody).toHaveBeenCalledTimes(1);
expect(component.getRemoveCCCExpModalParams).toHaveBeenCalledOnceWith(header, body, ctaText, ctaLoadingText);
expect(popoverController.create).toHaveBeenCalledOnceWith(
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText),
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText)
);
expect(trackingService.unlinkCorporateCardExpense).toHaveBeenCalledTimes(1);

Expand All @@ -725,7 +743,7 @@ export function TestCases1(getTestBed) {
expect(component.showSnackBarToast).toHaveBeenCalledOnceWith(
{ message: 'Successfully removed the card details from the expense.' },
'information',
['msb-info'],
['msb-info']
);
expect(trackingService.showToastMessage).toHaveBeenCalledOnceWith({
ToastContent: 'Successfully removed the card details from the expense.',
Expand Down Expand Up @@ -755,7 +773,7 @@ export function TestCases1(getTestBed) {
expect(transactionService.getRemoveCardExpenseDialogBody).toHaveBeenCalledTimes(1);
expect(component.getRemoveCCCExpModalParams).toHaveBeenCalledOnceWith(header, body, ctaText, ctaLoadingText);
expect(popoverController.create).toHaveBeenCalledOnceWith(
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText),
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText)
);
expect(trackingService.unlinkCorporateCardExpense).not.toHaveBeenCalled();
expect(component.showSnackBarToast).not.toHaveBeenCalled();
Expand Down Expand Up @@ -784,13 +802,13 @@ export function TestCases1(getTestBed) {
expect(transactionService.getRemoveCardExpenseDialogBody).toHaveBeenCalledTimes(1);
expect(component.getRemoveCCCExpModalParams).toHaveBeenCalledOnceWith(header, body, ctaText, ctaLoadingText);
expect(popoverController.create).toHaveBeenCalledOnceWith(
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText),
component.getRemoveCCCExpModalParams(header, body, ctaText, ctaLoadingText)
);
expect(trackingService.unlinkCorporateCardExpense).toHaveBeenCalledTimes(1);
expect(component.showSnackBarToast).toHaveBeenCalledOnceWith(
{ message: 'Successfully removed the card details from the expense.' },
'information',
['msb-info'],
['msb-info']
);
expect(trackingService.showToastMessage).toHaveBeenCalledOnceWith({
ToastContent: 'Successfully removed the card details from the expense.',
Expand All @@ -815,7 +833,7 @@ export function TestCases1(getTestBed) {
tick(500);

expect(popoverController.create).toHaveBeenCalledOnceWith(
component.getMarkDismissModalParams(getMarkDismissModalParamsData1, true),
component.getMarkDismissModalParams(getMarkDismissModalParamsData1, true)
);
expect(router.navigate).toHaveBeenCalledOnceWith(['/', 'enterprise', 'my_expenses']);
expect(component.showSnackBarToast).toHaveBeenCalledOnceWith({ message: 'Dismissed expense' }, 'information', [
Expand All @@ -841,13 +859,13 @@ export function TestCases1(getTestBed) {
tick(500);

expect(popoverController.create).toHaveBeenCalledOnceWith(
component.getMarkDismissModalParams(getMarkDismissModalParamsData2, true),
component.getMarkDismissModalParams(getMarkDismissModalParamsData2, true)
);
expect(router.navigate).toHaveBeenCalledOnceWith(['/', 'enterprise', 'my_expenses']);
expect(component.showSnackBarToast).toHaveBeenCalledOnceWith(
{ message: 'Marked expense as Personal' },
'information',
['msb-info'],
['msb-info']
);
expect(trackingService.showToastMessage).toHaveBeenCalledOnceWith({
ToastContent: 'Marked expense as Personal',
Expand All @@ -871,13 +889,13 @@ export function TestCases1(getTestBed) {
tick(500);

expect(popoverController.create).toHaveBeenCalledOnceWith(
component.getMarkDismissModalParams(getMarkDismissModalParamsData2, true),
component.getMarkDismissModalParams(getMarkDismissModalParamsData2, true)
);
expect(router.navigate).toHaveBeenCalledOnceWith(['/', 'enterprise', 'my_expenses']);
expect(component.showSnackBarToast).toHaveBeenCalledOnceWith(
{ message: 'Marked expense as Personal' },
'information',
['msb-info'],
['msb-info']
);
expect(trackingService.showToastMessage).toHaveBeenCalledOnceWith({
ToastContent: 'Marked expense as Personal',
Expand Down Expand Up @@ -972,7 +990,7 @@ export function TestCases1(getTestBed) {
of({
...orgSettingsData,
expense_settings: { ...orgSettingsData.expense_settings, split_expense_settings: { enabled: true } },
}),
})
);
component.costCenters$ = of(expectedCCdata);
projectsService.getAllActive.and.returnValue(of(projectsV1Data));
Expand Down Expand Up @@ -1001,7 +1019,61 @@ export function TestCases1(getTestBed) {
expect(projectsService.getAllActive).toHaveBeenCalledTimes(1);
expect(launchDarklyService.getVariation).toHaveBeenCalledOnceWith(
'show_project_mapped_categories_in_split_expense',
false,
false
);
});

actionSheetOptions[0].handler();
expect(component.splitExpCategoryHandler).toHaveBeenCalledTimes(1);
actionSheetOptions[1].handler();
expect(component.splitExpProjectHandler).toHaveBeenCalledTimes(1);
actionSheetOptions[2].handler();
expect(component.splitExpCostCenterHandler).toHaveBeenCalledTimes(1);
actionSheetOptions[3].handler();
expect(component.markPersonalHandler).toHaveBeenCalledTimes(1);
actionSheetOptions[4].handler();
expect(component.markDismissHandler).toHaveBeenCalledTimes(1);
actionSheetOptions[5].handler();
expect(component.removeCCCHandler).toHaveBeenCalledTimes(1);
done();
});

it('should get all action sheet options and call titleCasePipe transform method if project_id is defined', (done) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

change here

orgSettingsService.get.and.returnValue(
of({
...orgSettingsData,
expense_settings: { ...orgSettingsData.expense_settings, split_expense_settings: { enabled: true } },
})
);
component.costCenters$ = of(expectedCCdata);
projectsService.getAllActive.and.returnValue(of(projectsV1Data));
component.filteredCategories$ = of(categorieListRes);
component.txnFields$ = of(txnFieldData);
component.isCccExpense = 'tx3qHxFNgRcZ';
component.canDismissCCCE = true;
component.isCorporateCreditCardEnabled = true;
component.canRemoveCardExpense = true;
component.isExpenseMatchedForDebitCCCE = true;
spyOn(component, 'splitExpCategoryHandler');
spyOn(component, 'splitExpProjectHandler');
spyOn(component, 'splitExpCostCenterHandler');
spyOn(component, 'markPersonalHandler');
spyOn(component, 'markDismissHandler');
spyOn(component, 'removeCCCHandler');
launchDarklyService.getVariation.and.returnValue(of(true));
fixture.detectChanges();

let actionSheetOptions;

component.getActionSheetOptions().subscribe((res) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

let's name the variables with proper names based on context. res can be what is the response we get eg: like bankAccountResponse, transactionsResponse etc

Copy link
Contributor

Choose a reason for hiding this comment

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

here it can be actionSheetOptionsResponse

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, changing...

actionSheetOptions = res;
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we assigning res to actionSheetOptions? & I don't see it's usage

expect(res.length).toEqual(6);
expect(titleCasePipe.transform).toHaveBeenCalledOnceWith('Project');
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
expect(projectsService.getAllActive).toHaveBeenCalledTimes(1);
expect(launchDarklyService.getVariation).toHaveBeenCalledOnceWith(
'show_project_mapped_categories_in_split_expense',
false
);
});

Expand All @@ -1025,7 +1097,7 @@ export function TestCases1(getTestBed) {
of({
...orgSettingsData,
expense_settings: { ...orgSettingsData.expense_settings, split_expense_settings: { enabled: false } },
}),
})
);
component.costCenters$ = of(expectedCCdata);
projectsService.getAllActive.and.returnValue(of(projectsV1Data));
Expand All @@ -1047,7 +1119,7 @@ export function TestCases1(getTestBed) {
expect(projectsService.getAllActive).toHaveBeenCalledTimes(1);
expect(launchDarklyService.getVariation).toHaveBeenCalledOnceWith(
'show_project_mapped_categories_in_split_expense',
false,
false
);
done();
});
Expand All @@ -1058,7 +1130,7 @@ export function TestCases1(getTestBed) {
of({
...orgSettingsData,
expense_settings: { ...orgSettingsData.expense_settings, split_expense_settings: { enabled: false } },
}),
})
);
component.costCenters$ = of(expectedCCdata);
projectsService.getAllActive.and.returnValue(of(projectsV1Data));
Expand All @@ -1078,7 +1150,7 @@ export function TestCases1(getTestBed) {
expect(projectsService.getAllActive).toHaveBeenCalledTimes(1);
expect(launchDarklyService.getVariation).toHaveBeenCalledOnceWith(
'show_project_mapped_categories_in_split_expense',
false,
false
);
done();
});
Expand All @@ -1089,7 +1161,7 @@ export function TestCases1(getTestBed) {
of({
...orgSettingsData,
expense_settings: { ...orgSettingsData.expense_settings, split_expense_settings: { enabled: false } },
}),
})
);
component.costCenters$ = of(expectedCCdata);
projectsService.getAllActive.and.returnValue(of(projectsV1Data));
Expand All @@ -1109,7 +1181,7 @@ export function TestCases1(getTestBed) {
expect(projectsService.getAllActive).toHaveBeenCalledTimes(1);
expect(launchDarklyService.getVariation).toHaveBeenCalledOnceWith(
'show_project_mapped_categories_in_split_expense',
false,
false
);
done();
});
Expand Down Expand Up @@ -1158,7 +1230,7 @@ export function TestCases1(getTestBed) {
it('should return empty array if cost centers are not enabled', (done) => {
component.orgUserSettings$ = of(orgUserSettingsData);
orgSettingsService.get.and.returnValue(
of({ ...orgSettingsRes, cost_centers: { ...orgSettingsRes.cost_centers, enabled: false } }),
of({ ...orgSettingsRes, cost_centers: { ...orgSettingsRes.cost_centers, enabled: false } })
);
orgUserSettingsService.getAllowedCostCenters.and.returnValue(of(costCenterApiRes1));
fixture.detectChanges();
Expand Down
28 changes: 28 additions & 0 deletions src/app/fyle/add-edit-expense/add-edit-expense-3.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1232,6 +1232,34 @@ export function TestCases3(getTestBed) {
});
});

it('should get new expense observable without autofill and set currency equal to homeCurrency if recently used currency is undefined', (done) => {
orgSettingsService.get.and.returnValue(of(orgSettingsWithoutAutofill));
authService.getEou.and.resolveTo(apiEouRes);
component.orgUserSettings$ = of(orgUserSettingsData);
categoriesService.getAll.and.returnValue(of(orgCategoryData1));
component.homeCurrency$ = of('USD');
dateService.getUTCDate.and.returnValue(new Date('2023-01-24T11:30:00.000Z'));
spyOn(component, 'getInstaFyleImageData').and.returnValue(of(instaFyleData1));
recentLocalStorageItemsService.get.and.resolveTo(undefined);
component.recentlyUsedValues$ = of(recentlyUsedRes);
fixture.detectChanges();

component.getNewExpenseObservable().subscribe((res) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

expect(res).toEqual(expectedExpenseObservable3);
expect(component.source).toEqual('MOBILE_DASHCAM_SINGLE');
expect(component.isExpenseBankTxn).toBeFalse();
expect(component.instaFyleCancelled).toBeFalse();
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);

expect(authService.getEou).toHaveBeenCalledTimes(1);
expect(categoriesService.getCategoryByName).toHaveBeenCalledTimes(1);
expect(recentLocalStorageItemsService.get).toHaveBeenCalledOnceWith('recent-currency-cache');
expect(component.getInstaFyleImageData).toHaveBeenCalledTimes(1);
expect(dateService.getUTCDate).toHaveBeenCalledTimes(2);
done();
});
});

it('should get new expense observable from personal card txn and home currency does not match extracted data', (done) => {
activatedRoute.snapshot.params.personalCardTxn = JSON.stringify(apiPersonalCardTxnsRes.data);
orgSettingsService.get.and.returnValue(of(orgSettingsData));
Expand Down
6 changes: 3 additions & 3 deletions src/app/fyle/add-edit-expense/add-edit-expense.page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,15 +586,15 @@ export class AddEditExpensePage implements OnInit {
combineLatest(this.fg.controls.currencyObj.valueChanges, this.fg.controls.tax_group.valueChanges).subscribe(() => {
if (
this.fg.controls.tax_group.value &&
isNumber(taxGroupControl.value?.percentage) &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we have checked for this.fg.controls.tax_group.value, then no need to check for nullity again

isNumber(taxGroupControl.value.percentage) &&
this.fg.controls.currencyObj.value
) {
const amount =
currencyObjControl.value?.amount - currencyObjControl.value?.amount / (taxGroupControl.value?.percentage + 1);
currencyObjControl.value.amount - currencyObjControl.value.amount / (taxGroupControl.value.percentage + 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we have checked for this.fg.controls.currencyObj.value, then no need to check again


const formattedAmount = this.currencyService.getAmountWithCurrencyFraction(
amount,
currencyObjControl.value?.currency
currencyObjControl.value.currency
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above

);

this.fg.controls.tax_amount.setValue(formattedAmount);
Expand Down
Loading