From 0c1d7880b88e7f9a671a04c43a5ea22c407f0cb3 Mon Sep 17 00:00:00 2001 From: level09 Date: Sat, 11 Jul 2026 14:00:17 +0200 Subject: [PATCH 1/2] fix(search): group multi-query blocks correctly in refine/extend combination --- enferno/utils/search_utils.py | 50 +++++++++++++++----------- tests/test_search_query_combination.py | 47 ++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 tests/test_search_query_combination.py diff --git a/enferno/utils/search_utils.py b/enferno/utils/search_utils.py index 7d86675b7..f6ec51949 100644 --- a/enferno/utils/search_utils.py +++ b/enferno/utils/search_utils.py @@ -1,6 +1,6 @@ import re from dateutil.parser import parse -from sqlalchemy import or_, not_, and_, func, text, select, literal_column, bindparam +from sqlalchemy import or_, and_, func, text, select, literal_column, bindparam from sqlalchemy.sql.elements import BinaryExpression, ColumnElement from sqlalchemy import String, Integer, DateTime from sqlalchemy.dialects.postgresql import ARRAY @@ -95,24 +95,28 @@ def get_query(self): return select(Bulletin) # Get conditions from first query main_stmt, conditions = self.bulletin_query(self.search[0]) - final_conditions = conditions + combined = and_(*conditions) if conditions else None - # Handle nested queries by combining conditions + # Fold remaining query blocks left-to-right; each block's + # conditions are grouped with AND before its op is applied. if len(self.search) > 1: for i in range(1, len(self.search)): _, next_conditions = self.bulletin_query(self.search[i]) + if not next_conditions: + continue + block = and_(*next_conditions) op = self.search[i].get("op", "or") - - if op == "and": - final_conditions.extend(next_conditions) - elif op == "or": - # Combine conditions with OR - final_conditions = [or_(*conditions, *next_conditions)] + if combined is None: + combined = block + elif op == "and": + combined = and_(combined, block) + else: + combined = or_(combined, block) # Build final query with all conditions and default sorting result = select(Bulletin) - if final_conditions: - result = result.where(and_(*final_conditions)) + if combined is not None: + result = result.where(combined) return result elif self.cls == "actor": @@ -121,24 +125,28 @@ def get_query(self): return select(Actor) # Get conditions from first query main_stmt, conditions = self.actor_query(self.search[0]) - final_conditions = conditions + combined = and_(*conditions) if conditions else None - # Handle nested queries by combining conditions + # Fold remaining query blocks left-to-right; each block's + # conditions are grouped with AND before its op is applied. if len(self.search) > 1: for i in range(1, len(self.search)): _, next_conditions = self.actor_query(self.search[i]) + if not next_conditions: + continue + block = and_(*next_conditions) op = self.search[i].get("op", "or") - - if op == "and": - final_conditions.extend(next_conditions) - elif op == "or": - # Combine conditions with OR - final_conditions = [or_(*conditions, *next_conditions)] + if combined is None: + combined = block + elif op == "and": + combined = and_(combined, block) + else: + combined = or_(combined, block) # Build final query with all conditions and default sorting result = select(Actor) - if final_conditions: - result = result.where(and_(*final_conditions)) + if combined is not None: + result = result.where(combined) return result elif self.cls == "incident": diff --git a/tests/test_search_query_combination.py b/tests/test_search_query_combination.py new file mode 100644 index 000000000..36d7c8925 --- /dev/null +++ b/tests/test_search_query_combination.py @@ -0,0 +1,47 @@ +"""Multi-block advanced search combination (Refine/Extend) - BYNT-1760. + +Blocks must be grouped: each block's conditions AND-ed together before the +block's op (and/or) combines it with the accumulated query. +""" + +import re + +from enferno.utils.search_utils import SearchUtils + + +def compiled(q, cls): + stmt = SearchUtils(q, cls).get_query() + return str(stmt.compile(compile_kwargs={"literal_binds": True})) + + +def test_or_groups_block_conditions(): + # Block 2 has two conditions; they must stay AND-ed inside the OR, + # not be flattened into independent OR operands. + # AND binds tighter than OR, so correct grouping renders as + # "id IN (1) OR id IN (2) AND originid ...". The old flattening + # bug joined originid with OR instead. + sql = compiled( + [{"ids": [1]}, {"op": "or", "ids": [2], "originid": "x"}], + "bulletin", + ) + assert "AND lower(bulletin.originid)" in sql + assert "OR lower(bulletin.originid)" not in sql + + +def test_chained_or_keeps_all_blocks(): + # Regression: the middle OR block used to be silently dropped. + sql = compiled( + [{"ids": [1]}, {"op": "or", "ids": [2]}, {"op": "or", "ids": [3]}], + "actor", + ) + for n in (1, 2, 3): + assert f"IN ({n})" in sql + + +def test_and_after_or_applies_to_combined_result(): + # (block1 OR block2) AND block3 + sql = compiled( + [{"ids": [1]}, {"op": "or", "ids": [2]}, {"op": "and", "ids": [3]}], + "bulletin", + ) + assert re.search(r"\(.*IN \(1\).*OR.*IN \(2\).*\).*AND.*IN \(3\)", sql, re.DOTALL) From 3ec2c1b2da1da5be207f05839de675bb8ffc2908 Mon Sep 17 00:00:00 2001 From: level09 Date: Sat, 11 Jul 2026 14:45:40 +0200 Subject: [PATCH 2/2] refactor(search): extract shared block combination helper --- enferno/utils/search_utils.py | 61 +++++++++++------------------------ 1 file changed, 19 insertions(+), 42 deletions(-) diff --git a/enferno/utils/search_utils.py b/enferno/utils/search_utils.py index f6ec51949..0be2ce616 100644 --- a/enferno/utils/search_utils.py +++ b/enferno/utils/search_utils.py @@ -87,33 +87,30 @@ def get_ocr_matched_ids(self, bulletin_ids: list) -> set: return set() return self._ocr_matched_ids & set(bulletin_ids) + def _combine_query_blocks(self, build_query): + """Fold query blocks left-to-right; each block's conditions are + grouped with AND before its op (and/or) combines it with the rest.""" + combined = None + for block_q in self.search: + _, conditions = build_query(block_q) + if not conditions: + continue + block = and_(*conditions) + if combined is None: + combined = block + elif block_q.get("op", "or") == "and": + combined = and_(combined, block) + else: + combined = or_(combined, block) + return combined + def get_query(self): """Get the query for the given class.""" if self.cls == "bulletin": # Handle empty search - return all bulletins if not self.search: return select(Bulletin) - # Get conditions from first query - main_stmt, conditions = self.bulletin_query(self.search[0]) - combined = and_(*conditions) if conditions else None - - # Fold remaining query blocks left-to-right; each block's - # conditions are grouped with AND before its op is applied. - if len(self.search) > 1: - for i in range(1, len(self.search)): - _, next_conditions = self.bulletin_query(self.search[i]) - if not next_conditions: - continue - block = and_(*next_conditions) - op = self.search[i].get("op", "or") - if combined is None: - combined = block - elif op == "and": - combined = and_(combined, block) - else: - combined = or_(combined, block) - - # Build final query with all conditions and default sorting + combined = self._combine_query_blocks(self.bulletin_query) result = select(Bulletin) if combined is not None: result = result.where(combined) @@ -123,27 +120,7 @@ def get_query(self): # Handle empty search - return all actors if not self.search: return select(Actor) - # Get conditions from first query - main_stmt, conditions = self.actor_query(self.search[0]) - combined = and_(*conditions) if conditions else None - - # Fold remaining query blocks left-to-right; each block's - # conditions are grouped with AND before its op is applied. - if len(self.search) > 1: - for i in range(1, len(self.search)): - _, next_conditions = self.actor_query(self.search[i]) - if not next_conditions: - continue - block = and_(*next_conditions) - op = self.search[i].get("op", "or") - if combined is None: - combined = block - elif op == "and": - combined = and_(combined, block) - else: - combined = or_(combined, block) - - # Build final query with all conditions and default sorting + combined = self._combine_query_blocks(self.actor_query) result = select(Actor) if combined is not None: result = result.where(combined)