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

deleted seniors, request, and chapter #173

Merged
merged 4 commits into from
Apr 28, 2024
Merged

Conversation

nathan-j-edwards
Copy link
Collaborator

Description

  • Delete the chapter itself
  • Delete all associated seniors
  • Delete chapter request
  • Remove chapter ID from every member

Issues

#155

Screenshots

No user interface changes

Test

Submit new chapter request and accept. Add members, seniors, and files. Delete chapter and check database.

Copy link

vercel bot commented Apr 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
the-legacy-project2 ✅ Ready (Inspect) Visit Preview Apr 25, 2024 2:28am

Copy link
Collaborator

@sburchfield33 sburchfield33 left a comment

Choose a reason for hiding this comment

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

GJ!

src/app/api/chapter/[chapterId]/route.schema.ts Outdated Show resolved Hide resolved
src/app/api/chapter/[chapterId]/route.client.ts Outdated Show resolved Hide resolved
src/app/api/chapter/[chapterId]/route.ts Outdated Show resolved Hide resolved
src/app/api/chapter/[chapterId]/route.schema.ts Outdated Show resolved Hide resolved
src/app/api/chapter/[chapterId]/route.ts Outdated Show resolved Hide resolved
src/app/api/chapter/[chapterId]/route.ts Outdated Show resolved Hide resolved
src/app/api/chapter/[chapterId]/route.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@nickbar01234 nickbar01234 left a comment

Choose a reason for hiding this comment

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

Your changes are functionally correct! Echoing @sburchfield33 feedback, I think we can condensed the code a bit further. As mentioned, we want to reduce the number of API calls. This is important for a couple of reasons:

  1. Less network traffic is usually better
  2. Prisma handles transaction for us within 1 API call

In summary,

  1. We can use onDelete: Cascade for UserRequest
  2. Unset relationship on user side when a chapter is deleted. You may find this doc helpful.

Copy link
Collaborator

@sburchfield33 sburchfield33 left a comment

Choose a reason for hiding this comment

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

Looks good maybe look at my comments. I am a little confused about the ChapterRequest onDelete but otherwise looks good

@@ -131,6 +131,8 @@ model ChapterRequest {
motivation String
availabilities String
questions String

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is a chapterid field not necessary here?

@@ -144,6 +146,11 @@ model Chapter {
// Google Drive API related fields
chapterFolder String @default("")
permissions String[]

chapterRequestId String @db.ObjectId @unique
chapterRequest ChapterRequest @relation(fields: [chapterRequestId], references: [id], onDelete: Cascade)
Copy link
Collaborator

@sburchfield33 sburchfield33 Apr 28, 2024

Choose a reason for hiding this comment

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

Shouldn't the onDelete cascade be on the ChapterRequest field here? Wouldn't the onDelete here just delete the chapter if the chapterrequest is deleted?

I assume what you are trying to do here is delete the ChapterRequest if the chapter is deleted so I believe the onDelete should be on the chapterrequest side.

@nickbar01234 nickbar01234 self-requested a review April 28, 2024 20:53
Copy link
Collaborator

@nickbar01234 nickbar01234 left a comment

Choose a reason for hiding this comment

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

Discussed with @sburchfield33! We're good to go.

@nickbar01234 nickbar01234 merged commit cde0fb2 into main Apr 28, 2024
4 checks passed
@nickbar01234 nickbar01234 deleted the nathan/tyler/delete-chapter branch April 28, 2024 20:54
@nickbar01234 nickbar01234 linked an issue Apr 29, 2024 that may be closed by this pull request
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.

[Feature] Delete Chapter
3 participants