-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add validation to create evidence form #1115
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,10 @@ func CreateEvidenceMetadata(ctx context.Context, db *database.Connection, i Edit | |
return backend.WrapError("Unwilling to create evidence metadata", backend.UnauthorizedWriteErr(err)) | ||
} | ||
|
||
if i.Source == "" || i.Body == "" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's up to you if this is the way you want to do that, but generally you'd want to do these checks individually, as it'll provide more of an indication to the caller how to fix the issue. You also forgot to add a similar check in updateEvidence. I'd need to check, but I suspect you could still create empty evidence if you wanted by creating evidence with source and body, and then editing the evidence to remove the source and body. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might also look at |
||
return backend.WrapError("Source and Body cannot be empty.", backend.MissingValueErr("Source/Body")) | ||
} | ||
|
||
_, err = db.Insert("evidence_metadata", map[string]interface{}{ | ||
"evidence_id": evidence.ID, | ||
"source": i.Source, | ||
|
@@ -95,6 +99,10 @@ func UpsertEvidenceMetadata(ctx context.Context, db *database.Connection, i Upse | |
return backend.WrapError("Unwilling to edit evidence metadata", backend.UnauthorizedWriteErr(err)) | ||
} | ||
|
||
if i.Source == "" || i.Body == "" { | ||
return backend.WrapError("Source and Body cannot be empty.", backend.MissingValueErr("Source/Body")) | ||
} | ||
|
||
err = db.WithTx(ctx, func(tx *database.Transactable) { | ||
var metadata []models.EvidenceMetadata | ||
tx.Select(&metadata, sq.Select("*").From("evidence_metadata").Where(sq.Eq{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,6 +90,18 @@ export const CreateEvidenceModal = (props: { | |
const [selectedCBValue, setSelectedCBValue] = React.useState<string>(evidenceTypeOptions[0].value) | ||
const getSelectedOption = () => evidenceTypeOptions.filter(opt => opt.value === selectedCBValue)[0] | ||
|
||
// Validation logic for description and content fields | ||
const validateForm = () => { | ||
const errors = [] | ||
if (descriptionField.value.trim() === "") { | ||
errors.push("Description cannot be empty.") | ||
} | ||
if (getSelectedOption().value !== 'event' && binaryBlobField.value === null && codeblockField.value.code.trim() === "") { | ||
errors.push("Evidence content cannot be empty.") | ||
} | ||
return errors | ||
} | ||
Comment on lines
+94
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to run this, but docker seems to be misconfigured on this box. But, I think this will be okay. I don't think we have a lot of validation rules elsewhere to pull from, but I'll take a look to see if we solved this in a different way elsewhere. |
||
|
||
const formComponentProps = useForm({ | ||
fields: [descriptionField, binaryBlobField, adjustedAtField], | ||
onSuccess: () => { props.onCreated(); props.onRequestClose() }, | ||
|
@@ -106,6 +118,11 @@ export const CreateEvidenceModal = (props: { | |
data = { type: 'event' } | ||
} | ||
|
||
const formErrors = validateForm() | ||
if (formErrors.length > 0) { | ||
return Promise.reject(new Error(formErrors.join("\n"))) | ||
} | ||
|
||
return createEvidence({ | ||
adjustedAt: adjustedAtField.value, | ||
operationSlug: props.operationSlug, | ||
|
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.