feat: Implementation for connect cards part 2 #3389
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,32 @@ | ||
| import { Component, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChanges } from '@angular/core'; | ||
| import { | ||
| AbstractControl, | ||
| FormArray, | ||
| 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'; | ||
| import { CardNetworkType } from 'src/app/core/enums/card-network-type'; | ||
| import { statementUploadedCard, visaRTFCard } from 'src/app/core/mock-data/platform-corporate-card.data'; | ||
| 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 implements OnInit, OnChanges { | ||
| @Input() readOnly?: boolean = false; | ||
|
|
||
| @Input() orgSettings: OrgSettings; | ||
|
|
@@ -25,15 +41,154 @@ export class SpenderOnboardingConnectCardStepComponent { | |
|
|
||
| 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 fb: FormBuilder, | ||
| private popoverController: PopoverController | ||
| ) {} | ||
|
|
||
| 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
There was a problem hiding this comment. Ey! Don't forget to clean up those subscriptions, partner! The subscription in 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(() => {
|
||
| 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.successfulCards.length > 0) { | ||
| return ` | ||
|
There was a problem hiding this comment. why the same condition on both the if and else if? Even code rabbit did not notice this :D @bistaastha |
||
| 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 | ||
| .slice(this.cardsList.failedCards.length - 1) | ||
|
There was a problem hiding this comment. could be There was a problem hiding this comment. fixing this in follow up |
||
| .join(', ')} and ${this.cardsList.failedCards.slice(-1)}. | ||
| You can cancel and retry connecting the failed card or proceed to the next step.`; | ||
| } | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Aiyyo! We've got a duplicate condition here, machan! The first two conditions are exactly the same: if (this.cardsList.successfulCards.length > 0) {
// First block
} else if (this.cardsList.successfulCards.length > 0) {
// Second block
}This means the second block will never execute! 🎭 Here's how to fix it with style: - if (this.cardsList.successfulCards.length > 0) {
+ if (this.cardsList.successfulCards.length > 0 && this.cardsList.failedCards.length > 1) {
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.successfulCards.length > 0) {
+ } else if (this.cardsList.successfulCards.length > 0 && this.cardsList.failedCards.length === 1) { |
||
| 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
There was a problem hiding this comment. Mind it! Another subscription needs cleanup, machan! The subscription in Modify your subscription: - .subscribe(noop);
+ .pipe(takeUntil(this.destroy$))
+ .subscribe(noop);
|
||
|
|
||
| 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
There was a problem hiding this comment. 🧹 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 { | ||
| // Reactive forms are not strongly typed in Angular 13, so we need to cast the value to string | ||
| // TODO (Angular 14 >): Remove the type casting and directly use string type for the form control | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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!
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)