-
Notifications
You must be signed in to change notification settings - Fork 163
add query rules gui page and the menu option #2252
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
Conversation
🔍 Preview links for changed docs |
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.
Looks good from my side. I have added small questions.
You can do the following using the GUI without the need of any API calls: | ||
|
||
* For a new deployment setup, create new rules | ||
* For an existing deployment setup, edit and delete rules |
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.
creating new rules, editing and deleting rules are available for all deployments.
It feels like this text suggest you can create new rules only on new deployments.
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.
got it. I'll fix that.
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 iterating, I still have a few issues with the opening sections, and a few minor suggestions elsewhere :)
solutions/search/query-rules-ui.md
Outdated
|
||
# Query rules UI | ||
|
||
Query rules UI is a user interface that helps you create, edit, and delete query rules. The interface is implemented on top of the existing Query Rules API and provides non-technical users with an intuitive, streamlined interface to manage query rules. |
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.
"implemented on top of the existing Query Rules API" is a technical implementation detail, and might be TMI.
provides non-technical users with an intuitive, streamlined interface to manage query rules.
The docs aren't the place to say things like "intuitive, streamlined interface", the language is too vague and marketing-like.
I also don't understand why we have a sentence where one half is a technical implementation detail and the second half sounds like sales blurb. The tone just reads wrong.
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.
An alternative phrasing for the second half would be The Query rules UI provides search managers and content teams with a graphical interface to manage these rules without writing API calls or JSON configuration.
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.
This is a little confusing Liam, because I think you mentioned we don't want to specify types of users and leave that to users to self select. Let me know if I understood that right.
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.
Fair, I was trying to give an idea of a suitable alternative to non-technical users
if you were dead set on communicating that user category more specifically, but I agree we can just remove search managers and content teams with
from my suggestion. I made it clear in my first review that calling out "non-technical users" specifically was a no-no, because it's an ill-defined term. So I had to repeat myself here because this wasn't rectified.
And my suggestion addresses more issues than just "don't say this is for non-technical users", it also addresses the inappropriate sales/marketing stylelanguage.
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.
yes, I had erased that already. I think there was a bad commit there which is why you didn't see the change.
Co-authored-by: Liam Thompson <leemthompo@gmail.com>
Co-authored-by: Liam Thompson <leemthompo@gmail.com>
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.
Approving to unblock, with a few final suggestions. Please feel free to wait until tomorrow to polish off. Thanks for iterating, ideally we'd have a bit more time for the review feedback loop to be less rushed, but it's all good. Thanks @ketkee-aryamane!
Co-authored-by: Liam Thompson <leemthompo@gmail.com>
@ketkee-aryamane as a follow-up to this PR, we should add links from the API pages to the new Query Rules UI page :) |
yes, had that on mind. But I thought we would do it after the release. Do you suggest I do that today before the release? |
@ketkee-aryamane doesn't have to be today strictly, can create an issue to track if necessary :) |
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.
I know this has been merged already, but here is my followup feedback as requested by @ketkee-aryamane
--- | ||
|
||
# Query rules UI | ||
Use query rules to boost, pin, or exclude specific documents when queries contain certain keywords, phrases, or match defined search patterns. |
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.
This is incorrect, we only support pinning or excluding. When I read boost I don't think of pinning documents, I think of increasing the likelihood that they show up in the results, and unfortunately this is not supported. You could consider linking here to https://www.elastic.co/docs/reference/elasticsearch/rest-apis/searching-with-query-rules for more information about query rules as well. (I know you link to it further down)
|
||
The UI enables you to: | ||
|
||
- Set keyword triggers and conditions for when rules apply |
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.
Keyword or numerical conditions such as less than or greater than 😉
The UI enables you to: | ||
|
||
- Set keyword triggers and conditions for when rules apply | ||
- Pin, boost, or exclude specific documents in results |
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.
Again, we should remove boost here.
|
||
The Query Rules UI provides the same functionality as the API with one key difference in how documents are pinned: | ||
|
||
* The UI defaults to `docs` for maximum flexibility, but still allows `id`-based pinning for single-index searches through a simplified form. |
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.
This is a little confusing wording to me - IIRC ids were not that well supported in the UI, you can't edit an id based rule for example. Also I don't think the UI allows you to create an empty docs rule, which would behave in the same way as an id rule.
If you want to get full access to the Query Rules UI, you must have the following privileges: | ||
|
||
* Appropriate roles to access Kibana. For more information, refer to [Built-in roles](https://www.elastic.co/docs/deploy-manage/users-roles/cluster-or-deployment-auth/built-in-roles) or [Kibana privileges](https://www.elastic.co/docs/deploy-manage/users-roles/cluster-or-deployment-auth/kibana-privileges) | ||
* A custom role with `manage_search_query_rules` cluster privilege |
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.
I think we should think about removing custom
because some power user roles will have that by default, no?
|
||
## Accessing the Query Rules UI | ||
|
||
Go to your deployment and select **Query Rules** from the left navigation menu under **Relevance**. If you're not able to see the option, contact the administrator to review the role assigned to you. |
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.
I would replace this with contact *your* administrator
Go to your deployment and select **Query Rules** from the left navigation menu under **Relevance**. If you're not able to see the option, contact the administrator to review the role assigned to you. | ||
|
||
:::{image} /solutions/images/elasticsearch-query-rules-ui-home.png | ||
:alt: Landing page for Query Rules UI. |
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.
Warning, screenshots in docs is dangerous, because it has to be updated if and when the UI inevitably changes 😁
Linked to Document Query Rules UI
added a new page for the GUI, added the menu option on the left side