Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 798 forks source link

Search: media type is never returned even not set in excluded post type list #24681

Open kangzj opened 2 years ago

kangzj commented 2 years ago

Impacted plugin

Jetpack, Search

Steps to Reproduce

A clear and concise description of what you expected to happen.

Not so sure we should display media or not, but theoretically it should return media records.

What actually happened

No media records showing up at all.

Other information

No response

Operating System

No response

OS Version

No response

Browser

Chrome/Chromium, Firefox

Browser Version(s)

No response

robfelty commented 2 years ago

Based on my analysis of the indexing code, I don't think we have ever supported indexing attachments in Jetpack Instant Search, since we only index items with a post_status=public, and the status of attachments (media) is inherit. I think this has been the case since at least 2019. We do include some information about images within post documents though, including alt_text, so if you search for some text in an image caption, you should get results for the post that contains that image.

I recommend we get rid of the option to exclude media.

At this point, I don't think we plan to index attachments separately, for a number of reasons. The biggest reason is that it would add many documents to the index, which could cause scaling issues, and it would also increase the number of records for customers, which might inadvertently bump them into a higher paid tier.

kangzj commented 2 years ago

Thanks for the feedback @robfelty 👍

I agree there's no very good reasons to index media at least for now (I did see a plugin indexes pdf attachments). The excluded post type section picks up Media because Media is returned as a public and searchable type from get_post_types. Guess we'll have a look what's the best way to remove it.

gibrown commented 2 years ago

Based on my analysis of the indexing code, I don't think we have ever supported indexing attachments in Jetpack Instant Search, since we only index items with a post_status=public, and the status of attachments (media) is inherit

This is incorrect. Attachments can have a post status of public. We have tens of thousands of them in the current index.

kangzj commented 2 years ago

@gibrown thanks for the input. I've never seen any attachments with status public tho, any idea how this could happen? So you reckon we should revert the PR?

gibrown commented 2 years ago

Ya it seems like we should probably revert it. I am not positive how in the UI to adjust it, but maybe in the media library:

Screen Shot 2022-07-22 at 08 40 31

But any plugin could also set post_status to whatever they want.