use filters when searching for matching topics in agenda (#871)

* use filters when searching for matching topics in agenda

CPCN-745

Author: petrjasek

newsroom/agenda/agenda.py

@@ -30,6 +30,7 @@
     save_user_notifications,
     UserNotification,
 )
+from newsroom.search import BoolQuery, BoolQueryParams
 from newsroom.template_filters import is_admin_or_internal, is_admin
 from newsroom.utils import (
     get_user_dict,
@@ -501,7 +502,7 @@ def nested_query(path, query, inner_hits=True, name=None):
 planning_filters = coverage_filters + ["agendas"]
 
 
-def _filter_terms(filters, item_type):
+def _filter_terms(filters, item_type, highlights=False):
     must_term_filters = []
     must_not_term_filters = []
     for key, val in filters.items():
@@ -623,6 +624,7 @@ def _filter_terms(filters, item_type):
                                     path="planning_items",
                                     query={"bool": {"filter": [{"terms": {f"planning_items.{agg_field}": val}}]}},
                                     name=key,
+                                    inner_hits=highlights,
                                 ),
                             ],
                         },
@@ -864,12 +866,11 @@ def prefill_search_items(self, search):
 
         pass
 
-    def apply_filters(self, search, section_filters=None):
+    def apply_filters(self, search: SearchQuery, section_filters=None):
         """Generate and apply the different search filters
 
         :param newsroom.search.SearchQuery search: the search query instance
         """
-
         # First construct the product query
         self.apply_company_filter(search)
 
@@ -913,8 +914,8 @@ def apply_filters(self, search, section_filters=None):
                             {
                                 # Match Events before ``item_type`` field was added
                                 "bool": {
-                                    "must_not": {"exists": {"field": "item_type"}},
-                                    "filter": {"exists": {"field": "event_id"}},
+                                    "must_not": [{"exists": {"field": "item_type"}}],
+                                    "filter": [{"exists": {"field": "event_id"}}],
                                 },
                             },
                         ],
@@ -949,12 +950,12 @@ def apply_filters(self, search, section_filters=None):
                 {
                     "bool": {
                         "should": [
-                            {"bool": {"must_not": {"exists": {"field": "item_type"}}}},
+                            {"bool": {"must_not": [{"exists": {"field": "item_type"}}]}},
                             {"term": {"item_type": "event"}},
                             {
                                 "bool": {
-                                    "filter": {"term": {"item_type": "planning"}},
-                                    "must_not": {"exists": {"field": "event_id"}},
+                                    "filter": [{"term": {"item_type": "planning"}}],
+                                    "must_not": [{"exists": {"field": "event_id"}}],
                                 },
                             },
                         ],
@@ -979,14 +980,14 @@ def apply_product_filter(self, search, product):
             if product.get("planning_item_query") and search.item_type != "events":
                 search.planning_items_should.append(planning_items_query_string(product.get("planning_item_query")))
 
-    def apply_request_filter(self, search):
+    def apply_request_filter(self, search: SearchQuery, highlights=False):
         """Generate the request filters
 
         :param newsroom.search.SearchQuery search: The search query instance
         """
 
         if search.args.get("q"):
-            test_query = {"bool": {"should": []}}
+            test_query: BoolQuery = {"bool": {"should": []}}
             try:
                 q = json.loads(search.args.get("q"))
                 if isinstance(q, dict):
@@ -1014,17 +1015,21 @@ def apply_request_filter(self, search):
                 )
 
         if search.args.get("id"):
-            search.query["bool"]["filter"].append({"term": {"_id": search.args["id"]}})
+            search.query["bool"]["filter"].append({"ids": {"values": [search.args["id"]]}})
 
         if search.args.get("ids"):
-            search.query["bool"]["filter"].append({"terms": {"_id": search.args["ids"]}})
+            search.query["bool"]["filter"].append({"ids": {"values": search.args["ids"]}})
 
         if search.args.get("bookmarks"):
             set_saved_items_query(search.query, search.args["bookmarks"])
 
         if search.args.get("date_from") or search.args.get("date_to"):
             _set_event_date_range(search)
 
+        filters = self.parse_filters(search)
+        if filters:
+            self.set_bool_query_from_filters(search.query["bool"], filters, search.item_type, highlights=highlights)
+
         self.apply_request_advanced_search(search)
 
     def set_post_filter(self, source: Dict[str, Any], req: ParsedRequest, item_type: Optional[str] = None):
@@ -1038,8 +1043,6 @@ def set_post_filter(self, source: Dict[str, Any], req: ParsedRequest, item_type:
         if app.config.get("FILTER_BY_POST_FILTER", False):
             source["post_filter"] = {"bool": {}}
             self.set_bool_query_from_filters(source["post_filter"]["bool"], filters, item_type)
-        else:
-            self.set_bool_query_from_filters(source["query"]["bool"], filters, item_type)
 
     def gen_source_from_search(self, search):
         """Generate the eve source object from the search query instance
@@ -1277,9 +1280,13 @@ def update_coverage_details(coverage):
         return agenda_items
 
     def set_bool_query_from_filters(
-        self, bool_query: Dict[str, Any], filters: Dict[str, Any], item_type: Optional[str] = None
+        self,
+        bool_query: BoolQueryParams,
+        filters: Dict[str, Any],
+        item_type: Optional[str] = None,
+        highlights=False,
     ):
-        filter_terms = _filter_terms(filters, item_type)
+        filter_terms = _filter_terms(filters, item_type, highlights=highlights)
         bool_query.setdefault("filter", [])
         bool_query["filter"] += filter_terms["must_term_filters"]
 

newsroom/news_api/news/search_service.py

@@ -318,7 +318,7 @@ def validate_unknown_params(self, search, whitelist, allow_filtering=True):
                 desc = "Multiple values received for parameter ({})"
                 raise UnexpectedParameterError(desc=desc.format(param_name))
 
-    def apply_request_filter(self, search):
+    def apply_request_filter(self, search, highlights=True):
         """Generate the filters from request args
 
         :param newsroom.search.SearchQuery search:  The search query instance

newsroom/search/__init__.py

@@ -1,4 +1,5 @@
 from flask import render_template
+from typing import Dict, List, Literal, TypedDict, Union
 
 from newsroom.gettext import get_session_locale
 from newsroom.email import get_language_template_name
@@ -13,3 +14,73 @@ def render_search_tips_html(search_type) -> str:
     template_name = get_language_template_name(f"search_tips_{search_type}", locale, "html")
 
     return render_template(template_name)
+
+
+class QueryStringParams(TypedDict, total=False):
+    query: str
+    default_operator: Literal["AND", "OR"]
+    analyze_wildcard: bool
+    lenient: bool
+    fields: List[str]
+    type: Literal["cross_fields", "best_fields"]
+
+
+Value = Union[str, bool, int]
+
+
+class QueryStringQuery(TypedDict):
+    query_string: QueryStringParams
+
+
+class ExistsQueryParam(TypedDict):
+    field: str
+
+
+class ExistsQuery(TypedDict):
+    exists: ExistsQueryParam
+
+
+class MatchQuery(TypedDict):
+    match: Dict[str, Value]
+
+
+class TermQuery(TypedDict):
+    term: Dict[str, Value]
+
+
+class TermsQuery(TypedDict):
+    terms: Dict[str, List[Value]]
+
+
+class IDsQuery(TypedDict):
+    ids: Dict[Literal["values"], List[str]]
+
+
+class RangeQuery(TypedDict):
+    range: Dict[str, Dict[Literal["gte", "gt", "lte", "lt"], Value]]
+
+
+class BoolQueryParams(TypedDict, total=False):
+    must: List["Query"]
+    must_not: List["Query"]
+    should: List["Query"]
+    filter: List["Query"]
+    minimum_should_match: int
+
+
+class BoolQuery(TypedDict):
+    bool: BoolQueryParams
+
+
+class NestedQueryParams(TypedDict):
+    path: str
+    query: Union[TermQuery, BoolQuery]
+
+
+class NestedQuery(TypedDict):
+    nested: NestedQueryParams
+
+
+TermLevelQuery = Union[TermQuery, TermsQuery, IDsQuery, ExistsQuery, RangeQuery]
+FullTextQuery = Union[QueryStringQuery, MatchQuery]
+Query = Union[TermLevelQuery, FullTextQuery, BoolQuery, NestedQuery]

newsroom/search/service.py

@@ -13,6 +13,7 @@
 
 from newsroom import Service
 from newsroom.auth.utils import user_has_section_allowed
+from newsroom.search import BoolQuery, BoolQueryParams, QueryStringQuery
 from newsroom.search.config import (
     SearchGroupNestedConfig,
     get_nested_config,
@@ -48,7 +49,7 @@ def query_string(
     fields: List[str] = ["*"],
     multimatch_type: Literal["cross_fields", "best_fields"] = "cross_fields",
     analyze_wildcard=False,
-):
+) -> QueryStringQuery:
     query_string_settings = app.config["ELASTICSEARCH_SETTINGS"]["settings"]["query_string"]
     return {
         "query_string": {
@@ -106,6 +107,16 @@ class AdvancedSearchParams(TypedDict):
     fields: List[str]
 
 
+class SearchArgs(TypedDict, total=False):
+    q: str
+    id: str
+    ids: List[str]
+    size: int
+    bookmarks: str
+    ignore_latest: bool
+    filter: Union[Dict[str, str], str]
+
+
 class SearchQuery(object):
     """Class for storing the search parameters for validation and query generation"""
 
@@ -120,14 +131,14 @@ def __init__(self):
         self.requested_products = []
         self.advanced: Optional[AdvancedSearchParams] = None
 
-        self.args = {}
+        self.args: SearchArgs = {}
         self.lookup = {}
         self.projections = {}
         self.req = None
 
         self.aggs = None
         self.source = {}
-        self.query = {"bool": {"filter": [], "must": [], "must_not": [], "should": []}}
+        self.query: BoolQuery = {"bool": {"filter": [], "must": [], "must_not": [], "should": []}}
         self.highlight = None
         self.item_type = None
         self.planning_items_should = []
@@ -199,7 +210,7 @@ def _search_all_versions(self, search: SearchQuery, req, lookup):
 
         # Now run a query only using the IDs from the above search
         # This final search makes sure pagination still works
-        search.query["bool"] = {"filter": {"terms": {"_id": next_item_ids}}}
+        search.query["bool"] = {"filter": [{"ids": {"values": next_item_ids}}]}
         self.gen_source_from_search(search)
         internal_req = self.get_internal_request(search)
         res = self.internal_get(internal_req, search.lookup)
@@ -330,10 +341,11 @@ def get_internal_request(self, search):
 
         return internal_req
 
-    def set_bool_query_from_filters(self, bool_query: Dict[str, Any], filters: Dict[str, Any]):
+    def set_bool_query_from_filters(self, bool_query: BoolQueryParams, filters: Dict[str, Any]) -> None:
         for key, val in filters.items():
             if not val:
                 continue
+            bool_query.setdefault("must", [])
             bool_query["must"].append(
                 get_filter_query(key, val, self.get_aggregation_field(key), get_nested_config("items", key))
             )
@@ -576,7 +588,7 @@ def prefill_search_highlights(self, search, req):
             highlight_search.advanced = deepcopy(search.advanced)
 
             # Set up the search query for filtering
-            self.apply_request_filter(highlight_search)
+            self.apply_request_filter(highlight_search, highlights=True)
 
             # Set up highlighting settings
             highlight_search.source.setdefault("highlight", {})
@@ -742,31 +754,34 @@ def get_product_filter(self, search, product):
         if product.get("query"):
             return self.query_string(product["query"])
 
-    def apply_request_filter(self, search):
+    def parse_filters(self, search: SearchQuery) -> Optional[Dict[str, Any]]:
+        if search.args.get("filter"):
+            if isinstance(search.args["filter"], dict):
+                return search.args["filter"]
+            else:
+                try:
+                    return json.loads(search.args["filter"])
+                except TypeError:
+                    raise BadParameterValueError("Incorrect type supplied for filter parameter")
+        return None
+
+    def apply_request_filter(self, search: SearchQuery, highlights=False) -> None:
         if search.args.get("q"):
             search.query["bool"].setdefault("must", []).append(
                 self.query_string(search.args["q"], search.args.get("default_operator") or "AND")
             )
 
         if search.args.get("ids"):
-            search.query["bool"]["must"].append({"terms": {"_id": search.args["ids"]}})
+            search.query["bool"]["must"].append({"ids": {"values": search.args["ids"]}})
 
-        filters = None
-        if search.args.get("filter"):
-            if isinstance(search.args["filter"], dict):
-                filters = search.args["filter"]
-            else:
-                try:
-                    filters = json.loads(search.args["filter"])
-                except TypeError:
-                    raise BadParameterValueError("Incorrect type supplied for filter parameter")
+        filters = self.parse_filters(search)
 
         if not app.config.get("FILTER_BY_POST_FILTER", False):
             if filters:
                 if app.config.get("FILTER_AGGREGATIONS", True):
                     self.set_bool_query_from_filters(search.query["bool"], filters)
-                else:
-                    search.query["bool"]["must"].append(filters)
+                elif isinstance(filters, dict):
+                    search.query["bool"]["must"].append(filters)  # type: ignore
 
             if search.args.get("created_from") or search.args.get("created_to"):
                 search.query["bool"]["must"].append(self.versioncreated_range(search.args))
@@ -906,7 +921,7 @@ def get_matching_topics_for_item(self, topics, users, companies, query):
 
         return topic_matches
 
-    def apply_topic_args(self, topic, args=None):
+    def apply_topic_args(self, topic, args=None) -> SearchArgs:
         if args is None:
             args = {}
 
@@ -974,7 +989,7 @@ def get_items_by_query(self, search, size=10, aggs=None):
         internal_req = self.get_internal_request(search)
         return self.internal_get(internal_req, search.lookup)
 
-    def query_string(self, query, default_operator="AND"):
+    def query_string(self, query, default_operator="AND") -> QueryStringQuery:
         fields_config_key = "WIRE_SEARCH_FIELDS" if self.section == "wire" else "AGENDA_SEARCH_FIELDS"
         fields = app.config.get(fields_config_key, ["*"])
         return query_string(query, default_operator=default_operator, fields=fields)

newsroom/wire/search.py

@@ -162,7 +162,7 @@ def get_product_items(self, product_id: str, size: int, exclude_embargoed: bool
                     "bool": {
                         "should": [
                             {"range": {"embargoed": {"lt": "now"}}},
-                            {"bool": {"must_not": {"exists": {"field": "embargoed"}}}},
+                            {"bool": {"must_not": [{"exists": {"field": "embargoed"}}]}},
                         ]
                     }
                 }
@@ -269,13 +269,13 @@ def has_permissions(self, item, ignore_latest=False):
         except Forbidden:
             return False
 
-    def apply_request_filter(self, search):
+    def apply_request_filter(self, search, highlights=True):
         """Generate the filters from request args
 
         :param newsroom.search.SearchQuery search: The search query instance
         """
 
-        super().apply_request_filter(search)
+        super().apply_request_filter(search, highlights=highlights)
 
         if search.args.get("bookmarks"):
             set_bookmarks_query(search.query, search.args["bookmarks"])