-
Notifications
You must be signed in to change notification settings - Fork 800
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
Add/jetpack search custom results #36378
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Search plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
3046218
to
8b04349
Compare
fd12486
to
b9e9d24
Compare
I wonder if we should get some feedback from Crew on this diff in general. I feel like there may be ways to improve the UX of this filter. For instance, could we have a call something like:
Maybe we don't want that due to wanting to support both posts and comments, but it feels like it would be good to make it more readable and allowing easily reviewing a potentially long set of customizations. All of these customizations will also show up in the page source, which is probably fine, but feels a bit weird. |
I see your point, but I also feel like this is something we could add later. I developed this feature in this way, mimicking the It is also possible that there is a better solution going forward than the way that |
30c7637
to
4d491bb
Compare
This PR has been marked as stale. This happened because:
If this PR is still useful, please do a [trunk merge or rebase](https://github.com/Automattic/jetpack/blob/trunk/docs/git-workflow.md#keeping-your-branch-up-to-date) and otherwise make sure it's up to date and has clear testing instructions. You may also want to ping possible reviewers in case they've forgotten about it. Please close this PR if you think it's not valid anymore — if you do, please add a brief explanation. If the PR is not updated (or at least commented on) in another month, it will be automatically closed. |
0fefdd4
to
3a3d75e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this Robert! This is a feature users longing for 👍
It tests well and looks good. It doesn't break without the diff+filters , so I think it's safe to ship. I'd avoid the current release if possible tho.
@kangzj - thanks for testing. I don't think it is urgent to get into the latest release. What do I need to do to put it into the next release? I would like to merge before it gets stale, but then release whenever makes sense. |
* Adding option to specify custom results for jetpack search * Got it working * changelog * Added ability to use regex * changelog * changed data structure * changelog * fixed logic in api * updated TrainTracks events to give more information about the ui-algo * updated version numbers * fixed default value for customResults to empty array * fixed rebase bug
We frequently get requests from website owners who would like specific posts to be returned for specific queries. This can be thought of as "pinned posts" or "sponsored posts". This change adds the ability to specify which posts should be put at the top of search results for given queries.
Proposed changes:
Other information:
The queries and matching documents can be specified in PHP, e.g. in a theme's
functions.php
file like soNote that the document ids are of the form `<blog_id>-p-<post_id>'. This format is relatively easy to understand, though it does require one to know the blog_id of their site. This can be found by inspecting the queries to the public API when performing a search in the developer tools of a browser. The format is also flexible enough such that if you are searching across multiple blogs simultaneously, it allows one to specify pinned posts from multiple blogs. Also note that the order in which the ids are coded is the order in which the results will be returned. It is possible to specify up to 100 documents IDs for a given query pattern. If you specify more, it will simply be truncated.
There are two types of queries - by default, we will use exact matching. However, it is also possible to specify a query with a regular expression by preceding the query with "regex:". The regexes are evaluated anchored to the beginning and end of the query, such that "regex:hello.world" will match the query string "hello beautiful world", but not "i say hello to the world" or "hello world i love you". If you would like to get rid of this anchoring, you can add "." at the beginning or end of your pattern (or both). This is similar to how regular expressions work in Java. The reasoning behind this is that regular expressions are complicated and it is easy for inexperience developers to get in trouble with them, so we are trying to avoid that as much as possible. Note that the order in which the rules are added is important. The first matching rule will be used.
Jetpack product discussion
For now, I think we should only implement this with advanced users in mind, thus requiring PHP code. In the future, we might want to create a UI/UX to allow users to add rules in wp-admin.
We may also want to add a way to mark that these posts are being artificially added to the top of the search results.
Does this pull request change what data or activity we track or use?
Testing instructions:
In order to test this you will need a Jetpack site with Jetpack Search installed and active. In order to activate Jetpack Search you must have a user connection and purchase it. You can purchase the free plan. You will need to make sure that the site is properly synced and that the site has been indexed. You can check this by looking at the Jetpack Debugger or by looking at the Jetpack Search settings page, which will tell you how many documents have been indexed.
You also need to sandbox the public api, and apply this patch on your sandbox: D165510-code
After applying that patch, edit
public.api/rest/wpcom-json-endpoints/class.wpcom-json-api-search-site-v1-3-endpoint.php
adding a line like this around line 561: Use whatever blog_id you are testing with.We are only allowing the custom results functionality on a limited number of sites right now. That is why you need to add it manually there.
Add several posts of your choosing. I tested by adding a post with the title "hello universe". I then added a rule saying that searching for "hello world" should return the "hello universe" post on top, even though the default "hello world" post would be a better match. I added this in my themes functions.php file like so:
After adding this to the functions.php file, if you perform a search on your site, you should see the "hello universe" post come out as the top result