From d0eeaa08d5d77f5fdd1fccf92f936b4112d7f29a Mon Sep 17 00:00:00 2001 From: cproudlock Date: Fri, 21 Aug 2026 09:39:46 -0400 Subject: [PATCH] Find a knowledge base article by words from different fields An article's searchable text lives in three places: its title, its keywords, and the name of its topic. Nobody typing a search knows or cares which word came from which, so 'CMM Community' means "the article tagged community, under the CMM topic". Both search paths returned nothing for it. The plugin's own listing, which is what the KB page calls, built a single ilike('%CMM Community%'). That needs the whole phrase contiguous in ONE field, and no article has it: 'CMM' is only in the topic's name, 'Community' only in the keywords. Global search already split the query into words but only ever looked at title and keywords, so a word that only the topic could satisfy failed there too - the same bug wearing a different face. Every word must now be found somewhere across all three fields, in any order. _word_match gains an `extra` hook for a word that a RELATED row satisfies rather than a column of this table, which is how the topic joins in without colliding with the sort=='topic' join. The retired-topic rule is unchanged and pinned on both paths: matching more words is not a way past it. Verified against the 235-article dev library, where all three of these returned nothing before: 'Fieldglass Jefferson' (title word plus topic word), 'outage notification' (two keywords, not adjacent), 'compucom Jefferson'. The same phrase-only pattern is repeated in about ten other list endpoints (computers, machines, printers, network, printedparts, measuringtools, usb, notifications). Left alone here - fixing them properly means promoting the word-match helper onto the shopdb.api contract surface rather than copying it per plugin, which is a contract bump. --- plugins/knowledgebase/api/routes.py | 56 +++++--- shopdb/core/api/search.py | 39 +++++- .../test_knowledgebase_search_words.py | 128 ++++++++++++++++++ 3 files changed, 198 insertions(+), 25 deletions(-) create mode 100644 tests/test_plugins/test_knowledgebase_search_words.py diff --git a/plugins/knowledgebase/api/routes.py b/plugins/knowledgebase/api/routes.py index bae722b..70df875 100644 --- a/plugins/knowledgebase/api/routes.py +++ b/plugins/knowledgebase/api/routes.py @@ -43,6 +43,39 @@ def _visible_articles(): KnowledgeBase.appid.notin_(retired))) +def _search_clause(search): + """Articles containing EVERY word of the search, each word in the title, the + keywords, or the topic's name, in any order. + + A single `ilike('%CMM Community%')` needs the whole phrase contiguous in ONE + field. An article tagged 'community' under the CMM topic has 'CMM' only in + its topic's name and 'Community' only in its keywords, so it matched nothing + and the search came back empty. Splitting the query and requiring each word + somewhere is what people mean when they type two words. + + Kept in step with `_word_match` in shopdb/core/api/search.py, which does the + same job for global search. Two copies because the core helper is internal + and not on the plugin contract surface (ADR-001). + + Active applications only for the topic. A retired application is not a topic + anyone should be offered: matching its name surfaced its articles and printed + the retired app as their subject, which reads as though it were still in + service. + """ + words = [w for w in search.split() if w] or [''] + return db.and_(*[ + db.or_( + KnowledgeBase.shortdescription.ilike(f'%{w}%'), + KnowledgeBase.keywords.ilike(f'%{w}%'), + KnowledgeBase.appid.in_( + db.session.query(Application.appid).filter( + Application.appname.ilike(f'%{w}%'), + Application.isactive.is_(True))), + ) + for w in words + ]) + + @knowledgebase_bp.route('', methods=['GET']) @jwt_required(optional=True) def list_articles(): @@ -51,26 +84,11 @@ def list_articles(): query = _visible_articles() - # Search: title, keywords, and the topic (its Application's name). The topic - # is matched via an appid subquery instead of a join so it does not collide - # with the sort=='topic' join below; articles with no app just miss that - # clause and still match on title/keywords. + # The topic is matched via an appid subquery instead of a join so it does not + # collide with the sort=='topic' join below; articles with no app just miss + # that clause and still match on title/keywords. if search := request.args.get('search'): - like = f'%{search}%' - # Active applications only. A retired application is not a topic anyone - # should be offered: matching its name surfaced its articles and printed - # the retired app as their subject, which reads as though it were still - # in service. - topic_appids = db.session.query(Application.appid).filter( - Application.appname.ilike(like), - Application.isactive.is_(True)) - query = query.filter( - db.or_( - KnowledgeBase.shortdescription.ilike(like), - KnowledgeBase.keywords.ilike(like), - KnowledgeBase.appid.in_(topic_appids) - ) - ) + query = query.filter(_search_clause(search)) # Filter by topic/application if appid := request.args.get('appid'): diff --git a/shopdb/core/api/search.py b/shopdb/core/api/search.py index 02390b9..85c3753 100644 --- a/shopdb/core/api/search.py +++ b/shopdb/core/api/search.py @@ -31,7 +31,7 @@ search_bp = Blueprint('search', __name__) # cannot reorder. Relevance is applied afterwards in Python, over a stable set. -def _word_match(query, *columns): +def _word_match(query, *columns, extra=None): """SQL clause matching rows that contain EVERY word of the query, each word in any of the columns, in any order. @@ -39,14 +39,25 @@ def _word_match(query, *columns): multi-word search returned nothing. This splits the query into words and ANDs them (OR across the columns per word): 'CSF Roles' matches a row with 'CSF' and 'Roles' anywhere in the searched fields. + + `extra` is an optional callable taking one word and returning a further + clause to OR in for that word. Use it where a word may be satisfied by a + RELATED row rather than a column of this table - a knowledge base article + matching on its topic's name, say. Splitting per word matters there: without + it a word can only ever be matched by the columns, so a query naming the + topic and a keyword ('CMM Community') matches neither side alone. """ words = [w for w in query.split() if w] if not words: words = [''] - return db.and_(*[ - db.or_(*[c.ilike(f'%{w}%') for c in columns]) - for w in words - ]) + + def per_word(word): + clauses = [c.ilike(f'%{word}%') for c in columns] + if extra is not None: + clauses.append(extra(word)) + return db.or_(*clauses) + + return db.and_(*[per_word(w) for w in words]) def _require_enabled(name): @@ -253,6 +264,10 @@ def _search_knowledgebase(query, search_term): subject, which reads as though it were still in service. A null topic still matches. Not every article is about an application. + + The topic's NAME is searched too, per word. An article tagged 'community' + under the CMM topic has neither word in both of its own columns, so a search + for 'CMM Community' found it in neither the plugin's listing nor here. """ results = [] try: @@ -260,12 +275,24 @@ def _search_knowledgebase(query, search_term): from plugins.knowledgebase.models import KnowledgeBase retired = db.session.query(Application.appid).filter( Application.isactive.is_(False)) + + def topic_named(word): + """Articles whose topic is an ACTIVE application named like word. + + Active only, for the same reason the retired filter above exists: a + decommissioned application is not a topic anyone should be offered. + """ + return KnowledgeBase.appid.in_( + db.session.query(Application.appid).filter( + Application.appname.ilike(f'%{word}%'), + Application.isactive.is_(True))) + kb_articles = KnowledgeBase.query.filter( KnowledgeBase.isactive == True, db.or_(KnowledgeBase.appid.is_(None), KnowledgeBase.appid.notin_(retired)), _word_match(query, KnowledgeBase.shortdescription, - KnowledgeBase.keywords) + KnowledgeBase.keywords, extra=topic_named) ).order_by(KnowledgeBase.clicks.desc(), KnowledgeBase.linkid).limit(20).all() diff --git a/tests/test_plugins/test_knowledgebase_search_words.py b/tests/test_plugins/test_knowledgebase_search_words.py new file mode 100644 index 0000000..e26e15f --- /dev/null +++ b/tests/test_plugins/test_knowledgebase_search_words.py @@ -0,0 +1,128 @@ +"""Searching a knowledge base article by words that live in different fields. + +An article's searchable text is spread across three places: its title, its +keywords, and the name of its topic. People do not know or care which word came +from which, so they type the two words that identify the article and expect it +back - 'CMM Community' for an article tagged `community` under the CMM topic. + +Both search paths used one `ilike('%CMM Community%')`, which needs that exact +phrase contiguous in a SINGLE field. No article has it, so the search returned +nothing at all and the article looked absent rather than unfindable. Every word +must be found somewhere, in any order, across all three fields. + +Covers the plugin's own listing (`GET /api/knowledgebase?search=`, what the KB +page uses) and global search (`GET /api/search?q=`), which had the fault in two +different shapes: the listing did not split words, and global search split them +but never looked at the topic. +""" + +import pytest + +from shopdb.core.models import Application +from plugins.knowledgebase.models import KnowledgeBase + + +@pytest.fixture +def library(db): + """Articles whose identifying words are deliberately spread across fields.""" + topics = { + 'cmm': Application(appname='Zzcmm', isactive=True), + 'other': Application(appname='Zzunrelated', isactive=True), + 'retired': Application(appname='Zzretired Zzcmm', isactive=False), + } + db.session.add_all(topics.values()) + db.session.flush() + + articles = { + # The reported case: neither word is in the same field as the other. + 'split': KnowledgeBase( + shortdescription='Zzprotect viewer notes', linkurl='u1', + keywords='zzcommunity license', appid=topics['cmm'].appid, + isactive=True), + # Both words present but not adjacent, inside one field. + 'gapped': KnowledgeBase( + shortdescription='Zzpower outage plan alert zznotification', + linkurl='u2', keywords='', appid=None, isactive=True), + # Holds only one of the two words; must NOT come back. + 'halfmatch': KnowledgeBase( + shortdescription='Zzprotect viewer only', linkurl='u3', + keywords='license', appid=topics['other'].appid, isactive=True), + # Retired topic, matches both words. The existing rule still wins. + 'retired': KnowledgeBase( + shortdescription='Zzprotect viewer retired', linkurl='u4', + keywords='zzcommunity', appid=topics['retired'].appid, + isactive=True), + } + db.session.add_all(articles.values()) + db.session.commit() + return {'topics': topics, 'articles': articles} + + +def listed(client, search): + """Titles from the plugin's own listing, the one the KB page calls.""" + resp = client.get('/api/knowledgebase', query_string={'search': search}) + assert resp.status_code == 200, resp.get_json() + return [row['shortdescription'] for row in resp.get_json()['data']] + + +def globally(client, search): + """Knowledge base titles from global search.""" + resp = client.get('/api/search', query_string={'q': search}) + assert resp.status_code == 200, resp.get_json() + return [r['title'] for r in resp.get_json()['data']['results'] + if r['type'] == 'knowledgebase'] + + +# --- the reported case ------------------------------------------------------ + +def test_listing_finds_an_article_by_its_topic_and_its_keyword(client, library): + """'Zzcmm Zzcommunity': one word from the topic's name, one from the keywords. + + This is the bug as reported. A phrase match could satisfy neither field. + """ + assert 'Zzprotect viewer notes' in listed(client, 'Zzcmm Zzcommunity') + + +def test_global_search_finds_an_article_by_its_topic_and_its_keyword(client, library): + """Global search split the words but only ever looked at title and keywords, + so a word that only the topic could satisfy failed there too.""" + assert 'Zzprotect viewer notes' in globally(client, 'Zzcmm Zzcommunity') + + +# --- word order and adjacency ---------------------------------------------- + +def test_word_order_does_not_matter(client, library): + assert 'Zzprotect viewer notes' in listed(client, 'Zzcommunity Zzcmm') + + +def test_words_need_not_be_adjacent_within_one_field(client, library): + """'plan' and 'alert' sit between them in the title.""" + assert 'Zzpower outage plan alert zznotification' in listed( + client, 'Zzpower zznotification') + + +def test_the_whole_phrase_still_matches(client, library): + """Splitting must not break the searches that already worked.""" + assert 'Zzprotect viewer notes' in listed(client, 'Zzprotect viewer') + + +# --- what must still be excluded ------------------------------------------- + +def test_an_article_holding_only_one_word_is_not_returned(client, library): + """EVERY word must be found. ORing them would return most of the library and + bury the article the words actually describe.""" + found = listed(client, 'Zzprotect Zzcommunity') + assert 'Zzprotect viewer only' not in found + assert 'Zzprotect viewer notes' in found + + +def test_a_retired_topic_still_hides_the_article(client, library): + """Matching more words must not become a way past the retired rule.""" + assert 'Zzprotect viewer retired' not in listed(client, 'Zzprotect Zzcommunity') + assert 'Zzprotect viewer retired' not in globally(client, 'Zzprotect Zzcommunity') + + +def test_a_retired_topic_name_is_not_a_way_in(client, library): + """The retired application is named 'Zzretired Zzcmm'; its name matching a word + must not surface its articles.""" + assert listed(client, 'Zzretired') == []