From d7919d0d6b9dde05e057357b04f6dcb6c59c3187 Mon Sep 17 00:00:00 2001 From: Marcel Kornblum Date: Fri, 28 Jul 2023 11:53:39 +0100 Subject: [PATCH] Search improvements: souped-up explore page (#412) Can edit boost vars and view subquery boosts in action from the page --- src/extended_search/managers/__init__.py | 8 +- src/extended_search/settings.py | 2 +- src/extended_search/tests/test_settings.py | 4 +- src/search/templates/search/explore.html | 148 ++++++++++++++++-- .../search_results_single_category.html | 10 +- src/search/views.py | 87 +++++++++- 6 files changed, 227 insertions(+), 32 deletions(-) diff --git a/src/extended_search/managers/__init__.py b/src/extended_search/managers/__init__.py index a67ed9ce8..501f55da1 100644 --- a/src/extended_search/managers/__init__.py +++ b/src/extended_search/managers/__init__.py @@ -12,17 +12,17 @@ def get_indexed_field_name(model_field_name, analyzer): return f"{model_field_name}{field_name_suffix}" -def get_search_query(cls, query_str, model_class, *args, **kwargs): +def get_search_query(index_manager, query_str, model_class, *args, **kwargs): """ Uses the field mapping to derive the full nested SearchQuery """ query = None - for field_mapping in cls.get_mapping(): - query_elements = cls._get_search_query_from_mapping( + for field_mapping in index_manager.get_mapping(): + query_elements = index_manager._get_search_query_from_mapping( query_str, model_class, field_mapping ) if query_elements is not None: - query = cls._add_to_query( + query = index_manager._add_to_query( query, query_elements, ) diff --git a/src/extended_search/settings.py b/src/extended_search/settings.py index 6b37ccf64..49d648ae2 100644 --- a/src/extended_search/settings.py +++ b/src/extended_search/settings.py @@ -213,7 +213,7 @@ def __init__(self, *args, **kwargs): def _get_all_indexed_fields(self): fields = {} for model_cls in get_indexed_models(): - for search_field in model_cls.search_fields: + for search_field in model_cls.get_searchable_search_fields(): definition_cls = search_field.get_definition_model(model_cls) if definition_cls not in fields: fields[definition_cls] = set() diff --git a/src/extended_search/tests/test_settings.py b/src/extended_search/tests/test_settings.py index 1f71efdb9..e6ad526f1 100644 --- a/src/extended_search/tests/test_settings.py +++ b/src/extended_search/tests/test_settings.py @@ -258,12 +258,12 @@ def test_get_all_indexed_fields(self, mocker): mock_searchfield_2.get_definition_model.return_value = "--model--" mock_searchfield_3 = mocker.MagicMock() mock_searchfield_3.get_definition_model.return_value = "--second-model--" - mock_model_1.search_fields = [ + mock_model_1.get_searchable_search_fields.return_value = [ mock_searchfield_1, mock_searchfield_2, mock_searchfield_2, ] - mock_model_2.search_fields = [ + mock_model_2.get_searchable_search_fields.return_value = [ mock_searchfield_3, ] mock_get_models.return_value = [ diff --git a/src/search/templates/search/explore.html b/src/search/templates/search/explore.html index 85c7f929c..80b1ef438 100644 --- a/src/search/templates/search/explore.html +++ b/src/search/templates/search/explore.html @@ -11,8 +11,47 @@ {% endblock location %} {% block content %} - {% if perms.search.view_explore %} + {% if messages %} + {% for message in messages %} + {% if message.tags == 'info' %} +
+
+

Success

+
+
+

{{ message }}

+
+
+ {% elif message.tags == 'error' %} +
+
+

There is a problem

+
+ +
+
+
+ {% endif %} + {% endfor %} + {% endif %} + {% if perms.extended_search.view_explore %}
+
+ + + Warning + This is a complex area. Read up on the theory behind, and the basis of, our approach. + +
+

Configure

Boost variables
-
    - {% for boost in boost_variables %}
  • {{ boost.name }}: {{ boost.value }}
    • {% endfor %} -
+
+
+
+ {% for boost in boost_variables %} +
+
{{ boost.name }}
+
+ {% if perms.extended_search.change_setting %} +
+ {% csrf_token %} + + + +
+ {% else %} + {{ boost.value }} + {% endif %} +
+
+ {% endfor %} +
+
+
@@ -46,12 +109,73 @@

Subqueries

- {% for type in sub_queries %} -

{{ type.name|capfirst }}

-
    - {% for query in type.queries %}
  • {{ query.id }}: {{ query.value }}
    • {% endfor %} -
- {% endfor %} +

All pages

+ + + + + + + + + + + + {% for query in sub_queries.pages %} + + + + + + + {% endfor %} + +
FieldAnalyzerQuery typeCombined boost
{{ query.field }}{{ query.analyzer }}{{ query.query_type }}{{ query.boost|floatformat:1 }}
+ +

People

+ + + + + + + + + + + + {% for query in sub_queries.people %} + + + + + + + {% endfor %} + +
FieldAnalyzerQuery typeCombined boost
{{ query.field }}{{ query.analyzer }}{{ query.query_type }}{{ query.boost|floatformat:1 }}
+

Teams

+ + + + + + + + + + + + {% for query in sub_queries.teams %} + + + + + + + {% endfor %} + +
FieldAnalyzerQuery typeCombined boost
{{ query.field }}{{ query.analyzer }}{{ query.query_type }}{{ query.boost|floatformat:1 }}
@@ -63,8 +187,8 @@

Evaluate

data-module="govuk-accordion" id="accordion-default"> {% search_category category='all_pages' show_heading=True %} - {% comment %} {% search_category category='people' show_heading=True %} - {% search_category category='teams' show_heading=True %} {% endcomment %} + {% search_category category='people' show_heading=True %} + {% search_category category='teams' show_heading=True %} {% else %} diff --git a/src/search/templates/search/partials/search_results_single_category.html b/src/search/templates/search/partials/search_results_single_category.html index b838d8533..1d1670816 100644 --- a/src/search/templates/search/partials/search_results_single_category.html +++ b/src/search/templates/search/partials/search_results_single_category.html @@ -2,12 +2,8 @@ {% if search_category in "guidance,tools,news" %}
-
- {% endif %} - - {% search_category category=search_category %} - - {% if search_category in "guidance,tools,news" %} -
+
{% search_category category=search_category %}
+{% else %} + {% search_category category=search_category %} {% endif %} diff --git a/src/search/views.py b/src/search/views.py index 05a259d49..e3910d891 100644 --- a/src/search/views.py +++ b/src/search/views.py @@ -1,17 +1,30 @@ import logging + +from django.contrib.auth.decorators import user_passes_test +from django.contrib import messages from django.http import HttpRequest, HttpResponse from django.shortcuts import redirect from django.template.response import TemplateResponse from django.urls import reverse from django.views.decorators.http import require_http_methods -from search.templatetags.search import SEARCH_CATEGORIES +from wagtail.search.query import Or, Phrase, PlainText, Fuzzy + +from content.models import ContentPage, ContentPageIndexManager +from peoplefinder.models import Person, PersonIndexManager, Team, TeamIndexManager +from extended_search.backends.query import OnlyFields +from extended_search.models import Setting as SearchSetting from extended_search.settings import extended_search_settings +from search.templatetags.search import SEARCH_CATEGORIES logger = logging.getLogger(__name__) +def can_view_explore(): + return user_passes_test(lambda u: u.has_perm("extended_search.view_explore")) + + @require_http_methods(["GET"]) def search(request: HttpRequest, category: str = None) -> HttpResponse: query = request.GET.get("query", "") @@ -33,10 +46,21 @@ def search(request: HttpRequest, category: str = None) -> HttpResponse: return TemplateResponse(request, "search/search.html", context=context) +@can_view_explore() def explore(request: HttpRequest) -> HttpResponse: """ Administrative view for exploring search options, boosts, etc """ + if request.method == "POST": + if not request.user.has_perm("extended_search.change_setting"): + messages.error(request, "You are not authorised to edit settings") + + key = request.POST.get("key") + value = request.POST.get("value") + + SearchSetting.objects.update_or_create(key=key, defaults={"value": value}) + messages.info(request, f"Setting '{key}' saved") + query = request.GET.get("query", "") page = request.GET.get("page", "1") @@ -46,17 +70,68 @@ def explore(request: HttpRequest) -> HttpResponse: if "boost_parts" in k ] + subqueries = {"pages": [], "people": [], "teams": []} + analyzer_field_suffices = [ + (k, v["index_fieldname_suffix"]) + for k, v in extended_search_settings["analyzers"].items() + ] + for mapping in ContentPageIndexManager.get_mapping(): + field = ContentPageIndexManager._get_search_query_from_mapping( + query, ContentPage, mapping + ) + get_query_info(subqueries["pages"], field, mapping, analyzer_field_suffices) + for mapping in PersonIndexManager.get_mapping(): + field = PersonIndexManager._get_search_query_from_mapping( + query, Person, mapping + ) + get_query_info(subqueries["people"], field, mapping, analyzer_field_suffices) + for mapping in TeamIndexManager.get_mapping(): + field = TeamIndexManager._get_search_query_from_mapping(query, Team, mapping) + get_query_info(subqueries["teams"], field, mapping, analyzer_field_suffices) + context = { "search_url": reverse("search:explore"), "search_query": query, "search_category": "all", "page": page, "boost_variables": boost_vars, - "sub_queries": [ - {"name": "pages", "queries": [{"id": 1, "value": "{match_all: {}}"}]}, - {"name": "people", "queries": [{"id": 1, "value": "{match_all: {}}"}]}, - {"name": "teams", "queries": [{"id": 1, "value": "{match_all: {}}"}]}, - ], + "sub_queries": subqueries, } return TemplateResponse(request, "search/explore.html", context=context) + + +def get_query_info(fields, field, mapping, suffix_map): + if field is None: + return fields + + if isinstance(field, Or): + for f in field.subqueries: + fields = get_query_info(fields, f, mapping, suffix_map) + + elif isinstance(field, OnlyFields): + core_field = field.subquery.subquery + + analyzer_name = "tokenizer" + for analyzer, suffix in suffix_map: + if suffix and suffix in field.fields[0]: + analyzer_name = analyzer + + if isinstance(core_field, Phrase): + query_type = "phrase" + elif isinstance(core_field, Fuzzy): + query_type = "fuzzy" + elif isinstance(core_field, PlainText): + if core_field.operator == "and": + query_type = "query_and" + else: + query_type = "query_or" + fields.append( + { + "query_type": query_type, + "field": mapping["model_field_name"], + "analyzer": analyzer_name, + "boost": field.subquery.boost, + } + ) + return fields