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

Feature/update donation person #348

Merged
merged 3 commits into from
Nov 2, 2022
Merged

Conversation

stann1
Copy link
Contributor

@stann1 stann1 commented Oct 28, 2022

the backend part for: podkrepi-bg/frontend#958

@stann1 stann1 requested a review from igoychev October 28, 2022 07:56
@github-actions
Copy link

github-actions bot commented Oct 28, 2022

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.69% (+0.4% 🔼)
1884/2628
🔴 Branches
44.6% (+1.22% 🔼)
227/509
🔴 Functions
44.29% (+0.4% 🔼)
221/499
🟡 Lines
69.78% (+0.45% 🔼)
1667/2389

Test suite run success

176 tests passing in 64 suites.

Report generated by 🧪jest coverage report action from 5ddcba7

Copy link
Contributor

@igoychev igoychev left a comment

Choose a reason for hiding this comment

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

great overall, pls check my comments below

apps/api/src/donations/donations.service.ts Show resolved Hide resolved
Comment on lines +333 to +339
const targetDonor = await this.prisma.person.findFirst({
where: { id: updatePaymentDto.targetPersonId },
})
if (!targetDonor) {
throw new NotFoundException(
`Update failed. No person found with ID: ${updatePaymentDto.targetPersonId}`,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

May be we can skip checking for if the targetPersonId exists, because it will be checked automatically by the ForeignKey when updating the donation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true, but then the user will get a system unhandled exception, this way it is much more user friendly

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about handling the exception. Just found that Prisma has an error code for this case P2003 and the exception docs show how to handle it: https://www.prisma.io/docs/concepts/components/prisma-client/handling-exceptions-and-errors

apps/api/src/donations/donations.service.ts Outdated Show resolved Hide resolved
@stann1 stann1 merged commit 78c3c95 into master Nov 2, 2022
@stann1 stann1 deleted the feature/update-donation-person branch November 2, 2022 13:20
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.

2 participants