-
Notifications
You must be signed in to change notification settings - Fork 44
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
#464 - Add check for an existing person
and connect before updating donation
#467
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -460,7 +460,12 @@ export class CampaignService { | |
|
||
Logger.debug('Donation found by subscription: ', donation) | ||
} | ||
|
||
let person | ||
if (paymentData.billingEmail) { | ||
person = await this.prisma.person.findFirst({ | ||
where: { email: paymentData.billingEmail }, | ||
}) | ||
} | ||
//if missing create the donation with the incoming status | ||
if (!donation) { | ||
Logger.debug( | ||
|
@@ -485,7 +490,10 @@ export class CampaignService { | |
extPaymentMethodId: paymentData.paymentMethodId ?? '', | ||
billingName: paymentData.billingName, | ||
billingEmail: paymentData.billingEmail, | ||
person: paymentData.personId ? { connect: { id: paymentData.personId } } : {}, | ||
person: | ||
metadata?.isAnonymous !== 'true' && person | ||
? { connect: { id: person.id } } | ||
: undefined, | ||
}, | ||
select: donationNotificationSelect, | ||
}) | ||
|
@@ -497,11 +505,10 @@ export class CampaignService { | |
) | ||
throw new InternalServerErrorException(error) | ||
} | ||
|
||
return | ||
} | ||
//donation exists, so check if it is safe to update it | ||
else if (shouldAllowStatusChange(donation.status, newDonationStatus)) { | ||
if (shouldAllowStatusChange(donation.status, newDonationStatus)) { | ||
try { | ||
const updatedDonation = await this.prisma.donation.update({ | ||
where: { | ||
|
@@ -510,11 +517,16 @@ export class CampaignService { | |
data: { | ||
status: newDonationStatus, | ||
amount: paymentData.netAmount, | ||
chargedAmount: paymentData.chargedAmount, | ||
extCustomerId: paymentData.stripeCustomerId, | ||
extPaymentMethodId: paymentData.paymentMethodId, | ||
extPaymentIntentId: paymentData.paymentIntentId, | ||
billingName: paymentData.billingName, | ||
billingEmail: paymentData.billingEmail, | ||
person: | ||
metadata?.isAnonymous !== 'true' && person | ||
? { connect: { id: person.id } } | ||
: undefined, | ||
}, | ||
select: donationNotificationSelect, | ||
}) | ||
|
@@ -529,38 +541,13 @@ export class CampaignService { | |
) | ||
throw new InternalServerErrorException(error) | ||
} | ||
} | ||
//donation exists but we need to skip because previous status is from later event than the incoming | ||
else { | ||
Logger.warn( | ||
`Skipping update of donation with paymentIntentId: ${paymentData.paymentIntentId} | ||
and status: ${newDonationStatus} because the event comes after existing donation with status: ${donation.status}`, | ||
) | ||
return | ||
} | ||
|
||
//For successful donations we will also need to link them to user and add donation wish: | ||
if (newDonationStatus === DonationStatus.succeeded) { | ||
Logger.debug('metadata?.isAnonymous = ' + metadata?.isAnonymous) | ||
|
||
if (metadata?.isAnonymous != 'true') { | ||
await this.prisma.donation.update({ | ||
where: { id: donation.id }, | ||
data: { | ||
person: { | ||
connect: { | ||
email: paymentData.billingEmail, | ||
}, | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
Logger.debug('Saving donation wish ' + metadata?.wish) | ||
|
||
if (metadata?.wish) { | ||
await this.createDonationWish(metadata.wish, donation.id, campaign.id) | ||
} | ||
} | ||
Comment on lines
-541
to
-563
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The removed code was intended to be executed for Charge.Succeeded event so that only then to save the donation wish. Unfortunately moving it above doesn't guarantee that. So, please see my comment above that the actual bug is because this code is skipped by the forgotten 'return' on line 500. |
||
Logger.warn( | ||
`Skipping update of donation with paymentIntentId: ${paymentData.paymentIntentId} | ||
and status: ${newDonationStatus} because the event comes after existing donation with status: ${donation.status}`, | ||
) | ||
} | ||
|
||
async createDonationWish(message: string, donationId: string, campaignId: 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.
the bug is actually on this line:
If the Charge.succeeded event is sent first from Stripe - this 'return' here will skip the connection of the donation to the user by mail.