torchbox / wagtail-grapple

A Wagtail app that makes building GraphQL endpoints a breeze!
https://wagtail-grapple.readthedocs.io/en/latest/
Other
152 stars 57 forks source link

contentType filter on a Page doesn't work #170

Closed sks444 closed 3 years ago

sks444 commented 3 years ago

Query:

query {
  pageByID: page(id: 5) {
    contentType
  },
  pageByContentType: page(contentType: "about.AboutPage") {
    id
  }
}

Response:

{
  "data": {
    "pageByID": {
      "contentType": "about.AboutPage"
    },
    "pageByContentType": null
  }
}

Looks like we can do page = qs.first() after this line. So whenever there is only contentType filter passed we return the first page.

zerolab commented 3 years ago

Having seen this example in isolation, I don't think the content type filter makes sense on the page query. The page query is intendend to return a specific page matching given criteria. content type in isolation is like saying "give me the about page with.." and stopping there. What page? first, last? based on what criteria?

sks444 commented 3 years ago

Yeah..that's valid. contentType filter on it's own on a page doesn't make sense.

We just wanted to utilize this filter to get a page which just have one instance. As we can not hard code id, slug in frontend as those can be changed. Not sure if there is any other way to get a page with single instance.

sks444 commented 3 years ago

I suppose we can use it on pages with limit set to 1.

query {
  pages(contentType: "about.AboutPage" limit: 1) {
    ...on AboutPage {
      slug
    }
  }
}

So this issue should be resolved.

Although I do have one more doubt. Actually we have multi site setup and we are doing these queries under a site. But the support for contentType filter on pages under a site is not yet there. Do you think it would be helpful to have this filter there as well? So we can do something like:

query {
  site(hostname: "localhost") {
    pages(contentType:"about.AboutPage") {
      slug
    }
  }
}
zerolab commented 3 years ago

Hey @sks444 it does make sense to have parity between the global pages search and the site query one, where they make sense.

Are you happy to submit a PR with that?

sks444 commented 3 years ago

Sure. Will try to fix this one.

sks444 commented 3 years ago

Hi @zerolab, I added a pr for this over at https://github.com/GrappleGQL/wagtail-grapple/pull/171. Let me know your views.

zerolab commented 3 years ago

Closing as per https://github.com/GrappleGQL/wagtail-grapple/issues/170#issuecomment-826076611

171 fixes the use case outlined in https://github.com/GrappleGQL/wagtail-grapple/issues/170#issuecomment-826079726