-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Admin UI: big cleanup of query construction #2976
Admin UI: big cleanup of query construction #2976
Conversation
🦋 Changeset is good to goLatest commit: b45573b We got this. This PR includes changesets to release 29 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
This all looks fantastic. The one change to make is that this is actually a major
version bump on each package, since the API of getFilterGraphQL
has changed from returning a string to an object, which is a breaking change.
@timleslie done 👍 |
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.
Yep, looks good 👍
And now, my big weekend PR.
This refactors the Admin UI's list and item query construction. Instead of using complicated and very hard-to-read string concatenation, this makes use of query variables wherever possible.
id
argument.search
,where
,skip
,first,
andsortBy
). The newgetListQuery
function only takesfields
to control which fields we're querying since that can't be done via variables. Even better, using variables makes it clear exactly what arguments are accepted and their types.where
filters are just passed in via variables now,getFilterGraphQL
doesn't need to handle messy strings anymore and can just return plain objects!gqlCountQueries
was removed and replaced with a helper inHome/index.js
.