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: Better error response for wrong datetime format in REST filter #4111
base: dev
Are you sure you want to change the base?
Conversation
if map.contains_key("key") { | ||
FieldCondition::deserialize(serde_json::Value::Object(map)) | ||
.map(Condition::Field) | ||
.map_err(serde::de::Error::custom) | ||
} else if map.contains_key("is_empty") { | ||
IsEmptyCondition::deserialize(serde_json::Value::Object(map)) | ||
.map(Condition::IsEmpty) | ||
.map_err(serde::de::Error::custom) | ||
} else if map.contains_key("is_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.
with if-else statements it will be very easy to forget to implement some parts of the deserializer if we introduce new field here, as compiler won't show any errors
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.
@generall - fair enough, I'll try to change it, do you suggest any approach?
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.
A match
statement with strict enum variants for example. Though to be honest, I don't clearly see how such approach can be used here.
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 don't clearly see how such approach can be used here
yes that is true
.
strict enum variants
But isn't that requires introducing a new enum, and we might face the did not match any variant
again ?
All Submissions:
dev
branch. Did you create your branch fromdev
?New Feature Submissions:
cargo +nightly fmt --all
command prior to submission?cargo clippy --all --all-features
command?Changes to Core Features:
/claim #3531