-
Notifications
You must be signed in to change notification settings - Fork 212
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
feat: batch findUniqueOrThrow for getting tests feedback (co-authored by @hayes) #4019
Conversation
CodSpeed Performance ReportMerging #4019 Summary
|
Note: related to |
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.
Looks good 👍
None if throw_on_empty => responses.insert_error(GQLError::from_user_facing_error( | ||
user_facing_errors::query_engine::RecordRequiredButNotFound { | ||
cause: "Expected a record, found none.".to_owned(), | ||
} | ||
.into(), | ||
)), |
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 bit hacky but that should do the work 👍
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.
Indeed hacky, I tried to find some refactoring opportunity to consolidate this logic and the logic in the query interpreter that renders *OrThrow errors, but found it less clear than actually repeating myself.
if self.throw_on_empty() { | ||
format!("findUnique{}OrThrow", self.name) | ||
} else { | ||
format!("findUnique{}", self.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.
Would be great if we could use the schema::IdentifierType
enum here, but we can't really because we don't have access to the model. We could make that work by using a dyn trait, but it's not worth it atm. Just some out loud thoughts.
Supersedes #4014 so we can take it from here. Thank you @hayes, You will get credit in the git history for this work.