Search: a domain that FAILED no longer looks like one that found nothing
Every domain catches broadly and returns no rows so one broken table cannot take the whole search box down. That part is right. On its own, though, it makes a DOWN domain indistinguishable from an EMPTY one: the technician sees no results, is offered a spelling correction, and concludes the record does not exist. It does - the search just could not look. Failed domains are now named in the response and stated above the results. The suggestion is deliberately still offered on a degraded run. Suppressing it was tried first and is worse: a chronically unreachable domain, an external HR directory say, would silently disable spelling help for every search for ever with nothing saying why. Naming the failure is the honest fix; withholding a second useful signal is not. Guides search also moves to the published revision. It correctly limited itself to published GUIDES, but matched and displayed title/summary/tags/blocksjson from the Guide row - which is the WORKING copy. A published guide with unpublished edits matched on wording that only exists in the draft, and the reader opened it to find the word absent.
This commit is contained in:
cproudlock
@@ -61,6 +61,32 @@ def _word_match(query, *columns, extra=None):
|
||||
return db.and_(*[per_word(w) for w in words])
|
||||
|
||||
|
||||
def _notefailure(domain, exc):
|
||||
"""Record that one search domain failed this request, and log it.
|
||||
|
||||
Every domain below catches broadly and returns [] so one broken table cannot
|
||||
take the whole search box down. That is right, but on its own it makes a
|
||||
DOWN domain indistinguishable from an EMPTY one: the technician sees "no
|
||||
results", is told they may have misspelled it, and concludes the record does
|
||||
not exist. It does; the search just could not look.
|
||||
|
||||
The names collected here travel back in the response so the UI can say which
|
||||
part of the search did not run.
|
||||
"""
|
||||
from flask import g
|
||||
logger.error(f"{domain} search failed: {exc}")
|
||||
failures = getattr(g, '_searchfailures', None)
|
||||
if failures is None:
|
||||
failures = g._searchfailures = []
|
||||
if domain not in failures:
|
||||
failures.append(domain)
|
||||
|
||||
|
||||
def _searchfailures():
|
||||
from flask import g
|
||||
return list(getattr(g, '_searchfailures', []) or [])
|
||||
|
||||
|
||||
def _require_enabled(name):
|
||||
"""Raise ImportError when the named plugin is disabled.
|
||||
|
||||
@@ -253,7 +279,7 @@ def _search_applications(query, search_term):
|
||||
'relevance': relevance
|
||||
})
|
||||
except Exception as e:
|
||||
logger.error(f"Application search failed: {e}")
|
||||
_notefailure("Application", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -266,43 +292,53 @@ def _search_guides(query, search_term):
|
||||
is a LIKE against it: crude, but it finds the guide, and a guide corpus is
|
||||
tens of documents rather than millions of rows.
|
||||
|
||||
PUBLISHED ONLY. A draft is deliberately invisible to readers, and search is
|
||||
the one place that rule would otherwise leak - somebody would find a
|
||||
half-written procedure through the search box and follow it.
|
||||
PUBLISHED ONLY, and published TEXT only. A draft is deliberately invisible
|
||||
to readers, and search is the one place that rule would otherwise leak -
|
||||
somebody would find a half-written procedure through the search box and
|
||||
follow it.
|
||||
|
||||
Restricting to published GUIDES is not enough on its own, which is how this
|
||||
first shipped: title/summary/tags/blocksjson on the Guide row are the
|
||||
WORKING copy. A published guide with unpublished edits would match on a word
|
||||
that exists only in the draft, and hand the reader a hit that is not in the
|
||||
page when they open it. So the match, the title and the summary all come
|
||||
from the published revision.
|
||||
"""
|
||||
results = []
|
||||
try:
|
||||
_require_enabled('guides')
|
||||
from plugins.guides.models import Guide
|
||||
from plugins.guides.models import Guide, GuideRevision
|
||||
|
||||
rows = Guide.query.filter(
|
||||
published = db.aliased(GuideRevision)
|
||||
rows = db.session.query(Guide, published).join(
|
||||
published, Guide.publishedrevisionid == published.guiderevisionid
|
||||
).filter(
|
||||
Guide.isactive == True,
|
||||
Guide.publishedrevisionid.isnot(None),
|
||||
_word_match(query, Guide.title, Guide.summary, Guide.tags,
|
||||
Guide.blocksjson)
|
||||
_word_match(query, published.title, published.summary,
|
||||
published.tags, published.blocksjson)
|
||||
).order_by(Guide.modifieddate.desc()).limit(20).all()
|
||||
|
||||
for guide in rows:
|
||||
for guide, revision in rows:
|
||||
# A title match outranks a body match: someone searching "marker"
|
||||
# wants the guide called that, not every guide mentioning one.
|
||||
relevance = 10
|
||||
if query.lower() in (guide.title or '').lower():
|
||||
if query.lower() in (revision.title or '').lower():
|
||||
relevance += 20
|
||||
if guide.tags and query.lower() in guide.tags.lower():
|
||||
if revision.tags and query.lower() in revision.tags.lower():
|
||||
relevance += 15
|
||||
|
||||
results.append({
|
||||
'type': 'guide',
|
||||
'id': guide.guideid,
|
||||
'title': guide.title,
|
||||
'subtitle': guide.summary,
|
||||
'title': revision.title,
|
||||
'subtitle': revision.summary,
|
||||
'url': f"/guides/{guide.slug}",
|
||||
'relevance': relevance
|
||||
})
|
||||
except ImportError:
|
||||
pass # guides plugin absent or disabled
|
||||
except Exception as e:
|
||||
logger.error(f"Guides search failed: {e}")
|
||||
_notefailure("Guides", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -365,7 +401,7 @@ def _search_knowledgebase(query, search_term):
|
||||
except ImportError:
|
||||
pass # knowledgebase plugin absent or disabled
|
||||
except Exception as e:
|
||||
logger.error(f"KnowledgeBase search failed: {e}")
|
||||
_notefailure("KnowledgeBase", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -435,7 +471,7 @@ def _search_employees(query, search_term):
|
||||
'relevance': relevance
|
||||
})
|
||||
except Exception as e:
|
||||
logger.error(f"Employee search failed: {e}")
|
||||
_notefailure("Employee", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -485,7 +521,7 @@ def _search_assets(query, search_term):
|
||||
|
||||
results.append(_get_asset_result(asset, query, relevance))
|
||||
except Exception as e:
|
||||
logger.error(f"Asset search failed: {e}")
|
||||
_notefailure("Asset", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -525,7 +561,7 @@ def _search_measuringtools(query, search_term):
|
||||
except ImportError:
|
||||
pass # measuringtools plugin absent or disabled
|
||||
except Exception as e:
|
||||
logger.error(f"Measuring tool search failed: {e}")
|
||||
_notefailure("Measuring tool", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -576,7 +612,7 @@ def _search_usbdevices(query, search_term):
|
||||
except ImportError:
|
||||
pass # usb plugin absent or disabled
|
||||
except Exception as e:
|
||||
logger.error(f"USB device search failed: {e}")
|
||||
_notefailure("USB device", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -625,7 +661,7 @@ def _search_printeditems(query, search_term):
|
||||
except ImportError:
|
||||
pass # printedparts plugin absent or disabled
|
||||
except Exception as e:
|
||||
logger.error(f"Printed item search failed: {e}")
|
||||
_notefailure("Printed item", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -660,7 +696,7 @@ def _search_customfields(query, search_term):
|
||||
result['subtitle'] = field.label
|
||||
results.append(result)
|
||||
except Exception as e:
|
||||
logger.error(f"Custom field search failed: {e}")
|
||||
_notefailure("Custom field", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -688,7 +724,7 @@ def _search_by_ip(query, search_term):
|
||||
result['subtitle'] = comm.ipaddress
|
||||
results.append(result)
|
||||
except Exception as e:
|
||||
logger.error(f"IP search failed: {e}")
|
||||
_notefailure("IP", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -720,7 +756,7 @@ def _search_subnets(query):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Subnet search failed: {e}")
|
||||
_notefailure("Subnet", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -748,7 +784,7 @@ def _search_hostnames(query, search_term):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Computer hostname search failed: {e}")
|
||||
_notefailure("Computer hostname", e)
|
||||
|
||||
# Search Printers
|
||||
try:
|
||||
@@ -772,7 +808,7 @@ def _search_hostnames(query, search_term):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Printer hostname search failed: {e}")
|
||||
_notefailure("Printer hostname", e)
|
||||
|
||||
# Search Network Devices
|
||||
try:
|
||||
@@ -795,7 +831,7 @@ def _search_hostnames(query, search_term):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Network device hostname search failed: {e}")
|
||||
_notefailure("Network device hostname", e)
|
||||
|
||||
return results
|
||||
|
||||
@@ -843,7 +879,7 @@ def _search_notifications(query, search_term):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Notification search failed: {e}")
|
||||
_notefailure("Notification", e)
|
||||
return results
|
||||
|
||||
|
||||
@@ -877,7 +913,7 @@ def _search_vendor_model_type(query, search_term):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Machine vendor/model/type search failed: {e}")
|
||||
_notefailure("Machine vendor/model/type", e)
|
||||
|
||||
# Printers: vendor, model, printertype
|
||||
try:
|
||||
@@ -905,7 +941,7 @@ def _search_vendor_model_type(query, search_term):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Printer vendor/model/type search failed: {e}")
|
||||
_notefailure("Printer vendor/model/type", e)
|
||||
|
||||
# Network Devices: vendor, networkdevicetype
|
||||
try:
|
||||
@@ -931,7 +967,7 @@ def _search_vendor_model_type(query, search_term):
|
||||
except ImportError:
|
||||
pass
|
||||
except Exception as e:
|
||||
logger.error(f"Network device vendor/type search failed: {e}")
|
||||
_notefailure("Network device vendor/type", e)
|
||||
|
||||
return results
|
||||
|
||||
@@ -1252,12 +1288,24 @@ def global_search():
|
||||
# multi-word and cross-field matching changes may already have fixed a good
|
||||
# share of what used to fail. Guessing at that would be building for a
|
||||
# problem nobody has measured.
|
||||
degraded = _searchfailures()
|
||||
|
||||
suggestion = None
|
||||
if not unique_results:
|
||||
logger.info('search-no-results query=%r', query)
|
||||
# Offered, never applied. The response carries the better spelling and
|
||||
# the caller decides: silently searching for something else is how
|
||||
# somebody orders the wrong cartridge.
|
||||
#
|
||||
# Still offered on a degraded run, because the response also carries
|
||||
# `degraded` and the UI states it ABOVE the suggestion - the reader is
|
||||
# told the search was incomplete before being offered a spelling.
|
||||
#
|
||||
# Suppressing it instead was tried and is worse: a domain that is
|
||||
# chronically unreachable (an external HR directory, say) would silently
|
||||
# disable spelling help for every search, for ever, and nothing would
|
||||
# say why. Naming the failure is the honest fix; withholding the
|
||||
# suggestion just removes a second useful signal.
|
||||
suggestion = _suggestion_for(query)
|
||||
|
||||
# Limit total results
|
||||
@@ -1273,6 +1321,10 @@ def global_search():
|
||||
}
|
||||
if suggestion:
|
||||
response_data['suggestion'] = suggestion
|
||||
if degraded:
|
||||
# The UI says which part of the search did not run. Without this a
|
||||
# broken domain reads as "that record does not exist".
|
||||
response_data['degraded'] = degraded
|
||||
|
||||
redirect = _check_smart_redirect(query, classification)
|
||||
if redirect:
|
||||
|
||||
Reference in New Issue
Block a user