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

ACM Models not included in search results or sitemaps #516

Closed jasonbahl closed 7 months ago

jasonbahl commented 2 years ago

To reproduce

  1. create an ACM model and set it to "public"
  2. add some content to the model
  3. check your sitemap.xml or execute a search for content in the model and inspect the SQL that was executed

Also, to compare, register a post type directly in WordPress with PHP, and do the same steps:

register_post_type( 'custom_rabbit', [
    'public' => true,
    'label' => 'Custom Rabbits',
    'show_ui' => true,
    'publicly_queryable' => true,
    'show_in_rest' => true,
    'show_in_graphql' => true,
    'graphql_single_name' => 'CustomRabbit',
    'graphql_plural_name' => 'CustomRabbits'
]);

Expected behavior

I would expect ACM models that are set to "Public" to be shown in the sitemap.

I would expect search queries to search for public post types created by ACM.

Screenshots

I have an ACM Model named "Rabbits" that is set to public

CleanShot 2022-05-04 at 11 04 49

I created a Rabbit in this model:

CleanShot 2022-05-04 at 11 24 53

I also created a Rabbit in my "custom_rabbit" post type:

CleanShot 2022-05-04 at 11 34 06

I cannot see "Rabbits" in my sitemap.xml, but I do see the "custom_rabbits" in there:

CleanShot 2022-05-04 at 11 25 37

If I execute a search, for example mysite.com/?s=rabbit, and use Query Monitor to inspect the SQL that was generated, I see the following SQL:

SELECT SQL_CALC_FOUND_ROWS wp_posts.ID
FROM wp_posts
WHERE 1=1
AND (((wp_posts.post_title LIKE '%rabbit%')
OR (wp_posts.post_excerpt LIKE '%rabbit%')
OR (wp_posts.post_content LIKE '%rabbit%')))
AND ((wp_posts.post_type = 'post'
AND (wp_posts.post_status = 'publish'
OR wp_posts.post_status = 'private'))
OR (wp_posts.post_type = 'page'
AND (wp_posts.post_status = 'publish'
OR wp_posts.post_status = 'private'))
OR (wp_posts.post_type = 'attachment'
AND (wp_posts.post_status = 'publish'
OR wp_posts.post_status = 'private'))
OR (wp_posts.post_type = 'custom_rabbit'
AND (wp_posts.post_status = 'publish'
OR wp_posts.post_status = 'private')))
ORDER BY wp_posts.post_title LIKE '%rabbit%' DESC, wp_posts.post_date DESC
LIMIT 0, 10

This SQL doesn't attempt to search for the post_type = rabbit post type, but it does search for the "custom_rabbit" post type

We can see the search results, which do not include the content in ACM Rabbit (the SQL didn't even try to retrieve it).

CleanShot 2022-05-04 at 11 26 42

Version information

Additional context

There's some further discussion about public/private/publicly_queryable content and how it should be treated in WPGraphQL here that might be relevant: https://github.com/wp-graphql/wp-graphql/pull/2355

mindctrl commented 2 years ago

This is due to us registering post types with public => false, regardless of public/private setting (which is for API Visibility specifically and not "public" in the way WP uses the word), which is an artifact of how the project originally started. That is to say "never expose content publicly on the site" and "target headless only users via REST and GraphQL" only. Other ways of accessing the data were not part of the scope and actively avoided/ignored. That was done alongside the headless plugin, now called FaustWP, where we built in controls to prevent people from hitting the front-end of the WP site at all to force the API-only approach, avoid duplicate content on WP and JS site, etc.

Later on, the REST part of "target headless only users via REST and GraphQL" was dropped, effectively deprioritizing additional REST API improvements in favor of WPGraphQL. If I recall correctly, research showed headless developers were almost all using WPGraphQL. Even still we've tried to maintain some parity between the two APIs in terms of functionality and accessibility of data.

Maybe it's time we reconsider and register with public => true instead, or at the very least have that particular setting controlled by the public/private API Visibility setting like how the publicly_queryable argument is handled. However, I think that would technically make the API Visibility do more than the name implies and, in the case of existing sites using ACM, could expose data unintentionally after an update that changed the way the setting works. If we decide to do it, it would be a good change to make before releasing version 1.0.

We haven't been given guidance from the product manager to support non-headless use cases. We do target WP devs, but in the headless context (as of now). "Atlas" being in the name of the product, it's worth at least considering to what extent we deviate from headless use cases and support things like front end /?s=xx type queries. Even still, maybe this change makes sense regardless, particularly if it solves problems related to the oddities and misunderstandings of register_post_type() behavior.

The sitemap thing is interesting, given the above context. 🤔

I'm seeking some clarification and guidance from the PM. Will update when I know more.

jasonbahl commented 2 years ago

RE: Sitemaps

Ya, I believe that one of the solutions that @blakewilson has explored for Sitemaps in Faust is proxying the WordPress sitemap in some fashion, which would mean the Faust sitemaps only would include public content that's also included in the WordPress sitemaps.

Also, a lot of plugins, including ACM, etc that have Post Picker fields, populate the fields using get_post_types( [ 'public' => true ])

RE: Linking to other content

Another thing I just noticed, is that when writing content in another post type (in my case a "Page") if I go to include a link to content in my site, ACM models are not showing in the Link UI. My "Test rabbit" in my "custom_rabbit" post_type shows, but my ACM models do not show.

CleanShot 2022-05-04 at 17 05 23

mindctrl commented 2 years ago

Note: we're planning to do 'public' => true as part of a 1.0 release. No ETA on that yet, but we're close. We're just aligning with the rest of the org as far as I understand it.

@jasonbahl thanks for sharing these issues and for talking through all the things here, in Slack, and in the WPGraphQL repo. ❤️

mindctrl commented 7 months ago

Since ACM has entered an end-of-life phase, we're not going to make this change to the plugin. It's too much of a breaking change for consideration on a project without a future. 😢