Find a knowledge base article by words from different fields

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
+    # The topic is matched via an appid subquery instead of a join so it does not
-    # is matched via an appid subquery instead of a join so it does not collide
+    # collide with the sort=='topic' join below; articles with no app just miss
-    # with the sort=='topic' join below; articles with no app just miss that
+    # that clause and still match on title/keywords.
-    # clause and still match on title/keywords.
     if search := request.args.get('search'):
-        like = f'%{search}%'
+        query = query.filter(_search_clause(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)
-            )
-        )
 
     # Filter by topic/application
     if appid := request.args.get('appid'):

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])
+    def per_word(word):
-        for w in words
+        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()
 

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') == []