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 service for spender onboarding #3378

Merged
merged 2 commits into from
Dec 19, 2024
Merged

feat: Add service for spender onboarding #3378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d563aad
Minor
bistaastha Dec 18, 2024
3980083
Minor
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
5 changes: 5 additions & 0 deletions src/app/core/models/onboarding-state.enum.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export enum OnboardingState {
YET_TO_START = 'YET_TO_START',
IN_PROGRESS = 'IN_PROGRESS',
COMPLETED = 'COMPLETED',
}
Comment on lines +1 to +5
Copy link

Choose a reason for hiding this comment

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

🧹 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.

Suggested change
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',
}

12 changes: 12 additions & 0 deletions src/app/core/models/onboarding-status.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { OnboardingState } from './onboarding-state.enum';

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;
Comment on lines +6 to +10
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! 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.

state: OnboardingState;
}
Comment on lines +3 to +12
Copy link

Choose a reason for hiding this comment

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

🧹 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.

Suggested change
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;
}

4 changes: 4 additions & 0 deletions src/app/core/models/onboarding-step-status.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export interface OnboardingStepStatus {
is_configured: boolean;
is_skipped: boolean;
}
Comment on lines +1 to +4
Copy link

Choose a reason for hiding this comment

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

🧹 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.

Suggested change
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 };

3 changes: 3 additions & 0 deletions src/app/core/models/onboarding-welcome-step-status.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface OnboardingWelcomeStepStatus {
is_complete: boolean;
}
Comment on lines +1 to +3
Copy link

Choose a reason for hiding this comment

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

🧹 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.

Suggested change
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;
}

Empty file added src/app/core/services/spender-onboarding.service.spec.ts
View file
Open in desktop
Empty file.
80 changes: 80 additions & 0 deletions src/app/core/services/spender-onboarding.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
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) {}
Comment on lines +8 to +12
Copy link

Choose a reason for hiding this comment

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

🧹 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));
}
Comment on lines +14 to +18
Copy link

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
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'));
})
);
}


processConnectCardsStep(data: OnboardingStepStatus): Observable<OnboardingStepStatus> {
return this.spenderPlatformV1ApiService
.post<PlatformApiResponse<OnboardingStepStatus>>('/spender/onboarding/process_step_connect_cards', {
data,
})
.pipe(map((res) => res.data));
}
Comment on lines +20 to +26
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 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


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);
}
Comment on lines +49 to +63
Copy link

Choose a reason for hiding this comment

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

🧹 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.


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);
}
}
Comment on lines +1 to +80
Copy link

Choose a reason for hiding this comment

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

🛠️ 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
  })
)

Loading