-
Notifications
You must be signed in to change notification settings - Fork 38
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
pydantic v2 support #327
pydantic v2 support #327
Conversation
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## master #327 +/- ##
=======================================
Coverage 93.31% 93.31%
=======================================
Files 5 5
Lines 359 359
=======================================
Hits 335 335
Misses 24 24
|
admin_ui/src/utils.ts
Outdated
@@ -104,34 +125,44 @@ export function convertFormValue(params: { | |||
|
|||
if (value == "null") { | |||
value = null | |||
} else if (schema?.properties[key].type == "array") { | |||
} else if (schema.properties[key]["anyOf"][0].type == "array") { |
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.
Ideally we could do with some kind of function for doing this match. It's unlikely to happen but if the element we need isn't at the first index it would fail.
Something like:
const hasType = (property, type: string): boolean => {
const types = property.anyOf.map((i) => i.type)
return types.indexOf(type) != -1
}
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.
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.
If they're in order, then maybe getting the first element is the way to go.
I wonder what happens if we have a Piccolo column with required=True
- does it still return anyof
?
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.
No. If required=true
, not anyOf
is returned.
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.
Ideally we could do with some kind of function for doing this match. It's unlikely to happen but if the element we need isn't at the first index it would fail.
Something like:
const hasType = (property, type: string): boolean => { const types = property.anyOf.map((i) => i.type) return types.indexOf(type) != -1 }
In fact, for all columns whose type
or format
defined in Piccolo ORM pydantic.py, we don't need anyOf
. I will make a new commit with those changes. If it's not good, feel free to change it as you think is best.
d26ee8c
to
dd05068
Compare
} else if (schema?.properties[key].format == "uuid" && | ||
schema?.properties[key].extra["nullable"] == true | ||
&& value == "") { | ||
value = null | ||
} else if (schema?.properties[key].format == "email" && value == "") { | ||
} else if (schema?.properties[key].format == "email" && | ||
schema?.properties[key].extra["nullable"] == true | ||
&& value == "") { | ||
value = null | ||
} else if (schema?.properties[key].format == "date-time" && value == "") { | ||
} else if (schema?.properties[key].format == "date-time" && | ||
schema?.properties[key].extra["nullable"] == true | ||
&& value == "") { | ||
value = null | ||
} else if (schema?.properties[key].format == "date" && value == "") { | ||
} else if (schema?.properties[key].format == "date" && | ||
schema?.properties[key].extra["nullable"] == true | ||
&& value == "") { | ||
value = null | ||
} else if (schema?.properties[key].type == "integer" && value == "") { | ||
} else if (schema?.properties[key].format == "json" && | ||
schema?.properties[key].extra["nullable"] == true | ||
&& value == "") { | ||
value = null | ||
} else if (schema?.properties[key].type == "number" && value == "") { | ||
} | ||
else if (schema?.properties[key]["anyOf"][0].type == "integer" && |
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 wonder if it's possible to dramatically simplify this logic. I wonder if there are any column types left now, which when nullable and have an empty string as a value aren't converted to 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.
Maybe we can use a few variables to reduce code repetition. Something like this
export function convertFormValue(params: {
key: string
value: any
schema: Schema
}): any {
let { key, value, schema } = params
const columnTypeAnyOf = schema?.properties[key]["anyOf"][0].type
const columnFormat = schema?.properties[key].format
const columnNullable = schema?.properties[key].extra["nullable"] == true
const possibleFormats = [
"uuid",
"email",
"date-time",
"date",
"json",
"integer",
"number",
"string"
]
if (value == "null") {
value = null
} else if (schema?.properties[key].type == "array") {
value = JSON.parse(String(value))
} else if (possibleFormats.includes(columnFormat) &&
columnNullable
&& value == "") {
value = null
} else if (possibleFormats.includes(columnTypeAnyOf) &&
columnNullable
&& value == "") {
value = null
} else if (schema?.properties[key].extra.foreign_key == true &&
columnNullable &&
value == ""
) {
value = null
}
return value
}
Related to #313.