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

feat: Add Corporate Card task #3362

Closed
wants to merge 4 commits into from
Closed

feat: Add Corporate Card task #3362

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
a462faf
Minor
bistaastha Dec 17, 2024
88c6870
Minor
bistaastha Dec 17, 2024
a858682
Minor
bistaastha Dec 19, 2024
df00248
test: Add tests for Corporate Card task (#3370)
bistaastha Dec 19, 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
1 change: 1 addition & 0 deletions src/app/core/models/task-event.enum.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ export enum TASKEVENT {
openSentBackAdvance = 7,
mobileNumberVerification = 8,
commuteDetails = 9,
addCorporateCard = 10,
}
1 change: 1 addition & 0 deletions src/app/core/models/task-icon.enum.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ export enum TaskIcon {
MOBILE = 'phone',
LOCATION = 'location',
STARS = 'stars-filled',
CARD = 'card',
}
28 changes: 26 additions & 2 deletions src/app/core/services/tasks.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {
sentBackReportTaskSingularSample,
verifyMobileNumberTask2,
} from '../mock-data/task.data';
import { mastercardRTFCard } from '../mock-data/platform-corporate-card.data';
import { mastercardRTFCard, statementUploadedCard, visaRTFCard } from '../mock-data/platform-corporate-card.data';
import { OrgSettingsService } from './org-settings.service';
import { ExpensesService } from './platform/v1/spender/expenses.service';
import { expenseDuplicateSets } from '../mock-data/platform/v1/expense-duplicate-sets.data';
Expand All @@ -66,7 +66,6 @@ import {
expectedSentBackResponse,
expectedSentBackResponseSingularReport,
} from '../mock-data/report-stats.data';
import { expectedReportsSinglePage } from '../mock-data/platform-report.data';
import { OrgService } from './org.service';
import { orgData1 } from '../mock-data/org.data';
import { UtilityService } from './utility.service';
Expand Down Expand Up @@ -421,6 +420,30 @@ describe('TasksService', () => {
});
});

describe('getAddCorporateCardTask(): ', () => {
it('should return add corporate card task when no cards are enrolled', (done) => {
orgSettingsService.get.and.returnValue(of(orgSettingsRes));
corporateCreditCardExpenseService.getCorporateCards.and.returnValue(of([statementUploadedCard]));
const addCcSpy = spyOn(tasksService, 'mapAddCorporateCardTask');

tasksService.getAddCorporateCardTask().subscribe((tasks) => {
expect(corporateCreditCardExpenseService.getCorporateCards).toHaveBeenCalled();
expect(tasksService.mapAddCorporateCardTask).toHaveBeenCalled();
expect(addCcSpy).toHaveBeenCalledOnceWith();
done();
});
});

it('should return undefined when there are enrolled cards', (done) => {
corporateCreditCardExpenseService.getCorporateCards.and.returnValue(of([mastercardRTFCard, visaRTFCard]));
tasksService.getAddCorporateCardTask().subscribe((tasks) => {
expect(corporateCreditCardExpenseService.getCorporateCards).toHaveBeenCalled();
expect(tasks).toEqual([]);
done();
});
});
});

it('should be able to fetch advancesTaskCount', (done) => {
tasksService.advancesTaskCount$.next(10);
tasksService
Expand Down Expand Up @@ -780,6 +803,7 @@ describe('TasksService', () => {
corporateCreditCardExpenseService.getCorporateCards.and.returnValue(of([mastercardRTFCard]));
orgSettingsService.get.and.returnValue(of(orgSettingsRes));
employeesService.getCommuteDetails.and.returnValue(of(commuteDetailsResponseData));
corporateCreditCardExpenseService.getCorporateCards.and.returnValue(of([mastercardRTFCard]));
}

it('should be able to fetch tasks with no filters', (done) => {
Expand Down
44 changes: 42 additions & 2 deletions src/app/core/services/tasks.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ export class TasksService {
sentBackAdvances: this.getSentBackAdvanceTasks(),
setCommuteDetails: this.getCommuteDetailsTasks(),
teamReports: this.getTeamReportsTasks(showTeamReportTask),
addCorporateCard: this.getAddCorporateCardTask(),
}).pipe(
map(
({
Expand All @@ -330,6 +331,7 @@ export class TasksService {
teamReports,
sentBackAdvances,
setCommuteDetails,
addCorporateCard,
}) => {
this.totalTaskCount$.next(
mobileNumberVerification.length +
Expand All @@ -340,7 +342,8 @@ export class TasksService {
teamReports.length +
potentialDuplicates.length +
sentBackAdvances.length +
setCommuteDetails.length
setCommuteDetails.length +
addCorporateCard.length
);
this.expensesTaskCount$.next(draftExpenses.length + unreportedExpenses.length + potentialDuplicates.length);
this.reportsTaskCount$.next(sentBackReports.length + unsubmittedReports.length);
Expand All @@ -364,7 +367,8 @@ export class TasksService {
.concat(unsubmittedReports)
.concat(unreportedExpenses)
.concat(teamReports)
.concat(sentBackAdvances);
.concat(sentBackAdvances)
.concat(addCorporateCard);
} else {
return this.getFilteredTaskList(filters, {
potentialDuplicates,
Expand Down Expand Up @@ -527,6 +531,24 @@ export class TasksService {
}
}

getAddCorporateCardTask(): Observable<DashboardTask[]> {
return forkJoin([this.orgSettingsService.get(), this.corporateCreditCardExpenseService.getCorporateCards()]).pipe(
map(([orgSettings, cards]) => {
const isRtfEnabled =
(orgSettings.visa_enrollment_settings.allowed && orgSettings.visa_enrollment_settings.enabled) ||
(orgSettings.mastercard_enrollment_settings.allowed && orgSettings.mastercard_enrollment_settings.enabled);
const isCCCEnabled =
orgSettings.corporate_credit_card_settings.allowed && orgSettings.corporate_credit_card_settings.enabled;
const rtfCards = cards.filter((card) => card.is_visa_enrolled || card.is_mastercard_enrolled);
if (isRtfEnabled && isCCCEnabled && rtfCards.length === 0) {
return this.mapAddCorporateCardTask();
bistaastha marked this conversation as resolved.
Show resolved Hide resolved
} else {
return [] as DashboardTask[];
}
})
);
}

getPotentialDuplicatesTasks(): Observable<DashboardTask[]> {
return this.expensesService.getDuplicateSets().pipe(
map((duplicateSets) => {
Expand Down Expand Up @@ -560,6 +582,24 @@ export class TasksService {
return task;
}

mapAddCorporateCardTask(): DashboardTask[] {
const task = [
{
hideAmount: true,
header: 'Add Corporate Card',
subheader: 'Add your corporate card to track expenses.',
icon: TaskIcon.CARD,
ctas: [
{
content: 'Add Card',
event: TASKEVENT.addCorporateCard,
},
],
},
];
return task;
}
Comment on lines +585 to +601
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Type safety is the superstar of clean code, thalaiva!

The implementation works but let's make it more robust with proper typing.

Add type safety with this style:

mapAddCorporateCardTask(): DashboardTask[] {
+ const task: DashboardTask[] = [
-  const task = [
    {
      hideAmount: true,
      header: 'Add Corporate Card',
      subheader: 'Add your corporate card to track expenses.',
      icon: TaskIcon.CARD,
      ctas: [
        {
+         content: 'Add Card' as const,
          event: TASKEVENT.addCorporateCard,
        },
      ],
    },
  ];
  return task;
}
📝 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
mapAddCorporateCardTask(): DashboardTask[] {
const task = [
{
hideAmount: true,
header: 'Add Corporate Card',
subheader: 'Add your corporate card to track expenses.',
icon: TaskIcon.CARD,
ctas: [
{
content: 'Add Card',
event: TASKEVENT.addCorporateCard,
},
],
},
];
return task;
}
mapAddCorporateCardTask(): DashboardTask[] {
const task: DashboardTask[] = [
{
hideAmount: true,
header: 'Add Corporate Card',
subheader: 'Add your corporate card to track expenses.',
icon: TaskIcon.CARD,
ctas: [
{
content: 'Add Card' as const,
event: TASKEVENT.addCorporateCard,
},
],
},
];
return task;
}


mapPotentialDuplicatesTasks(duplicateSets: string[][]): DashboardTask[] {
if (duplicateSets.length > 0) {
const duplicateIds = duplicateSets.reduce((acc, curVal) => acc.concat(curVal), []);
Expand Down
8 changes: 8 additions & 0 deletions src/app/fyle/dashboard/tasks/tasks-1.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import {
taskCtaData8,
taskCtaData9,
} from 'src/app/core/mock-data/task-cta.data';
import { OrgSettingsService } from 'src/app/core/services/org-settings.service';
import { orgSettingsPendingRestrictions } from 'src/app/core/mock-data/org-settings.data';

export function TestCases1(getTestBed) {
return describe('test case set 1', () => {
Expand All @@ -60,6 +62,7 @@ export function TestCases1(getTestBed) {
let router: jasmine.SpyObj<Router>;
let activatedRoute: jasmine.SpyObj<ActivatedRoute>;
let networkService: jasmine.SpyObj<NetworkService>;
let orgSettingsService: jasmine.SpyObj<OrgSettingsService>;

beforeEach(waitForAsync(() => {
const TestBed = getTestBed();
Expand All @@ -79,13 +82,18 @@ export function TestCases1(getTestBed) {
router = TestBed.inject(Router) as jasmine.SpyObj<Router>;
activatedRoute = TestBed.inject(ActivatedRoute) as jasmine.SpyObj<ActivatedRoute>;
networkService = TestBed.inject(NetworkService) as jasmine.SpyObj<NetworkService>;
orgSettingsService = TestBed.inject(OrgSettingsService) as jasmine.SpyObj<OrgSettingsService>;
}));

it('should create', () => {
expect(component).toBeTruthy();
});

it('ngOnInit(): should call setupNetworkWatcher once', () => {
orgSettingsService.get.and.returnValue(of(orgSettingsPendingRestrictions));
component.isVisaRTFEnabled$ = of(true);
component.isMastercardRTFEnabled$ = of(true);
component.isYodleeEnabled$ = of(true);
Comment on lines +93 to +96
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Style ah style! Combine these observables for better readability, macha!

These observables can be combined into a single setup block for better maintainability.

Apply this refactor:

-orgSettingsService.get.and.returnValue(of(orgSettingsPendingRestrictions));
-component.isVisaRTFEnabled$ = of(true);
-component.isMastercardRTFEnabled$ = of(true);
-component.isYodleeEnabled$ = of(true);
+// Setup all observables in one block
+beforeEach(() => {
+  orgSettingsService.get.and.returnValue(of(orgSettingsPendingRestrictions));
+  component.isVisaRTFEnabled$ = of(true);
+  component.isMastercardRTFEnabled$ = of(true);
+  component.isYodleeEnabled$ = of(true);
+});
📝 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
orgSettingsService.get.and.returnValue(of(orgSettingsPendingRestrictions));
component.isVisaRTFEnabled$ = of(true);
component.isMastercardRTFEnabled$ = of(true);
component.isYodleeEnabled$ = of(true);
// Setup all observables in one block
beforeEach(() => {
orgSettingsService.get.and.returnValue(of(orgSettingsPendingRestrictions));
component.isVisaRTFEnabled$ = of(true);
component.isMastercardRTFEnabled$ = of(true);
component.isYodleeEnabled$ = of(true);
});

spyOn(component, 'setupNetworkWatcher');
fixture.detectChanges();

Expand Down
51 changes: 50 additions & 1 deletion src/app/fyle/dashboard/tasks/tasks-2.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ComponentFixture, fakeAsync, tick, waitForAsync } from '@angular/core/testing';
import { ModalController } from '@ionic/angular';
import { ModalController, PopoverController } from '@ionic/angular';

import { TasksComponent } from './tasks.component';
import { TasksService } from 'src/app/core/services/tasks.service';
Expand Down Expand Up @@ -48,6 +48,12 @@ import { apiEouRes } from 'src/app/core/mock-data/extended-org-user.data';
import { OrgService } from 'src/app/core/services/org.service';
import { orgData1 } from 'src/app/core/mock-data/org.data';
import { FyOptInComponent } from 'src/app/shared/components/fy-opt-in/fy-opt-in.component';
import { Component, Input } from '@angular/core';
import { AddCorporateCardComponent } from '../../manage-corporate-cards/add-corporate-card/add-corporate-card.component';
import { By } from '@angular/platform-browser';
import { OrgUserSettingsService } from 'src/app/core/services/org-user-settings.service';
import { CorporateCreditCardExpenseService } from 'src/app/core/services/corporate-credit-card-expense.service';
import { OrgSettingsService } from 'src/app/core/services/org-settings.service';

export function TestCases2(getTestBed) {
return describe('test case set 2', () => {
Expand All @@ -71,6 +77,10 @@ export function TestCases2(getTestBed) {
let spenderReportsService: jasmine.SpyObj<SpenderReportsService>;
let approverReportsService: jasmine.SpyObj<ApproverReportsService>;
let orgService: jasmine.SpyObj<OrgService>;
let popoverController: jasmine.SpyObj<PopoverController>;
let orgUserSettingsService: jasmine.SpyObj<OrgUserSettingsService>;
let corporateCreditCardExpenseService: jasmine.SpyObj<CorporateCreditCardExpenseService>;
let orgSettingsService: jasmine.SpyObj<OrgSettingsService>;

beforeEach(waitForAsync(() => {
const TestBed = getTestBed();
Expand All @@ -94,6 +104,14 @@ export function TestCases2(getTestBed) {
spenderReportsService = TestBed.inject(SpenderReportsService) as jasmine.SpyObj<SpenderReportsService>;
approverReportsService = TestBed.inject(ApproverReportsService) as jasmine.SpyObj<ApproverReportsService>;
orgService = TestBed.inject(OrgService) as jasmine.SpyObj<OrgService>;
popoverController = TestBed.inject(PopoverController) as jasmine.SpyObj<PopoverController>;
orgUserSettingsService = TestBed.inject(OrgUserSettingsService) as jasmine.SpyObj<OrgUserSettingsService>;
corporateCreditCardExpenseService = TestBed.inject(
CorporateCreditCardExpenseService
) as jasmine.SpyObj<CorporateCreditCardExpenseService>;
let addCardPopoverSpy: jasmine.SpyObj<HTMLIonPopoverElement>;
popoverController.create.and.returnValues(Promise.resolve(addCardPopoverSpy));
orgSettingsService = TestBed.inject(OrgSettingsService) as jasmine.SpyObj<OrgSettingsService>;
}));

describe('init():', () => {
Expand Down Expand Up @@ -182,6 +200,37 @@ export function TestCases2(getTestBed) {
});
});

it('onAddCorporateCardClick(): should open card popover', fakeAsync(() => {
// Mock the observables
component.isVisaRTFEnabled$ = of(true);
component.isMastercardRTFEnabled$ = of(true);
component.isYodleeEnabled$ = of(true);

// Mock the PopoverController
const addCardPopoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
addCardPopoverSpy.present.and.resolveTo();
addCardPopoverSpy.onDidDismiss.and.resolveTo({});

popoverController.create.and.resolveTo(addCardPopoverSpy);

// Call the method
component.onAddCorporateCardClick();
tick();

// Assert popover creation
expect(popoverController.create).toHaveBeenCalledOnceWith({
component: AddCorporateCardComponent,
cssClass: 'fy-dialog-popover',
componentProps: {
isVisaRTFEnabled: true,
isMastercardRTFEnabled: true,
isYodleeEnabled: true,
},
});

expect(addCardPopoverSpy.present).toHaveBeenCalledTimes(1);
}));
Comment on lines +203 to +232
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Mind-blowing test, but let's make it more powerful!

The test covers the happy path, but we should also test error scenarios and dismissal handling.

Add these additional test cases:

it('onAddCorporateCardClick(): should handle popover dismissal', fakeAsync(() => {
  component.isVisaRTFEnabled$ = of(true);
  component.isMastercardRTFEnabled$ = of(true);
  component.isYodleeEnabled$ = of(true);

  const addCardPopoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
  addCardPopoverSpy.present.and.resolveTo();
  addCardPopoverSpy.onDidDismiss.and.resolveTo({ data: { success: true } });

  popoverController.create.and.resolveTo(addCardPopoverSpy);

  component.onAddCorporateCardClick();
  tick();

  expect(addCardPopoverSpy.onDidDismiss).toHaveBeenCalledTimes(1);
}));

it('onAddCorporateCardClick(): should handle popover creation failure', fakeAsync(() => {
  component.isVisaRTFEnabled$ = of(true);
  component.isMastercardRTFEnabled$ = of(true);
  component.isYodleeEnabled$ = of(true);

  popoverController.create.and.rejectWith(new Error('Failed to create popover'));

  component.onAddCorporateCardClick();
  tick();

  expect(popoverController.create).toHaveBeenCalledTimes(1);
}));


it('onMobileNumberVerificationTaskClick(): should open opt in modal', fakeAsync(() => {
authService.getEou.and.resolveTo(apiEouRes);
const optInModalSpy = jasmine.createSpyObj('optInModal', ['present', 'onWillDismiss']);
Expand Down
13 changes: 12 additions & 1 deletion src/app/fyle/dashboard/tasks/tasks.component.setup.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TestBed } from '@angular/core/testing';
import { IonicModule, ModalController } from '@ionic/angular';
import { IonicModule, ModalController, PopoverController } from '@ionic/angular';

import { TasksComponent } from './tasks.component';
import { TasksService } from 'src/app/core/services/tasks.service';
Expand All @@ -24,6 +24,8 @@ import { OrgSettingsService } from 'src/app/core/services/org-settings.service';
import { SpenderReportsService } from 'src/app/core/services/platform/v1/spender/reports.service';
import { ApproverReportsService } from 'src/app/core/services/platform/v1/approver/reports.service';
import { OrgService } from 'src/app/core/services/org.service';
import { OrgUserSettingsService } from 'src/app/core/services/org-user-settings.service';
import { CorporateCreditCardExpenseService } from 'src/app/core/services/corporate-credit-card-expense.service';

describe('TasksComponent', () => {
const getTestBed = () => {
Expand Down Expand Up @@ -65,6 +67,10 @@ describe('TasksComponent', () => {
'getAllReportsByParams',
]);
const approverReportsServiceSpy = jasmine.createSpyObj('ApproverReportsService', ['getAllReportsByParams']);
const orgUserSettingsServiceSpy = jasmine.createSpyObj('OrgUserSettingsService', ['get']);
const corporateCreditCardExpenseServiceSpy = jasmine.createSpyObj('CorporateCreditCardExpenseService', [
'getCorporateCards',
]);
const matSnackBarSpy = jasmine.createSpyObj('MatSnackBar', ['openFromComponent']);
const snackbarPropertiesSpy = jasmine.createSpyObj('SnackbarPropertiesService', ['setSnackbarProperties']);
const orgServiceSpy = jasmine.createSpyObj('OrgService', ['getPrimaryOrg', 'getCurrentOrg']);
Expand All @@ -77,8 +83,10 @@ describe('TasksComponent', () => {
},
},
};
const popoverControllerSpy = jasmine.createSpyObj('PopoverController', ['create', 'onDidDismiss']);
const networkServiceSpy = jasmine.createSpyObj('NetworkService', ['connectivityWatcher', 'isOnline']);

const addCardPopoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Mind it! The addCardPopoverSpy is declared but not used properly.

The spy is declared but never initialized, which could lead to undefined behavior in tests.

Apply this fix:

-const addCardPopoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
+const addCardPopoverSpy = jasmine.createSpyObj<HTMLIonPopoverElement>('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
+addCardPopoverSpy.present.and.returnValue(Promise.resolve());
+addCardPopoverSpy.onDidDismiss.and.returnValue(Promise.resolve({}));
📝 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
const addCardPopoverSpy = jasmine.createSpyObj('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
const addCardPopoverSpy = jasmine.createSpyObj<HTMLIonPopoverElement>('HTMLIonPopoverElement', ['present', 'onDidDismiss']);
addCardPopoverSpy.present.and.returnValue(Promise.resolve());
addCardPopoverSpy.onDidDismiss.and.returnValue(Promise.resolve({}));

TestBed.configureTestingModule({
declarations: [TasksComponent],
imports: [IonicModule.forRoot(), RouterTestingModule],
Expand All @@ -102,6 +110,9 @@ describe('TasksComponent', () => {
{ provide: SpenderReportsService, useValue: spenderReportsServiceSpy },
{ provide: ApproverReportsService, useValue: approverReportsServiceSpy },
{ provide: OrgService, useValue: orgServiceSpy },
{ provide: PopoverController, useValue: popoverControllerSpy },
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Thalaiva says: Fix the spy setup!

The returnValues is used incorrectly with an undefined spy.

Apply this fix:

-popoverController.create.and.returnValues(Promise.resolve(addCardPopoverSpy));
+popoverController.create.and.returnValue(Promise.resolve(addCardPopoverSpy));

{ provide: OrgUserSettingsService, useValue: orgUserSettingsServiceSpy },
{ provide: CorporateCreditCardExpenseService, useValue: corporateCreditCardExpenseServiceSpy },
],
schemas: [NO_ERRORS_SCHEMA],
}).compileComponents();
Expand Down
Loading
Loading