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

#464 - Add check for an existing person and connect before updating donation #467

Conversation

dimitur2204
Copy link
Contributor

@dimitur2204 dimitur2204 commented Mar 16, 2023

Closes #464

Motivation and context

Quick fix to check if a person exists and then connect to the donation accordingly.
This whole flow needs revisiting and will be addressed in #466

Testing

Tested out the whole flow for both logged, anonymous and not logged and it is working as expected

@github-actions
Copy link

github-actions bot commented Mar 16, 2023

✅ Tests will run for this PR. Once they succeed it can be merged.

@dimitur2204 dimitur2204 requested review from igoychev and slavcho and removed request for igoychev March 16, 2023 15:37
@dimitur2204 dimitur2204 self-assigned this Mar 16, 2023
@dimitur2204 dimitur2204 added the run tests Allows running the tests workflows for forked repos label Mar 16, 2023
@github-actions github-actions bot removed the run tests Allows running the tests workflows for forked repos label Mar 16, 2023
@dimitur2204 dimitur2204 changed the title fix - check for an existing person and connect before updating donation #464 - Add check for an existing person and connect before updating donation Mar 16, 2023
@@ -497,11 +505,10 @@ export class CampaignService {
)
throw new InternalServerErrorException(error)
}

return
Copy link
Contributor

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.

Comment on lines -541 to -563
//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)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

[Bug] All donations are showing as anonymous
2 participants