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.

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

fix: paddle transaction management for gifting #2648

Merged
merged 10 commits into from
Feb 10, 2025
Merged

fix: paddle transaction management for gifting #2648

merged 10 commits into from
Feb 10, 2025

Conversation

sshanzel
Copy link
Member

@sshanzel sshanzel commented Feb 8, 2025

One-time payments are not considered subscriptions in Paddle's dictionary, so we must handle the event for gifting within the TransactionCompleted. Events available are as below:

Screenshot 2025-02-08 at 12 38 33 PM

Test cases still check out.

Comment on lines -89 to -115
const { gifter_id: gifterId } = customData;
const duration = subscriptionGiftDuration;
const isGift = !!gifterId;
if (isGift) {
if (userId === gifterId) {
logger.error({ type: 'paddle', data }, 'User and gifter are the same');
return false;
}

const gifterUser = await con
.getRepository(User)
.findOneBy({ id: gifterId });
if (!gifterUser) {
logger.error({ type: 'paddle', data }, 'Gifter user not found');
return false;
}

const targetUser = await con.getRepository(User).findOne({
select: ['subscriptionFlags'],
where: { id: userId },
});
if (isPlusMember(targetUser?.subscriptionFlags?.cycle)) {
logger.error({ type: 'paddle', data }, 'User is already a Plus member');
return false;
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to its own function to lessen the noise here.

Copy link
Contributor

@capJavert capJavert left a comment

Choose a reason for hiding this comment

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

Just some questions

src/routes/webhooks/paddle.ts Outdated Show resolved Hide resolved
src/routes/webhooks/paddle.ts Outdated Show resolved Hide resolved
src/routes/webhooks/paddle.ts Outdated Show resolved Hide resolved
@sshanzel sshanzel requested a review from capJavert February 10, 2025 08:52

This comment has been minimized.

src/routes/webhooks/paddle.ts Outdated Show resolved Hide resolved
src/routes/webhooks/paddle.ts Show resolved Hide resolved
Comment on lines 421 to 423
export const processTransactionCompleted = async (
result: TransactionCompletedEvent,
) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make it accept object { data } or whatever, easier to expand later

Copy link
Member Author

Choose a reason for hiding this comment

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

The whole object was needed originally, and didn't want to make any change outside the gifting, but yeah, I can update the other functions too.

Copy link
Contributor

@capJavert capJavert Feb 10, 2025

Choose a reason for hiding this comment

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

@sshanzel you can still do the whole object, just wanted to move to object syntax since you are adding this helper.

Copy link
Contributor

Choose a reason for hiding this comment

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

export const processTransactionCompleted = async ({
  result: TransactionCompletedEvent,
}) => Promise

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, cool, yeah, we can do that.

@@ -405,6 +370,66 @@ const notifyNewPaddleTransaction = async ({
await webhooks.transactions.send({ blocks });
};

export const processGiftedPayment = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@capJavert capJavert left a comment

Choose a reason for hiding this comment

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

Looked at the code more, few more questions 🙏

Copy link
Contributor

@capJavert capJavert left a comment

Choose a reason for hiding this comment

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

Approving, good to go after comments are resolved.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

@rebelchris rebelchris left a comment

Choose a reason for hiding this comment

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

Nothing besides Ante's stuff, but it's kind of hard to see as so much is swapped in diff.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

pulumi bot commented Feb 10, 2025

🍹 The Update (preview) for dailydotdev/api/prod was successful.

Resource Changes

    Name                                            Type                           Operation
~   vpc-native-personalized-digest-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-private-deployment                   kubernetes:apps/v1:Deployment  update
~   vpc-native-update-trending-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-ws-deployment                        kubernetes:apps/v1:Deployment  update
~   vpc-native-update-source-public-threshold-cron  kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-gifted-plus-cron               kubernetes:batch/v1:CronJob    update
~   vpc-native-hourly-notification-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron           kubernetes:batch/v1:CronJob    update
~   vpc-native-generic-referral-reminder-cron       kubernetes:batch/v1:CronJob    update
~   vpc-native-update-highlighted-views-cron        kubernetes:batch/v1:CronJob    update
-   vpc-native-api-migration-75584513               kubernetes:batch/v1:Job        delete
~   vpc-native-validate-active-users-cron           kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron             kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-deployment       kubernetes:apps/v1:Deployment  update
~   vpc-native-update-current-streak-cron           kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-update-tags-str-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-deployment                           kubernetes:apps/v1:Deployment  update
~   vpc-native-generate-search-invites-cron         kubernetes:batch/v1:CronJob    update
~   vpc-native-bg-deployment                        kubernetes:apps/v1:Deployment  update
~   vpc-native-daily-digest-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-tag-view-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-update-tag-recommendations-cron      kubernetes:batch/v1:CronJob    update
+   vpc-native-api-migration-7c5d7b35               kubernetes:batch/v1:Job        create
~   vpc-native-check-analytics-report-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-update-views-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-user-companies-cron     kubernetes:batch/v1:CronJob    update

@sshanzel sshanzel merged commit 5bdfe3d into main Feb 10, 2025
8 checks passed
@sshanzel sshanzel deleted the MI-785 branch February 10, 2025 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants