-
Notifications
You must be signed in to change notification settings - Fork 15
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
test: Add tests for Connnect Card - 1 #3410
base: FYLE-86cx2t82k-tests-for-connect-card
Are you sure you want to change the base?
test: Add tests for Connnect Card - 1 #3410
Conversation
WalkthroughSuperstar style, I'll break it down! 🌟 This pull request is all about supercharging the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Sequence DiagramsequenceDiagram
participant Component as SpenderOnboardingConnectCardStep
participant Service as CorporateCreditCardExpenseService
participant Form as FormGroup
Component->>Form: Initialize Form
Component->>Service: Attempt Card Enrollment
alt Enrollment Successful
Service-->>Component: Success Response
else Enrollment Failed
Service-->>Component: Error Response
Component->>Component: Setup Error Messages
Component->>Form: Update Form State
end
Thalaiva's coding style: Precise, powerful, and always leaving a stylish mark! 😎🚀 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR description must contain a link to a ClickUp (case-insensitive) |
1 similar comment
PR description must contain a link to a ClickUp (case-insensitive) |
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts (1)
Line range hint
3-3
: Mind it! Remove the unused import, partner!The
ChangeDetectorRef
is imported but never used in the code. Let's keep our imports clean and tidy!-import { ChangeDetectorRef, Component, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChanges } from '@angular/core'; +import { Component, EventEmitter, Input, OnChanges, OnInit, Output, SimpleChanges } from '@angular/core';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.spec.ts
(1 hunks)src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts
(3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint
src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts
[error] 3-3: 'ChangeDetectorRef' is defined but never used
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (12.x)
🔇 Additional comments (5)
src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.ts (3)
91-91
: Superstar style security handling, macha!Excellent work on masking the card numbers by only using the last 4 digits in error messages. This is a top-notch security practice that protects sensitive card information!
214-215
: Mind-blowing fix for the validator binding!The
.bind(this)
ensures our validators maintain the correct context. Without this, the validators could lose their 'this' context and crash like a villain's lair!
115-115
: Style-ah implemented security, thalaiva!The same card number masking is consistently applied here too. This kind of consistency in security practices is what separates heroes from zeros!
src/app/fyle/spender-onboarding/spender-onboarding-connect-card-step/spender-onboarding-connect-card-step.component.spec.ts (2)
24-59
: What a grand entrance for our test setup, machan!The test setup is as perfectly orchestrated as a mass hero entry scene! Excellent job with:
- Proper spy creation for all services
- Complete module configuration
- Default mock responses
83-124
: These test cases are punching like my signature moves!Superb coverage of error scenarios with special focus on card number masking. The test cases verify that sensitive data is always protected, just like how I protect the innocent in my movies!
describe('enrollMultipleCards(): ', () => { | ||
it('should handle successful card enrollment', fakeAsync(() => { | ||
corporateCreditCardExpenseService.getCorporateCards.and.returnValue( | ||
of([statementUploadedCard, { ...statementUploadedCard, id: 'bacc15bbrRGWzg' }]) | ||
); | ||
component.ngOnInit(); | ||
|
||
const stepCompleteSpy = spyOn(component.isStepComplete, 'emit'); | ||
const showErrorPopoverSpy = spyOn(component, 'showErrorPopover'); | ||
const setupErrorMessagesSpy = spyOn(component, 'setupErrorMessages'); | ||
realTimeFeedService.enroll.and.returnValues(of(statementUploadedCard), of(statementUploadedCard)); | ||
component.enrollMultipleCards(component.enrollableCards); | ||
tick(); | ||
expect(component.cardsList.successfulCards).toEqual(['**** 5555', '**** 5555']); | ||
expect(component.cardsEnrolling).toBeFalse(); | ||
expect(stepCompleteSpy).toHaveBeenCalledWith(true); | ||
expect(showErrorPopoverSpy).not.toHaveBeenCalled(); | ||
expect(setupErrorMessagesSpy).not.toHaveBeenCalled(); | ||
})); | ||
|
||
it('should handle unsuccessful card enrollment', fakeAsync(() => { | ||
corporateCreditCardExpenseService.getCorporateCards.and.returnValue( | ||
of([statementUploadedCard, { ...statementUploadedCard, id: 'bacc15bbrRGWzg' }]) | ||
); | ||
component.ngOnInit(); | ||
|
||
const stepCompleteSpy = spyOn(component.isStepComplete, 'emit'); | ||
const showErrorPopoverSpy = spyOn(component, 'showErrorPopover'); | ||
realTimeFeedService.enroll.and.returnValues( | ||
of(statementUploadedCard), | ||
throwError(() => new Error('This card already exists in the system')) | ||
); | ||
component.enrollMultipleCards(component.enrollableCards); | ||
tick(); | ||
expect(component.cardsList.successfulCards).toEqual(['**** 5555']); | ||
expect(component.cardsList.failedCards).toEqual(['**** 5555']); | ||
expect(component.cardsEnrolling).toBeFalse(); | ||
expect(stepCompleteSpy).toHaveBeenCalledWith(true); | ||
expect(showErrorPopoverSpy).toHaveBeenCalledTimes(1); | ||
})); | ||
}); | ||
|
||
describe('enrollSingularCard(): ', () => { | ||
it('should handle successful card enrollment', fakeAsync(() => { | ||
corporateCreditCardExpenseService.getCorporateCards.and.returnValue(of([])); | ||
component.ngOnInit(); | ||
|
||
component.fg.controls.card_number.setValue('41111111111111111'); | ||
const stepCompleteSpy = spyOn(component.isStepComplete, 'emit'); | ||
const showErrorPopoverSpy = spyOn(component, 'showErrorPopover'); | ||
const setupErrorMessagesSpy = spyOn(component, 'setupErrorMessages'); | ||
realTimeFeedService.enroll.and.returnValues(of(statementUploadedCard)); | ||
component.enrollSingularCard(); | ||
tick(); | ||
expect(component.cardsList.successfulCards).toEqual(['**** 1111']); | ||
expect(component.cardsEnrolling).toBeFalse(); | ||
expect(stepCompleteSpy).toHaveBeenCalledWith(true); | ||
expect(showErrorPopoverSpy).not.toHaveBeenCalled(); | ||
expect(setupErrorMessagesSpy).not.toHaveBeenCalled(); | ||
})); | ||
|
||
it('should handle unsuccessful card enrollment', fakeAsync(() => { | ||
corporateCreditCardExpenseService.getCorporateCards.and.returnValue(of([])); | ||
component.ngOnInit(); | ||
|
||
component.fg.controls.card_number.setValue('41111111111111111'); | ||
const stepCompleteSpy = spyOn(component.isStepComplete, 'emit'); | ||
const showErrorPopoverSpy = spyOn(component, 'showErrorPopover'); | ||
realTimeFeedService.enroll.and.returnValues( | ||
throwError(() => new Error('This card already exists in the system')) | ||
); | ||
component.enrollSingularCard(); | ||
tick(); | ||
expect(component.cardsList.failedCards).toEqual(['**** 1111']); | ||
expect(component.cardsEnrolling).toBeFalse(); | ||
expect(showErrorPopoverSpy).toHaveBeenCalledTimes(1); | ||
})); | ||
}); |
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)
The test coverage is powerful, but let's make it legendary!
The enrollment tests are solid like a punch sequence, but we could add more power with these additional test cases:
- Network timeout scenarios
- Multiple consecutive failures
- Rate limiting responses
Would you like me to help craft these additional test cases?
Coverage - remaining coverage to be added in follow up:
Summary by CodeRabbit
Bug Fixes
Tests