Skip to content
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

Expanded _ksListsMeta query input to take a key list #2715

Closed

Conversation

Vultraz
Copy link
Contributor

@Vultraz Vultraz commented Apr 12, 2020

I was thinking more about the where input I added to _ksListsMeta in #2664, and it occurred to me that that change was perhaps not the most useful. If you already know the key you want - for example, User - you could just do a _UsersMeta query instead of _ksListsMeta(where: { key: "User" }).

This expands key to accept an array, so as to be more versitile:

query {
  _ksListsMeta(where: { key: ["User", "Todo"] }) {
    name
  }
}

gives

{
  "data": {
    "_ksListsMeta": [
      {
        "name": "User"
      },
      {
        "name": "Todo"
      }
    ]
  }
}

Though, admittedly, one could still just do...

query {
  _UsersMeta {
    name
  }
  _TodosMeta {
    name
  }
}

So perhaps the where input still isn't that useful after all... 🤔 @jesstelford thoughts?

@changeset-bot
Copy link

changeset-bot bot commented Apr 12, 2020

🦋 Changeset is good to go

Latest commit: 769a7a4

We got this.

This PR includes changesets to release 1 package
Name Type
@keystonejs/keystone Patch

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

@Vultraz Vultraz force-pushed the expand-list-meta-query-input branch from 84cd682 to 270f820 Compare May 27, 2020 01:23
@Vultraz
Copy link
Contributor Author

Vultraz commented May 27, 2020

Added an aux filter param too:

_ksListsMeta(where: { auxiliary: false })

@Vultraz Vultraz changed the title Expanded _ksListsMeta query input to take a key list Expanded _ksListsMeta query input to take a key list, added aux filter May 27, 2020
@Vultraz
Copy link
Contributor Author

Vultraz commented May 27, 2020

Async callback not invoked failure.

@MadeByMike
Copy link
Contributor

Hmmm I'm less sure we need this one @Vultraz, not sure we needed the where either... 🤔

Isn't this exactly why GraphQL and not rest:

query {
  _UsersMeta {
    name
  }
  _TodosMeta {
    name
  }
}

Why not make GraphQL the API?

@Vultraz
Copy link
Contributor Author

Vultraz commented May 27, 2020

Originally, I was just implementing #1279 as @jesstelford requested. Admittedly, there is some benefit to being able to just pass in an array of list keys without manually constructing the query, but I'm not sure I'll actually need to use that.

The auxiliary flag is definitely useful, though, since the admin UI doesn't need aux lists.

@MadeByMike
Copy link
Contributor

Yep auxiliary is good. I'd merge that right away.

I want to get another opinion on adding the array. Writing a larger query is not too bad and you could write a helper to generate it if you needed it for huge amounts of lists.

Up to you if you want to merge auxiliary now or wait for more input on the array input. Other people have stronger opinions of the shape of this API than me.

@Vultraz
Copy link
Contributor Author

Vultraz commented May 27, 2020

Split the auxiliary change into #3052

@Vultraz Vultraz changed the title Expanded _ksListsMeta query input to take a key list, added aux filter Expanded _ksListsMeta query input to take a key list May 27, 2020
@Vultraz Vultraz force-pushed the expand-list-meta-query-input branch from b260f2f to 769a7a4 Compare May 27, 2020 02:53
@MadeByMike MadeByMike closed this Jul 9, 2020
@MadeByMike
Copy link
Contributor

I think we arrived at not needing this. Closing.

@Vultraz Vultraz deleted the expand-list-meta-query-input branch July 9, 2020 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants