-
Notifications
You must be signed in to change notification settings - Fork 78
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
fix: add patch to feed default ValidationOptions on table shorttext #7029
Conversation
customVal: null, | ||
selectedValidation: null, |
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'm wondering if we should add the prefill properties too, just for db consistency's sake
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.
Hm those are marked as optional on types. But if we look at inheritance it should be populated.
Both are correct. I in favour of keeping it as simple as possible.
newField: FieldUpdateDto, | ||
) => { | ||
if (newField.fieldType === BasicField.Table) { | ||
const defaultValidationOptions = { |
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.
const defaultValidationOptions = { | |
const defaultValidationOptions: TextValidationOptions = { |
(import TextValidationOptions from '../../../../../shared/types')
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.
Makes sense. Added in b8cb466
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.
lgtm! nit: add TextValidationOptions for type safety
Problem
Due to this issue on Mongoose 6.x, discriminated fields doesn't respect inherited defaults.
This currently affects our Table field where a
dropdown
column is updated toshortText
. This would then be updated into the DB without aValidationOptions
field. Consequently, code that referencescolumn.ValidationOptions.*
would throw an error as the object isundefined
.Note: Inserting a new
shortText
column uses the helper function that contains theValidationOptions
field, thus not affected by this change.Solution
Add a patch that checks for shortText and provide a defaulted
ValidationOptions
object to it.Tests
Bug fix tests, reproduction steps