Tests will be created soon 🤞

Tests will be created soon 🤞

@blakewilson were you waiting on me for anything before you write tests? let me know 🙏🏻

Tests will be created soon 🤞

@blakewilson were you waiting on me for anything before you write tests? let me know 🙏🏻

@jasonbahl No, I'm currently trying to resolve some failing tests. It looks like the changes in this PR have some unintended side effects, so trying to get those resolved before writing new tests.

uh oh. I was working on updating tests to use WPGraphQLTestCase, and now a bunch of things are broken. Probably my fault 🤦🏻‍♂️

uh oh. I was working on updating tests to use WPGraphQLTestCase, and now a bunch of things are broken. Probably my fault 🤦🏻‍♂️

The last action to run before your latest commits also wasnt working (https://github.com/wp-graphql/wp-graphql/actions/runs/6398962072/job/17370176424) , so the cause might still be outside that diff.

uh oh. I was working on updating tests to use WPGraphQLTestCase, and now a bunch of things are broken. Probably my fault 🤦🏻‍♂️

The last action to run before your latest commits also wasnt working (https://github.com/wp-graphql/wp-graphql/actions/runs/6398962072/job/17370176424) , so the cause might still be outside that diff.

Weird. When I ran these independently they worked, wonder if something is conflicting. I can look into this

@blakewilson I am working with @josephfusco to debug the test suite issues. We believe it's something to do with the setup/tear down of some other suite(s)

@blakewilson

I added this to both of Blakes tests:

$this->set_permalink_structure( '' );

(and copied that method from here: https://github.com/wp-graphql/wp-graphql/blob/develop/tests/wpunit/PostObjectQueriesTest.php#L88-L93)

and they pass.

So it's a conflict with permalinks. Probably need to manually investigate why it works with default permalinks but not other pretty permalinks.

@blakewilson

I added this to both of Blakes tests:

$this->set_permalink_structure( '' );

(and copied that method from here: https://github.com/wp-graphql/wp-graphql/blob/develop/tests/wpunit/PostObjectQueriesTest.php#L88-L93)

and they pass.

So it's a conflict with permalinks. Probably need to manually investigate why it works with default permalinks but not other pretty permalinks.

Awesome work @jasonbahl @josephfusco! Thanks for looking into this.

So does this mean that there is something else in WPGraphQL that needs to be looked at to fix the permalinks issue? Does this affect all "query by URI" things, or just things relating to this PR? Any suggestions on places to look first?

@blakewilson im not sure yet. We diagnosed the issue but didn't have a chance to manually test further with different permalinks.

My hunch is that the resolve_by_uri() needs some work to resolve properly when asPreview is true and various permalink structures are in play

@blakewilson ok, so I manually tested and it looks like the issue is related to draft posts. Since draft posts aren't public, they don't have a "pretty permalink" issued (WordPress doesn't make them publicly visible by URI) so I think idType: URI won't work with draft posts.

We might be able to fix it under the hood, and I'll keep digging, but here's the gist of what's happening:

Published Post

If I click "preview" on a published post, I get a URL like so: /blog/2023/10/09/published/?preview_id=6866&preview_nonce=1e626c85ac&preview=true&_thumbnail_id=3390

I can take that URI and pass it to the query like so:

{
  post(id: "/blog/2023/10/09/published/?preview_id=6866&preview_nonce=1e626c85ac&preview=true&_thumbnail_id=3390", asPreview: true, idType: URI) {
    id
    isPreview
    databaseId
    uri
  }
}

And I get the preview node as expected:

Draft Posts

If I click "preview" on a draft post, I get a link like so (no pretty permalink, even though I have pretty permalinks enabled): /?p=6874&preview=true

If I pass this URI to the query:

{
  post(id: "/?p=6874&preview=true", asPreview: true, idType: URI) {
    id
    isPreview
    databaseId
    uri
  }
}

I get null:

The tests are making assertions with pretty permalinks enabled (carried over from the PostObjectQueriesTest and never reset), which I think has exposed that this feature isn't fully working as expected.

That said, I might argue that instead of passing the following args to the query: id: "/?p=6874&preview=true", asPreview: true, idType: URI the client might be better off parsing the query params and passing the following query:

{
  post(id: "6874", asPreview: true, idType: DATABASE_ID ) {
    id
    isPreview
    databaseId
    uri
  }
}

Which does work:

So, as a user I could click "preview" on a draft post.

The client app (Faust, etc) could parse query params, if there are any, and decide what query to send.

For example, Faust could determine that preview=true exists and set the asPreview: true in the query then determine that p=${id} or preview_id=${id} exists and use those as the values for id: $p or id: $preview_id, and set idType: DATABASE_ID.

@blakewilson ok, I think I might have figured it out.

2 things:

1. The tests were using get_permalink( $draft_id ) which doesn't match the link you get when you click preview on a draft, so I updated to get_preview_post_link( $draft_id ).

2. There was a bug where the WP_Query was receiving both 'p' => $id and 'pagename' => '' and this was causing WP_Query to generate SQL queries like:

SELECT
    wp_posts.*
FROM
    wp_posts
WHERE
    1=1
    AND (wp_posts.ID = '0')
    AND wp_posts.ID = 6874
    AND wp_posts.post_type = 'post'
ORDER BY
    wp_posts.post_date DESC;

There's no way we'll find a post where wp_posts.ID = '0' AND wp_posts.ID = 6874.

I added some logic that determines if the parsed_url has query params, and if so, we check if the query_params are equal to the "pagename" and if so, we unset "pagename" because that means the path is only query params and nothing else.

This leads to the proper query being generated without the AND (wp_posts.ID = '0') line, allowing the resolver to continue and return the node, or the preview node if asPreview => true

I tested this locally, both idType: SLUG & idType: URI are working as expected.

Code Climate has analyzed commit 8d252f1 and detected 2 issues on this pull request.

Here's the issue category breakdown:

View more on Code Climate.