-
Notifications
You must be signed in to change notification settings - Fork 33
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
Database integrity fixes #243
Conversation
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.
Looks good to me! Only one question, which I'm sure is the right thing to do, but since I will certainly need to do it from time to time, I'd like to understand better.
@@ -31,6 +31,7 @@ public IActionResult OnGet() | |||
|
|||
public async Task<IActionResult> OnPostAsync() | |||
{ | |||
ModelState.Remove("Puzzle.Event"); |
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.
What does this do?
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.
I see it in many places but its meaning isn't clear to me.
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.
It tells the page not to validate the Event property -- as submitted, it's invalid because it's null (and thus a violation of [Required]
. We set it further down before sending it to the database. There are some similar cases on Edit pages where we just don't roundtrip a required property (usually the DB just don't get changed for those).
… paths (Event->ContentFile and Event->Puzzle->ContentFile)
105e151
to
41799b0
Compare
Fix various database integrity issues that would either allow corruption or prevent deleting entities. Changes include:
[Required]
to foreign key properties where we want cascading deletes (or where it would be bad if the value was ever null)[Required]
properties. This typically happens on Create or Edit pages where we set those values after the user submits it or where we don't want to round-trip it through editing.ModelState.Remove
lets us remove validation for only the fields we know won't matter.Fixes #177, fixes #106, fixes #59, fixes #104