wpengine / atlas-content-modeler

Content modeling plugin for WordPress
https://developers.wpengine.com/docs/atlas-content-modeler
GNU General Public License v2.0
165 stars 13 forks source link

SQL Generated for post queries connected to users has errors #347

Closed jasonbahl closed 2 years ago

jasonbahl commented 3 years ago

The SQL Query generated for WPGraphQL connections from a user to posts of any post_type includes an error.

This appears to only be the case when using ACM to generate post types. I believe it's something to do with the Post 2 Post mechanism included in ACM, but not quite sure yet.

To reproduce

  1. create a "rabbits" model in ACM (see exported JSON below)
  2. create a few rabbits
  3. query for users, then their connected rabbits in GraphQL
  4. inspect the GraphiQL Query Log
  5. see the SQL generated with an error (test the query in TablePlus, for example)

Expected behavior

When posts of a custom post type are queried in connection to their author, I expect to see posts of said type.

I can get correct behavior when registering the post_type via code or via Custom Post Type or CPT UI, then making the same query.


GraphQL Query:

{
  users {
    nodes {
      id
      name
      rabbits {
        nodes {
          databaseId
        }
      }
    }
  }
}

JSON export of ACM models:

{"book":{"show_in_rest":true,"show_in_graphql":true,"singular":"Book","plural":"Books","slug":"book","api_visibility":"public","model_icon":"dashicons-book-alt","description":"A collection of books.","fields":{"1629223852861":{"show_in_rest":true,"show_in_graphql":true,"type":"text","id":"1629223852861","position":"0","name":"Title","slug":"title","required":true,"isTitle":true,"inputType":"single","minChars":"","maxChars":""},"1629223878399":{"show_in_rest":true,"show_in_graphql":true,"type":"number","id":"1629223878399","position":"10000","name":"Price","slug":"price","required":true,"minChars":"","maxChars":"","numberType":"decimal","minValue":"","maxValue":"","step":""},"1629223889389":{"show_in_rest":true,"show_in_graphql":true,"type":"richtext","id":"1629223889389","position":"20000","name":"Description","slug":"description","minChars":"","maxChars":""}}},"cat":{"show_in_rest":true,"show_in_graphql":true,"singular":"Cat","plural":"Cats","slug":"cat","api_visibility":"public","model_icon":"dashicons-admin-post","description":"Some description","fields":{"1631655608229":{"show_in_rest":true,"show_in_graphql":true,"type":"text","id":"1631655608229","position":"0","name":"Name","slug":"name","required":true,"isTitle":true,"inputType":"single","minChars":"","maxChars":""},"1635113542370":{"show_in_rest":true,"show_in_graphql":true,"type":"number","id":"1635113542370","position":"10000","name":"phone number","slug":"phoneNumber","required":false,"minChars":"","maxChars":"","numberType":"integer","minValue":"","maxValue":"","step":""}}},"dog":{"show_in_rest":true,"show_in_graphql":true,"singular":"Dog","plural":"Dogs","slug":"dog","api_visibility":"public","model_icon":"dashicons-admin-post","description":"","fields":{"1631805426026":{"show_in_rest":true,"show_in_graphql":true,"type":"text","id":"1631805426026","position":"0","name":"name","slug":"name","required":false,"isTitle":false,"inputType":"single","minChars":"","maxChars":""}}},"car":{"show_in_rest":true,"show_in_graphql":true,"singular":"Car","plural":"Cars","slug":"car","api_visibility":"public","model_icon":"dashicons-admin-post","description":"Collection of Cars","fields":{"1634053800408":{"show_in_rest":true,"show_in_graphql":true,"type":"text","id":"1634053800408","position":"0","name":"Make","slug":"make","required":true,"isTitle":true,"inputType":"single","minChars":"","maxChars":""},"1634053810077":{"show_in_rest":true,"show_in_graphql":true,"type":"text","id":"1634053810077","position":"10000","name":"Model","slug":"model","required":false,"isTitle":false,"inputType":"single","minChars":"","maxChars":""}}},"rabbit":{"show_in_rest":true,"show_in_graphql":true,"singular":"Rabbit","plural":"Rabbits","slug":"rabbit","api_visibility":"public","model_icon":"dashicons-admin-post","description":"","fields":{"1636386434879":{"show_in_rest":true,"show_in_graphql":true,"type":"text","id":"1636386434879","position":"0","name":"Name","slug":"name","isTitle":true,"inputType":"single","required":true,"description":"","minChars":"","maxChars":""}}},"cage":{"show_in_rest":true,"show_in_graphql":true,"singular":"Cage","plural":"Cages","slug":"cage","api_visibility":"public","model_icon":"dashicons-admin-post","description":"","fields":{"1636386459277":{"show_in_rest":true,"show_in_graphql":true,"type":"text","id":"1636386459277","position":"0","name":"Name","slug":"name","isTitle":true,"inputType":"single","required":true,"description":"","minChars":"","maxChars":""},"1636388389604":{"show_in_rest":true,"show_in_graphql":true,"type":"relationship","id":"1636388389604","position":"10000","name":"Rabbit","slug":"rabbit","reference":"rabbit","cardinality":"one-to-one","enableReverse":true,"reverseName":"Cage","reverseSlug":"cage","required":false,"description":"The rabbit inside the cage"}}}}

ERROR IN SQL

SELECT
    wp_posts.ID
FROM
    wp_posts
WHERE
    1 = 1
    AND(n 0 = 1 n) # <-- what is this??
    AND wp_posts.post_author IN(1)
    AND wp_posts.post_type = 'post'
    AND((wp_posts.post_status = 'publish'))
GROUP BY
    wp_posts.ID
ORDER BY
    wp_posts.post_date DESC,
    wp_posts.ID DESC
LIMIT 0,
11

Expected SQL:

SELECT
    wp_posts.ID
FROM
    wp_posts
WHERE
    1 = 1
    AND wp_posts.post_author IN(1)
    AND wp_posts.post_type = 'post'
    AND((wp_posts.post_status = 'publish'))
GROUP BY
    wp_posts.ID
ORDER BY
    wp_posts.post_date DESC,
    wp_posts.ID DESC
LIMIT 0,
11
jasonbahl commented 3 years ago

I created a video to document what I'm experiencing: https://youtu.be/TcG8CdSif90

jasonbahl commented 3 years ago

As noted in the video, this appears to impact User -> Post connections throughout, not just for models created by ACM.

Having ACM active breaks existing queries such as:

{
  users {
    nodes {
       id
       name
       posts { # posts authored by the user are not returned when ACM is active
         nodes {
           title
         }
       }
    }
  }
}
mindctrl commented 3 years ago

@jasonbahl which version of ACM are you using?

mindctrl commented 3 years ago

I can't reproduce this bug on a fresh site with just ACM and WPGraphQL installed. I imported the JSON data provided above, created a few posts, and used the queries shown here and I get the posts and rabbits expected.

jasonbahl commented 3 years ago

@mindctrl I'm using ACM v0.9.0 and WPGraphQL v1.6.7

jasonbahl commented 3 years ago

@mindctrl I'll try and isolate further. I do have a few other plugins active, so I'll spin up an environment with nothing other than ACM + WPGraphQL and see if I can reproduce.

jasonbahl commented 3 years ago

@mindctrl after going back and forth in Slack with you, we determined that the bug was related to a custom taxonomy I had registered named author.

The taxonomies aren't included in the import/export of ACM Models (probably should be or have a separate import/export for them?)

This is why it wasn't easy to reproduce on your end initially.

I had a taxonomy registered as such:

Screen Shot 2021-11-10 at 10 42 08 AM

Even though the Taxonomy is registered only to the Books Model, it appears to be affecting queries of other models, including core post types such as posts.

jasonbahl commented 3 years ago

Let me see if I can reproduce this without ACM, just registering an Author taxonomy in code with WPGraphQL 🤔

jasonbahl commented 3 years ago

Ok, I tried to reproduce by registering an author taxonomy with Custom Post Type UI, but it prevented me from creating it because of this: https://developer.wordpress.org/reference/functions/register_taxonomy/#reserved-terms

Of course, CPT UI just said I couldn't create it, I had to dig to find out why.

I think our error message could be better for users than that of CPT UI. Even I, a seasoned WordPress user, didn't know why they were rejecting this. I checked my taxonomies and I did not have an author one already, so the error message was misleading.

I realized after digging that it was a reserved keyword because of how WP_Query works, but not because it's a duplicate taxonomy as the error message made it seem.

Screen Shot 2021-11-10 at 10 53 55 AM

jason-konen-WPE commented 2 years ago

Jira ticket for this is: https://wpengine.atlassian.net/browse/MTKA-1332

mindctrl commented 2 years ago

Fixed in https://github.com/wpengine/atlas-content-modeler/pull/387