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

added posts crud #14

Merged
merged 5 commits into from
Aug 16, 2024
Merged

added posts crud #14

merged 5 commits into from
Aug 16, 2024

Conversation

FearsomeRover
Copy link
Contributor

Closes #2

id,
},
});
if (currentPost.authorId !== user.authSchId && user.role !== 'BODY_ADMIN') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it a good idea to only allow the original author to edit/delete posts?

id,
},
});
if (currentPost.authorId !== user.authSchId && user.role !== 'BODY_ADMIN') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

@FearsomeRover FearsomeRover requested a review from Isti01 July 23, 2024 12:56
Comment on lines 44 to 59
async update(id: number, updatePostDto: UpdatePostDto) {
return this.prisma.post.update({
where: {
id,
},
data: updatePostDto,
});
}

async remove(id: number) {
return this.prisma.post.delete({
where: {
id,
},
});
}
Copy link
Member

Choose a reason for hiding this comment

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

prisma update and delete methods throw an exception if no record is found, we should catch so no 500 error is thrown

Copy link
Member

@Tschonti Tschonti left a comment

Choose a reason for hiding this comment

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

Sorry for reviewing it in stages, I haven't started the app until now 🫠

but these bugs could've easily been found with some basic manual testing xd

Comment on lines 34 to 50
try {
return this.prisma.post.findUniqueOrThrow({
where: {
id,
},
include: {
author: true,
},
});
} catch (e) {
if (e instanceof Prisma.PrismaClientKnownRequestError) {
if (e.code === 'P2025') {
throw new NotFoundException('This post does not exist.');
}
throw new InternalServerErrorException('An error occurred.');
}
}
Copy link
Member

Choose a reason for hiding this comment

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

classic mistake: trying to catch an error in an async call without awaiting it

Comment on lines 75 to 92
try {
return this.prisma.post.delete({
where: {
id,
},
});
} catch (e) {
if (e instanceof Prisma.PrismaClientKnownRequestError) {
switch (e.code) {
case 'P2000':
throw new BadRequestException('The content is too long.');
case 'P2025':
throw new NotFoundException('This post does not exist.');
}
}
throw new InternalServerErrorException('An error occurred.');
}
}
Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
Member

@Tschonti Tschonti left a comment

Choose a reason for hiding this comment

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

sorry az előző review elég passzív agresszív volt xd

@mozsarmate mozsarmate merged commit 34944a0 into main Aug 16, 2024
3 checks passed
@FearsomeRover FearsomeRover deleted the feat/2-posts-crud branch August 29, 2024 09:17
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.

Post CRUD endpoints on backend
4 participants