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

Return expected data when querying seo data of custom taxonomy #57

Merged
merged 7 commits into from
Jun 16, 2023

Conversation

lucguerraz
Copy link
Contributor

What

When querying a custom taxonomy the seo data is wrong or is null. Expected behaviour would be for it to return the data set in the taxonomy

Why

When querying a custom taxonomy to be able to display a taxonomy archive with a query such as this one:

query ExampleQuery {
  exampletax(id: "example-tax-1", idType: SLUG) {
    id
    name
    slug
    examplecpts {
      nodes {
        title
        slug
      }
    }
    seo {
      title
      robots
      description
      canonicalUrl
      openGraph {
        locale
        type
        title
        description
        url
        siteName
        updatedTime
        image {
          url
          width
          height
          type
        }
        twitterMeta {
          card
          title
          image
        }
      }
    }
  }
}

the returned data is either wrong or null, such as this:

{
  "data": {
    "exampletax": {
      "id": "dGVybToy",
      "name": "Example Tax #1",
      "slug": "example-tax-1",
      "examplecpts": {
        "nodes": [
          {
            "title": "Example Post #2",
            "slug": "example-post-2"
          },
          {
            "title": "Example Post #1",
            "slug": "example-post-1"
          }
        ]
      },
      "seo": {
        "title": null,
        "robots": [
          "follow",
          "index"
        ],
        "description": null,
        "canonicalUrl": null,
        "openGraph": {
          "locale": "EN_US",
          "type": "article",
          "title": null,
          "description": null,
          "url": null,
          "siteName": "RankMath GraphQL Example",
          "updatedTime": null,
          "image": null,
          "twitterMeta": {
            "card": "SUMMARY_LARGE_IMAGE",
            "title": null,
            "image": null
          }
        }
      }
    }
  }
}

After applying my proposed fix the returned data is as expected:

{
  "data": {
    "exampletax": {
      "id": "dGVybToy",
      "name": "Example Tax #1",
      "slug": "example-tax-1",
      "examplecpts": {
        "nodes": [
          {
            "title": "Example Post #2",
            "slug": "example-post-2"
          },
          {
            "title": "Example Post #1",
            "slug": "example-post-1"
          }
        ]
      },
      "seo": {
        "title": "Example Tax #1 Archives - RankMath GraphQL Example",
        "robots": [
          "follow",
          "index",
          "max-snippet:-1",
          "max-video-preview:-1",
          "max-image-preview:large"
        ],
        "description": null,
        "canonicalUrl": "http://localhost:8888/exampletax/example-tax-1/",
        "openGraph": {
          "locale": "EN_US",
          "type": "article",
          "title": "Example Tax #1 Archives - RankMath GraphQL Example",
          "description": "This is a short description for the Example Tax #1",
          "url": "http://localhost:8888/exampletax/example-tax-1/",
          "siteName": "RankMath GraphQL Example",
          "updatedTime": null,
          "image": null,
          "twitterMeta": {
            "card": "SUMMARY_LARGE_IMAGE",
            "title": "Example Tax #1 Archives - RankMath GraphQL Example",
            "image": null
          }
        }
      }
    }
  }
}

How

By adding a wp() function call to the setup_post_head method of the Model\Seo class this resolves the problem. The inline comment of this method mentions this being a shim of an internal RankMath method, the RankMath method calls this wp() function to complete it's job. After trying it, I discovered that this resolved the problem without any observable ill effects.

Testing Instructions

Create a custom Post type and a custom taxonomy, query the custom taxonomy archive to get the seo data

Additional Info

Checklist:

  • My code is tested to the best of my abilities.
  • My code follows the WordPress Coding Standards.
  • My code has proper inline documentation.
  • I have added unit tests to verify the code works as intended.
  • The changes in this PR have been noted in CHANGELOG.md

@justlevine
Copy link
Member

Thanks for the PR @lucguerraz !

It seems calling wp() out of context is causing the tests to fail, although it's unclear at a glance whether this is due to Rank Math's usage of static variable interfering with the tests themselves or an actual production bug caused by trying to reset the global wp() instance inside the model 🤔.

If you don't beat me too it, I'll investigate as soon as I get a free moment.

cc: #40

@justlevine justlevine added the status: needs review 🧐 Needs to be reviewed by a codeowner label Jun 15, 2023
@justlevine
Copy link
Member

justlevine commented Jun 16, 2023

Hey @lucguerraz

Thanks to your use of wp(), I was able to track down and address the issue on the locally-scoped query used by the individual TermNodeSeoModel.

Would you be able to pull down the changes to this pr and confirm things are now working?

@lucguerraz
Copy link
Contributor Author

Hi @justlevine

Thanks for taking a look, your new changes work exactly the same as when I opened the pull request (but without the errors obviously).

So if you ask me, you can merge this PR :)

@justlevine
Copy link
Member

Test is failing on WP6.2 + PHP 8.1 due to an upstream issue with docker (ref: docker-library/wordpress#833).

Gonna consider this safe to merge anyway due to the other combos passing.

Since this case happens to be responsible for CI, theres no coverage report. From local:
image

@justlevine justlevine merged commit 6f8216f into AxeWP:develop Jun 16, 2023
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs review 🧐 Needs to be reviewed by a codeowner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants