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.

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: Implementation for connect cards part 2 #3389

Open
wants to merge 13 commits into
base: FYLE-86cx2t82k-base-feature-branch
Choose a base branch
from
4 changes: 4 additions & 0 deletions src/app/core/models/popover-cards-list.model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export interface PopoverCardsList {
successfulCards: string[];
failedCards: string[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,84 +5,102 @@
This will help you bring your card transactions into Fyle as expenses instantly.
</div>

<div class="connect-card__input-label">
<span>Corporate card</span>
</div>

<div
class="connect-card__input-inner-container"
[ngClass]="{ 'connect-card__input-inner-container--error': cardForm.touched && cardForm.invalid }"
>
<input
class="smartlook-show connect-card__card-number-input pl-0"
inputmode="numeric"
[formControl]="cardForm"
mask="0000 0000 0000 0000"
data-testid="card-number-input"
appAutofocus
[timeout]="500"
required
placeholder="Enter corporate card number"
/>
<form formGroup="fg" *ngIf="fg">
<div *ngIf="enrollableCards?.length > 0">
<div *ngFor="let card of enrollableCards; let i = index">
<div class="connect-card__input-label">
<span>Corporate card</span>
</div>

<ion-icon
*ngIf="!cardType || cardType === cardNetworkTypes.OTHERS"
src="../../../../assets/svg/card.svg"
class="connect-card__input-default-icon"
data-testid="default-icon"
></ion-icon>
<div
class="connect-card__input-inner-container"
[ngClass]="{ 'connect-card__input-inner-container--error': false }"
>
<input
class="smartlook-show connect-card__card-number-input pl-0"
inputmode="numeric"
placeholder="xxxx xxxx xxxx"
data-testid="card-number-input"
appAutofocus
[timeout]="500"
[formControlName]="'card_number_' + i"
required
(input)="onCardNumberUpdate(card, 'card_number_' + i)"
/>

<img
*ngIf="cardType === cardNetworkTypes.VISA"
src="../../../../assets/images/visa-logo.png"
class="connect-card__input-visa-icon"
data-testid="visa-icon"
/>
<div>
{{ card?.card_number || '' }}
</div>

<img
*ngIf="cardType === cardNetworkTypes.MASTERCARD"
src="../../../../assets/images/mastercard-logo.png"
class="connect-card__input-mastercard-icon"
data-testid="mastercard-icon"
/>
</div>
<ion-icon
*ngIf="cardValuesMap?.[card.id].card_type === 'Others'"
src="../../../../assets/svg/card.svg"
class="connect-card__input-default-icon"
data-testid="default-icon"
></ion-icon>

<div class="connect-card__input-error-space"></div>
<img
*ngIf="cardValuesMap?.[card.id].card_type === 'Visa'"
src="../../../../assets/images/visa-logo.png"
class="connect-card__input-visa-icon"
data-testid="visa-icon"
/>

<div *ngIf="cardForm.touched && cardForm.invalid" class="connect-card__input-errors" data-testid="error-message">
<span *ngIf="cardForm.errors.invalidCardNumber">Please enter a valid card number.</span>
<img
*ngIf="cardValuesMap?.[card.id].card_type === 'Mastercard'"
src="../../../../assets/images/mastercard-logo.png"
class="connect-card__input-mastercard-icon"
data-testid="mastercard-icon"
/>
</div>

<ng-container *ngIf="cardForm.errors.invalidCardNetwork">
<span *ngIf="isVisaRTFEnabled && isMastercardRTFEnabled; else visaOnlyOrg"
>Enter a valid Visa or Mastercard number. If you have other cards, please contact your admin.</span
>
<div class="connect-card__input-error-space"></div>

<ng-template #visaOnlyOrg>
<!-- Check if only visa is enabled -->
<span *ngIf="cardForm.errors.invalidCardNetwork && isVisaRTFEnabled; else mastercardOnlyOrg"
>Enter a valid Visa number. If you have other cards, please contact your admin.</span
<div
*ngIf="fg.controls['card_number_' + i]?.touched && fg.controls['card_number_' + i]?.invalid"
class="add-corporate-card__input-errors"
data-testid="error-message"
>
</ng-template>
<span *ngIf="fg.controls['card_number_' + i]?.errors.invalidCardNumber"
>Please enter a valid card number.</span
>

<ng-template #mastercardOnlyOrg>
<!-- Check if only mastercard is enabled -->
<span *ngIf="cardForm.errors.invalidCardNetwork && isMastercardRTFEnabled"
>Enter a valid Mastercard number. If you have other cards, please contact your admin.</span
>
</ng-template>
</ng-container>
<ng-container *ngIf="fg.controls['card_number_' + i]?.errors.invalidCardNetwork">
<span *ngIf="isVisaRTFEnabled && isMastercardRTFEnabled; else visaOnlyOrg"
>Enter a valid Visa or Mastercard number. If you have other cards, please contact your admin.</span
>

<span *ngIf="cardForm.errors.enrollmentError">
{{ enrollmentFailureMessage }}
</span>
</div>
<ng-template #visaOnlyOrg>
<!-- Check if only visa is enabled -->
<span
*ngIf="
fg.controls['card_number_' + i]?.errors.invalidCardNetwork && isVisaRTFEnabled;
else mastercardOnlyOrg
"
>Enter a valid Visa number. If you have other cards, please contact your admin.</span
>
</ng-template>

<ng-template #mastercardOnlyOrg>
<!-- Check if only mastercard is enabled -->
<span *ngIf="fg.controls['card_number_' + i]?.errors.invalidCardNetwork && isMastercardRTFEnabled"
>Enter a valid Mastercard number. If you have other cards, please contact your admin.</span
>
</ng-template>
</ng-container>
</div>
</div>
</div>
<div *ngIf="enrollableCards.length === 0"></div>
</form>
</div>
<div class="connect-card__primary-cta-container">
<ion-button
class="btn-primary connect-card__primary-cta"
fill="clear"
aria-label="Navigate back to sign in page"
role="button"
(click)="enrollCards()"
>
Continue
</ion-button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
justify-content: flex-end;
}

&__card-number-input {
width: fit-content !important;
margin-right: 24px;
&::placeholder {
word-spacing: 24px;
}
}

&__heading {
color: $black;
font-size: 20px;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,32 @@
import { Component, EventEmitter, Input, Output } from '@angular/core';
import { AbstractControl, FormControl, ValidationErrors } from '@angular/forms';
import { Component, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChanges } from '@angular/core';
import {
AbstractControl,
FormArray,

Check failure on line 4 in src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts

View workflow job for this annotation

GitHub Actions / Run linters

'FormArray' is defined but never used
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Aiyyo, Unused Imports Detected, Da!

FormArray, finalize, statementUploadedCard, and visaRTFCard are not used anywhere. Time to remove them and keep your code dancing light, machan!

- import {
-   FormArray,
-   ...
- } from '@angular/forms';

- import { catchError, concatMap, finalize, from, ... } from 'rxjs';

- import { statementUploadedCard, visaRTFCard } from 'src/app/core/mock-data/platform-corporate-card.data';

Also applies to: 12-12, 14-14

🧰 Tools
🪛 GitHub Check: Run linters

[failure] 4-4:
'FormArray' is defined but never used

🪛 eslint

[error] 4-4: 'FormArray' is defined but never used.

(@typescript-eslint/no-unused-vars)

FormBuilder,
FormControl,
FormGroup,
ValidationErrors,
Validators,
} from '@angular/forms';
import { PopoverController } from '@ionic/angular';
import { catchError, concatMap, finalize, from, map, noop, of, switchMap, tap } from 'rxjs';

Check failure on line 12 in src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts

View workflow job for this annotation

GitHub Actions / Run linters

'finalize' is defined but never used
import { CardNetworkType } from 'src/app/core/enums/card-network-type';
import { statementUploadedCard, visaRTFCard } from 'src/app/core/mock-data/platform-corporate-card.data';

Check failure on line 14 in src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts

View workflow job for this annotation

GitHub Actions / Run linters

'statementUploadedCard' is defined but never used

Check failure on line 14 in src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts

View workflow job for this annotation

GitHub Actions / Run linters

'visaRTFCard' is defined but never used
import { OrgSettings } from 'src/app/core/models/org-settings.model';
import { OverlayResponse } from 'src/app/core/models/overlay-response.modal';
import { PlatformCorporateCard } from 'src/app/core/models/platform/platform-corporate-card.model';
import { PopoverCardsList } from 'src/app/core/models/popover-cards-list.model';
import { CorporateCreditCardExpenseService } from 'src/app/core/services/corporate-credit-card-expense.service';
import { RealTimeFeedService } from 'src/app/core/services/real-time-feed.service';
import { PopupAlertComponent } from 'src/app/shared/components/popup-alert/popup-alert.component';

@Component({
selector: 'app-spender-onboarding-connect-card-step',
templateUrl: './spender-onboarding-connect-card-step.component.html',
styleUrls: ['./spender-onboarding-connect-card-step.component.scss'],
})
export class SpenderOnboardingConnectCardStepComponent {

export class SpenderOnboardingConnectCardStepComponent implements OnInit, OnChanges {
@Input() readOnly?: boolean = false;

@Input() orgSettings: OrgSettings;
Expand All @@ -25,13 +41,153 @@

cardType = CardNetworkType;

enrollableCards: PlatformCorporateCard[];

cardValuesMap: Record<string, { card_type: string; card_number: string }> = {};

rtfCardType: CardNetworkType;

cardsList: PopoverCardsList = {
successfulCards: [],
failedCards: [],
};

fg: FormGroup;

constructor(
private corporateCreditCardExpensesService: CorporateCreditCardExpenseService,
private realTimeFeedService: RealTimeFeedService
private realTimeFeedService: RealTimeFeedService,
private fb: FormBuilder,
private popoverController: PopoverController
) {}

ionViewWillEnter(): void {
this.cardForm = new FormControl('', [this.cardNumberValidator.bind(this), this.cardNetworkValidator.bind(this)]);
enrollCards(): void {
const cards = this.enrollableCards;
from(cards)
.pipe(
concatMap((card) =>
this.realTimeFeedService.enroll(card.card_number, card.id).pipe(
map(() => {
this.cardsList.successfulCards.push(`**** ${card.card_number.slice(-4)}`);
}),
catchError(() => {
this.cardsList.failedCards.push(`**** ${card.card_number.slice(-4)}`);
return of(null);
})
)
)
)
.subscribe(() => {
if (this.cardsList.failedCards.length > 0) {
this.showErrorPopover();
} else {
this.isStepCompleted.emit(true);
}
});
}
Comment on lines +64 to +87
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ey! Don't forget to clean up those subscriptions, partner!

The subscription in enrollCards() needs proper cleanup to prevent memory leaks. Let's handle it with style!

Add this to your class:

+ private destroy$ = new Subject<void>();

  ngOnDestroy(): void {
+   this.destroy$.next();
+   this.destroy$.complete();
  }

Then modify your subscription:

- .subscribe(() => {
+ .pipe(takeUntil(this.destroy$))
+ .subscribe(() => {

Committable suggestion skipped: line range outside the PR's diff.


generateMessage(): string {
if (this.cardsList.successfulCards.length > 0) {
return 'We ran into an issue while processing your request. You can cancel and retry connecting the failed card or proceed to the next step.';
} else if (this.cardsList.failedCards.length > 0) {
return `
We ran into an issue while processing your request for the card ${this.cardsList.failedCards[0]}.
You can cancel and retry connecting the failed card or proceed to the next step.`;
} else {
return `
We ran into an issue while processing your request for the card ${this.cardsList.failedCards
Copy link
Contributor

Choose a reason for hiding this comment

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

could be for the card or for the cards no? better be for the card(s)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixing this in follow up

.slice(this.cardsList.failedCards.length - 1)
.join(', ')} and ${this.cardsList.failedCards.slice(-1)}.
You can cancel and retry connecting the failed card or proceed to the next step.`;
}
}

showErrorPopover(): void {
const errorPopover = this.popoverController.create({
componentProps: {
title: 'Status summary',
message: this.generateMessage(),
primaryCta: {
text: 'Proceed anyway',
action: 'close',
},
secondaryCta: {
text: 'Cancel',
action: 'cancel',
},
cardsList: this.cardsList.successfulCards.length > 0 ? this.cardsList : {},
},
component: PopupAlertComponent,
cssClass: 'pop-up-in-center',
});

from(errorPopover)
.pipe(
tap((errorPopover) => errorPopover.present()),
switchMap((errorPopover) => errorPopover.onWillDismiss()),
map((response: OverlayResponse<{ action?: string }>) => {
if (response?.data?.action === 'close') {
this.isStepCompleted.emit(true);
}
})
)
.subscribe(noop);
Comment on lines +105 to +134
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! Another subscription needs cleanup, machan!

The subscription in showErrorPopover() also needs proper cleanup. Style is in the details! 😎

Modify your subscription:

- .subscribe(noop);
+ .pipe(takeUntil(this.destroy$))
+ .subscribe(noop);

Committable suggestion skipped: line range outside the PR's diff.

}

ngOnChanges(changes: SimpleChanges): void {
if (changes.orgSettings.currentValue) {
this.isVisaRTFEnabled = this.orgSettings.visa_enrollment_settings.enabled;
this.isMastercardRTFEnabled = this.orgSettings.mastercard_enrollment_settings.enabled;
}
}

ngOnInit(): void {
this.fg = this.fb.group({});
this.corporateCreditCardExpensesService
.getCorporateCards()
.pipe(
map((corporateCards) => {
// Filter enrollable cards
this.enrollableCards = corporateCards.filter((card) => card.data_feed_source === 'STATEMENT_UPLOAD');

// Add form controls for each enrollable card
this.enrollableCards.forEach((card, index) => {
const controlName = `card_number_${index}`;
this.cardValuesMap[card.id] = {
card_number: card.card_number,
card_type: CardNetworkType.OTHERS,
};
this.fg.addControl(
controlName,
this.fb.control('', [
Validators.required,
Validators.maxLength(12),
this.cardNumberValidator.bind(this),
this.cardNetworkValidator.bind(this),
])
);
});
})
)
.subscribe();
}
Comment on lines +144 to +173
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Looking sharp! But let's make it TypeScript-style strong, partner! 💪

The form initialization is solid, but we can make it even better with proper TypeScript types for the form controls.

+ interface CardFormControls {
+   [key: `card_number_${number}`]: FormControl<string>;
+ }

- fg: FormGroup;
+ fg: FormGroup<CardFormControls>;

Also, consider using the new typed forms feature when upgrading to Angular 14+:

this.fg = this.fb.group<CardFormControls>({});


onCardNumberUpdate(card: PlatformCorporateCard, inputControlName: string): void {
this.formatCardNumber(this.fg.controls[inputControlName]);
this.cardValuesMap[card.id].card_type = this.realTimeFeedService.getCardTypeFromNumber(
this.cardValuesMap[card.id].card_number
);
}

formatCardNumber(input: AbstractControl): void {
// Remove all non-numeric characters
let value = (input.value as string).replace(/\D/g, '');

// Format the value in groups of 4
value = value.replace(/(\d{4})(?=\d)/g, '$1 ');

// Set the formatted value back to the input
input.setValue(value);
}

private cardNumberValidator(control: AbstractControl): ValidationErrors {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,12 @@
</div>
<div class="spender-onboarding__skip-cta" (click)="skipConnectCardOnboardingStep()">Skip</div>
</div>
<div class="spender-onboarding__component-container">
<app-spender-onboarding-connect-card-step
[orgSettings]="orgSettings"
(isStepCompleted)="skipOnboardingStep()"
></app-spender-onboarding-connect-card-step>
</div>
Comment on lines +28 to +33
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Hey boss! Let's add some loading state magic! ✨

The component container could use a loading state to handle those micro-moments when the card data is being fetched. When you're as fast as lightning like me, every millisecond counts!

Here's a style-packed solution:

 <div class="spender-onboarding__component-container">
+  <div *ngIf="isCardDataLoading" class="spender-onboarding__loading">
+    <ion-spinner name="crescent"></ion-spinner>
+  </div>
   <app-spender-onboarding-connect-card-step
     [orgSettings]="orgSettings"
     (isStepCompleted)="skipOnboardingStep()"
+    *ngIf="!isCardDataLoading"
   ></app-spender-onboarding-connect-card-step>
 </div>

Committable suggestion skipped: line range outside the PR's diff.


</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Component } from '@angular/core';
import { forkJoin, from, map, switchMap } from 'rxjs';
import { ExtendedOrgUser } from 'src/app/core/models/extended-org-user.model';

Check failure on line 3 in src/app/fyle/spender-onboarding/spender-onboarding.page.ts

View workflow job for this annotation

GitHub Actions / Run linters

'ExtendedOrgUser' is defined but never used
import { LoaderService } from 'src/app/core/services/loader.service';
import { OrgUserService } from 'src/app/core/services/org-user.service';
import { OnboardingStep } from './models/onboarding-step.enum';
Expand Down Expand Up @@ -88,5 +88,4 @@
this.spenderOnboardingService.markSmsOptInStepAsComplete();
}
}

}
Loading
Loading