-
Notifications
You must be signed in to change notification settings - Fork 4
[MKT-748]:feat/implement tracking services for cancelled subscriptions #351
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
base: master
Are you sure you want to change the base?
Conversation
|
xabg2
left a comment
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.
Add the tests for this new klaviyo service.
| }, | ||
| }); | ||
|
|
||
| console.log(`[Klaviyo] ${eventName} tracked for ${email}`); |
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.
Use Logger instead
| console.log(`[Klaviyo] ${eventName} tracked for ${email}`); | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : 'Unknown error'; | ||
| console.error(`[Klaviyo] ${eventName} failed for ${email}:`, message); |
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.
Use Logger instead
| try { | ||
| await klaviyoService.trackSubscriptionCancelled(customer.email); | ||
| } catch (error) { | ||
| log.error(`[KLAVIYO] Failed to track cancellation for ${customerId}`); |
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.
Log the error here also, and it would be nice to use Logger we can remove log in the future.
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.
Export the class here directly, e.g.: const klaviyoService = new KlaviyoService(); so you can export it directly without instantiating it every time (you can use the api key directly).
| baseUrl: string | undefined = process.env.KLAVIYO_BASE_URL | ||
| ) { | ||
| if (!apiKey) { | ||
| throw new Error("Klaviyo API Key is required."); |
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.
Use throw new BadRequestError('...') instead.
| } | ||
|
|
||
| export class KlaviyoTrackingService { | ||
| private readonly apiKey: string; |
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.
You can assign here directly the variables if you want, no need to pass it as props. Also, use config file instead of process.env.
| async trackSubscriptionCancelled(email: string): Promise<void> { | ||
| await this.trackEvent({ | ||
| email, | ||
| eventName: 'Subscription Cancelled', |
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.
nit: you can extract this to an enum so you can do:
TRACK_ENENTS.SubscriptionCancelled or smth like that.
|
Check Sonarcloud and tests, they are failing @jaaaaavier |




This task, which is linked to additional PRs in other repositories, aims to track users who cancel their plans and send that data to Klaviyo. While Klaviyo already integrates with Stripe, it lacks this specific functionality, which is why we created this solution.
Changes:
Comment
Need to add two Environment variables