-
Notifications
You must be signed in to change notification settings - Fork 1
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
Make mutation pre/post checks optional #635
Conversation
pub table_name: sql::ast::TableName, | ||
pub by_columns: NonEmpty<metadata::database::ColumnInfo>, | ||
pub columns_prefix: String, | ||
pub pre_check: Option<CheckArgument>, |
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.
The Option
here isn't necessary, this can be CheckArgument
.
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's optional that the user provides the argument, but the argument will always be in the schema, just nullable)
@@ -24,7 +24,7 @@ pub struct InsertMutation { | |||
pub table_name: sql::ast::TableName, | |||
pub objects_argument_name: models::ArgumentName, | |||
pub columns: BTreeMap<models::FieldName, metadata::database::ColumnInfo>, | |||
pub post_check: CheckArgument, | |||
pub post_check: Option<CheckArgument>, |
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.
The Option
here isn't necessary, this can be CheckArgument
.
@@ -32,18 +32,11 @@ pub struct UpdateByKey { | |||
pub by_columns: NonEmpty<metadata::database::ColumnInfo>, | |||
pub columns_prefix: String, | |||
pub update_columns_argument_name: models::ArgumentName, | |||
pub pre_check: Constraint, | |||
pub post_check: Constraint, | |||
pub pre_check: Option<CheckArgument>, |
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.
Both the Option
s here aren't necessary, they can be CheckArgument
.
let post_predicate_json = | ||
mutation.post_check | ||
.as_ref() | ||
.and_then(|post_check| arguments.get(&post_check.argument_name)) |
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.
👍
e1cdc15
to
379c281
Compare
01c2301
to
b3f1bd4
Compare
<!-- The PR description should answer 2 (maybe 3) important questions: --> ### What Pulled from #635 into own PR. We add `mutations_prefix: Option<String>` to v5 configuration. When left `null` (for existing projects), we default to `v2_mutation_name`, but it can be replaced with `horse_mutation_name` etc. Most people will just want to use it to remove the prefix though, and that is the default for new projects. `mutationsPrefix: ""` will get rid of the prefix altogether, which I assume most users would want. ### How <!-- How is it trying to accomplish it (what are the implementation steps)? --> --------- Co-authored-by: Brandon Martin <brandon@codedmart.com>
What