feat: Add service for spender onboarding #3378
Conversation
WalkthroughIn a superstar style, this pull request introduces a comprehensive onboarding system for the application! We've crafted a robust set of models and a service that tracks user onboarding with precision. The changes include an enumeration for onboarding states, interfaces for tracking onboarding status, and a dedicated service to manage the entire onboarding process with multiple steps and configurations. Changes
Sequence DiagramsequenceDiagram
participant User
participant OnboardingService
participant API
User->>OnboardingService: Start Onboarding
OnboardingService->>API: Get Onboarding Status
API-->>OnboardingService: Return Current Status
OnboardingService->>User: Display Onboarding Steps
User->>OnboardingService: Complete/Skip Steps
OnboardingService->>API: Update Step Status
API-->>OnboardingService: Confirm Update
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/app/core/models/onboarding-state.enum.ts(1 hunks)src/app/core/models/onboarding-status.model.ts(1 hunks)src/app/core/models/onboarding-step-status.model.ts(1 hunks)src/app/core/models/onboarding-welcome-step-status.model.ts(1 hunks)src/app/core/services/spender-onboarding.service.ts(1 hunks)
| export interface OnboardingWelcomeStepStatus { | ||
| is_complete: boolean; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mind it! The interface looks perfect, but let's add some swagger with JSDoc!
The interface is clean and purposeful like my signature moves! But let me show you how to make it even more powerful, partner!
+/**
+ * Interface to track the completion status of the welcome step in user onboarding.
+ */
export interface OnboardingWelcomeStepStatus {
is_complete: boolean;
}📝 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.
| export interface OnboardingWelcomeStepStatus { | |
| is_complete: boolean; | |
| } | |
| /** | |
| * Interface to track the completion status of the welcome step in user onboarding. | |
| */ | |
| export interface OnboardingWelcomeStepStatus { | |
| is_complete: boolean; | |
| } |
| export interface OnboardingStepStatus { | ||
| is_configured: boolean; | ||
| is_skipped: boolean; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Magizhchi! Let's make this interface bulletproof like Thalaiva!
The interface is good, but we can make it legendary! When a step is skipped, it can't be configured, and vice versa. Let's add some type magic!
+/**
+ * Represents the status of an onboarding step with mutually exclusive states
+ */
-export interface OnboardingStepStatus {
- is_configured: boolean;
- is_skipped: boolean;
-}
+export type OnboardingStepStatus =
+ | { is_configured: true; is_skipped: false }
+ | { is_configured: false; is_skipped: true }
+ | { is_configured: false; is_skipped: false };📝 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.
| export interface OnboardingStepStatus { | |
| is_configured: boolean; | |
| is_skipped: boolean; | |
| } | |
| /** | |
| * Represents the status of an onboarding step with mutually exclusive states | |
| */ | |
| export type OnboardingStepStatus = | |
| | { is_configured: true; is_skipped: false } | |
| | { is_configured: false; is_skipped: true } | |
| | { is_configured: false; is_skipped: false }; |
| export enum OnboardingState { | ||
| YET_TO_START = 'YET_TO_START', | ||
| IN_PROGRESS = 'IN_PROGRESS', | ||
| COMPLETED = 'COMPLETED', | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Kabali da! This enum is powerful, but let's give it some style!
The enum is perfect like a well-choreographed fight scene! But every superstar needs an introduction, so let's add some documentation!
+/**
+ * Represents the various states of the user onboarding process.
+ *
+ * @enum {string}
+ * @property {string} YET_TO_START - Initial state before onboarding begins
+ * @property {string} IN_PROGRESS - User is actively going through onboarding
+ * @property {string} COMPLETED - User has finished the onboarding process
+ */
export enum OnboardingState {
YET_TO_START = 'YET_TO_START',
IN_PROGRESS = 'IN_PROGRESS',
COMPLETED = 'COMPLETED',
}📝 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.
| export enum OnboardingState { | |
| YET_TO_START = 'YET_TO_START', | |
| IN_PROGRESS = 'IN_PROGRESS', | |
| COMPLETED = 'COMPLETED', | |
| } | |
| /** | |
| * Represents the various states of the user onboarding process. | |
| * | |
| * @enum {string} | |
| * @property {string} YET_TO_START - Initial state before onboarding begins | |
| * @property {string} IN_PROGRESS - User is actively going through onboarding | |
| * @property {string} COMPLETED - User has finished the onboarding process | |
| */ | |
| export enum OnboardingState { | |
| YET_TO_START = 'YET_TO_START', | |
| IN_PROGRESS = 'IN_PROGRESS', | |
| COMPLETED = 'COMPLETED', | |
| } |
| step_connect_cards_is_configured: boolean; | ||
| step_connect_cards_is_skipped: boolean; | ||
| step_sms_opt_in_is_configured: boolean; | ||
| step_sms_opt_in_is_skipped: boolean; | ||
| step_show_welcome_modal_is_complete: boolean; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Style! Let's ensure our steps can't be both configured and skipped at the same time!
Like how I can't be both the hero and villain in the same scene, a step can't be both configured and skipped! Consider adding runtime validation.
Consider implementing a validation decorator or method to ensure that is_configured and is_skipped flags are not both true for the same step.
| export interface OnboardingStatus { | ||
| user_id: string; | ||
| org_id: string; | ||
| step_connect_cards_is_configured: boolean; | ||
| step_connect_cards_is_skipped: boolean; | ||
| step_sms_opt_in_is_configured: boolean; | ||
| step_sms_opt_in_is_skipped: boolean; | ||
| step_show_welcome_modal_is_complete: boolean; | ||
| state: OnboardingState; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Mind it! Let's add some powerful documentation to this interface, shall we?
Like a well-choreographed fight scene, every piece should know its role! Add JSDoc comments to explain the purpose of this interface and its properties.
+/**
+ * Represents the user's onboarding status and progress through various steps
+ */
export interface OnboardingStatus {
+ /** Unique identifier of the user */
user_id: string;
+ /** Organization identifier the user belongs to */
org_id: string;
+ /** Indicates if cards connection step is configured */
step_connect_cards_is_configured: boolean;
+ /** Indicates if cards connection step is skipped */
step_connect_cards_is_skipped: boolean;
+ /** Indicates if SMS opt-in step is configured */
step_sms_opt_in_is_configured: boolean;
+ /** Indicates if SMS opt-in step is skipped */
step_sms_opt_in_is_skipped: boolean;
+ /** Indicates if welcome modal step is completed */
step_show_welcome_modal_is_complete: boolean;
+ /** Current state of the onboarding process */
state: OnboardingState;
}📝 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.
| export interface OnboardingStatus { | |
| user_id: string; | |
| org_id: string; | |
| step_connect_cards_is_configured: boolean; | |
| step_connect_cards_is_skipped: boolean; | |
| step_sms_opt_in_is_configured: boolean; | |
| step_sms_opt_in_is_skipped: boolean; | |
| step_show_welcome_modal_is_complete: boolean; | |
| state: OnboardingState; | |
| } | |
| /** | |
| * Represents the user's onboarding status and progress through various steps | |
| */ | |
| export interface OnboardingStatus { | |
| /** Unique identifier of the user */ | |
| user_id: string; | |
| /** Organization identifier the user belongs to */ | |
| org_id: string; | |
| /** Indicates if cards connection step is configured */ | |
| step_connect_cards_is_configured: boolean; | |
| /** Indicates if cards connection step is skipped */ | |
| step_connect_cards_is_skipped: boolean; | |
| /** Indicates if SMS opt-in step is configured */ | |
| step_sms_opt_in_is_configured: boolean; | |
| /** Indicates if SMS opt-in step is skipped */ | |
| step_sms_opt_in_is_skipped: boolean; | |
| /** Indicates if welcome modal step is completed */ | |
| step_show_welcome_modal_is_complete: boolean; | |
| /** Current state of the onboarding process */ | |
| state: OnboardingState; | |
| } |
| processConnectCardsStep(data: OnboardingStepStatus): Observable<OnboardingStepStatus> { | ||
| return this.spenderPlatformV1ApiService | ||
| .post<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding/process_step_connect_cards', { | ||
| data, | ||
| }) | ||
| .pipe(map((res) => res.data)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mind it! Let's avoid repeating ourselves like a broken record!
These process methods are more similar than my punch dialogues! Let's create a reusable helper method.
+private processStep<T>(endpoint: string, data: T): Observable<T> {
+ return this.spenderPlatformV1ApiService
+ .post<PlatformApiResponse<T>>(`/spender/onboarding/${endpoint}`, { data })
+ .pipe(
+ map((res) => res.data),
+ catchError((error) => {
+ console.error(`Failed to process step ${endpoint}:`, error);
+ return throwError(() => new Error(`Failed to process step ${endpoint}`));
+ })
+ );
+}
processConnectCardsStep(data: OnboardingStepStatus): Observable<OnboardingStepStatus> {
- return this.spenderPlatformV1ApiService
- .post<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding/process_step_connect_cards', {
- data,
- })
- .pipe(map((res) => res.data));
+ return this.processStep('process_step_connect_cards', data);
}Also applies to: 28-32, 34-40
| import { Injectable } from '@angular/core'; | ||
| import { map, Observable } from 'rxjs'; | ||
| import { SpenderPlatformV1ApiService } from './spender-platform-v1-api.service'; | ||
| import { PlatformApiResponse } from '../models/platform/platform-api-response.model'; | ||
| import { OnboardingWelcomeStepStatus } from '../models/onboarding-welcome-step-status.model'; | ||
| import { OnboardingStepStatus } from '../models/onboarding-step-status.model'; | ||
|
|
||
| @Injectable({ | ||
| providedIn: 'root', | ||
| }) | ||
| export class OnboardingService { | ||
| constructor(private spenderPlatformV1ApiService: SpenderPlatformV1ApiService) {} | ||
|
|
||
| getOnboardingStatus(): Observable<OnboardingStepStatus> { | ||
| return this.spenderPlatformV1ApiService | ||
| .get<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding') | ||
| .pipe(map((res) => res.data)); | ||
| } | ||
|
|
||
| processConnectCardsStep(data: OnboardingStepStatus): Observable<OnboardingStepStatus> { | ||
| return this.spenderPlatformV1ApiService | ||
| .post<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding/process_step_connect_cards', { | ||
| data, | ||
| }) | ||
| .pipe(map((res) => res.data)); | ||
| } | ||
|
|
||
| processSmsOptInStep(data: OnboardingStepStatus): Observable<OnboardingStepStatus> { | ||
| return this.spenderPlatformV1ApiService | ||
| .post<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding/process_step_sms_opt_in', { data }) | ||
| .pipe(map((res) => res.data)); | ||
| } | ||
|
|
||
| processWelcomeModalStep(data: OnboardingWelcomeStepStatus): Observable<OnboardingWelcomeStepStatus> { | ||
| return this.spenderPlatformV1ApiService | ||
| .post<PlatformApiResponse<OnboardingWelcomeStepStatus>>('/spender/onboarding/process_step_show_welcome_modal', { | ||
| data, | ||
| }) | ||
| .pipe(map((res) => res.data)); | ||
| } | ||
|
|
||
| markWelcomeModalStepAsComplete(): Observable<OnboardingWelcomeStepStatus> { | ||
| const data: OnboardingWelcomeStepStatus = { | ||
| is_complete: true, | ||
| }; | ||
| return this.processWelcomeModalStep(data); | ||
| } | ||
|
|
||
| markConnectCardsStepAsComplete(): Observable<OnboardingStepStatus> { | ||
| const data: OnboardingStepStatus = { | ||
| is_configured: true, | ||
| is_skipped: false, | ||
| }; | ||
| return this.processConnectCardsStep(data); | ||
| } | ||
|
|
||
| skipConnectCardsStep(): Observable<OnboardingStepStatus> { | ||
| const data: OnboardingStepStatus = { | ||
| is_configured: false, | ||
| is_skipped: true, | ||
| }; | ||
| return this.processConnectCardsStep(data); | ||
| } | ||
|
|
||
| markSmsOptInStepAsComplete(): Observable<OnboardingStepStatus> { | ||
| const data: OnboardingStepStatus = { | ||
| is_configured: true, | ||
| is_skipped: false, | ||
| }; | ||
| return this.processSmsOptInStep(data); | ||
| } | ||
|
|
||
| skipSmsOptInStep(): Observable<OnboardingStepStatus> { | ||
| const data: OnboardingStepStatus = { | ||
| is_configured: false, | ||
| is_skipped: true, | ||
| }; | ||
| return this.processSmsOptInStep(data); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Time for some epic retry logic, thalaiva style!
Like how I never give up, let's add retry logic to our API calls!
Add the following import and apply retry operators to all API calls:
import { retry, retryWhen, delay, take } from 'rxjs/operators';
// Apply to all API calls
.pipe(
retry({
count: 3,
delay: 1000,
resetOnSuccess: true
})
)| @Injectable({ | ||
| providedIn: 'root', | ||
| }) | ||
| export class OnboardingService { | ||
| constructor(private spenderPlatformV1ApiService: SpenderPlatformV1ApiService) {} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Testing is like my stunts - it needs to be perfect!
Don't forget to add unit tests for this service, covering all the epic scenarios!
Would you like me to generate comprehensive unit tests for this service? Just say the word!
| getOnboardingStatus(): Observable<OnboardingStepStatus> { | ||
| return this.spenderPlatformV1ApiService | ||
| .get<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding') | ||
| .pipe(map((res) => res.data)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Magizhchi! But let's handle errors like a boss!
The API call needs proper error handling, partner! When things go wrong, we should handle it with style!
getOnboardingStatus(): Observable<OnboardingStepStatus> {
return this.spenderPlatformV1ApiService
.get<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding')
- .pipe(map((res) => res.data));
+ .pipe(
+ map((res) => res.data),
+ catchError((error) => {
+ console.error('Failed to fetch onboarding status:', error);
+ return throwError(() => new Error('Failed to fetch onboarding status'));
+ })
+ );
}📝 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.
| getOnboardingStatus(): Observable<OnboardingStepStatus> { | |
| return this.spenderPlatformV1ApiService | |
| .get<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding') | |
| .pipe(map((res) => res.data)); | |
| } | |
| getOnboardingStatus(): Observable<OnboardingStepStatus> { | |
| return this.spenderPlatformV1ApiService | |
| .get<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding') | |
| .pipe( | |
| map((res) => res.data), | |
| catchError((error) => { | |
| console.error('Failed to fetch onboarding status:', error); | |
| return throwError(() => new Error('Failed to fetch onboarding status')); | |
| }) | |
| ); | |
| } |
| markConnectCardsStepAsComplete(): Observable<OnboardingStepStatus> { | ||
| const data: OnboardingStepStatus = { | ||
| is_configured: true, | ||
| is_skipped: false, | ||
| }; | ||
| return this.processConnectCardsStep(data); | ||
| } | ||
|
|
||
| skipConnectCardsStep(): Observable<OnboardingStepStatus> { | ||
| const data: OnboardingStepStatus = { | ||
| is_configured: false, | ||
| is_skipped: true, | ||
| }; | ||
| return this.processConnectCardsStep(data); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Like my signature moves, let's make our step processing more robust!
The step completion methods could use some validation to prevent invalid states.
markConnectCardsStepAsComplete(): Observable<OnboardingStepStatus> {
+ if (this.currentStep !== 'connect_cards') {
+ return throwError(() => new Error('Invalid step sequence'));
+ }
const data: OnboardingStepStatus = {
is_configured: true,
is_skipped: false,
};
return this.processConnectCardsStep(data);
}Committable suggestion skipped: line range outside the PR's diff.
|
bistaastha
merged commit d4fd372
into
FYLE-86cx2t82k-base-feature-branch
Summary by CodeRabbit
New Features
YET_TO_START,IN_PROGRESS, andCOMPLETED.Documentation