diff --git a/frontend/src/views/SearchResults.vue b/frontend/src/views/SearchResults.vue index 80d262f8..1eb53316 100644 --- a/frontend/src/views/SearchResults.vue +++ b/frontend/src/views/SearchResults.vue @@ -117,6 +117,7 @@ const typeLabels = { computer: 'PC', application: 'App', knowledgebase: 'KB', + guide: 'Guide', printer: 'Printer', network_device: 'Network', measuring_tool: 'Measuring Tool', @@ -136,6 +137,7 @@ const filterTypeMap = { measuringtools: ['measuring_tool'], applications: ['application'], knowledgebase: ['knowledgebase'], + guides: ['guide'], notifications: ['notification'], employees: ['employee'], usb: ['usb_device'], @@ -151,6 +153,7 @@ const filterList = [ { key: 'measuringtools', label: 'Measuring Tools' }, { key: 'applications', label: 'Apps' }, { key: 'knowledgebase', label: 'KB' }, + { key: 'guides', label: 'Guides' }, { key: 'notifications', label: 'Notices' }, { key: 'employees', label: 'Employees' }, { key: 'usb', label: 'USB' }, @@ -424,6 +427,7 @@ watch(results, () => { --rt-computer-bg: #e8f5e9; --rt-computer-fg: #2e7d32; --rt-application-bg: #fff3e0; --rt-application-fg: #e65100; --rt-knowledgebase-bg: #f3e5f5; --rt-knowledgebase-fg: #7b1fa2; + --rt-guide-bg: #e8f0fe; --rt-guide-fg: #1a56b8; --rt-printer-bg: #fce4ec; --rt-printer-fg: #c2185b; --rt-network-bg: #fff8e1; --rt-network-fg: #f57f17; --rt-measuring-bg: #e0f7fa; --rt-measuring-fg: #00838f; @@ -437,6 +441,7 @@ watch(results, () => { .result-type.computer { background: var(--rt-computer-bg); color: var(--rt-computer-fg); } .result-type.application { background: var(--rt-application-bg); color: var(--rt-application-fg); } .result-type.knowledgebase { background: var(--rt-knowledgebase-bg); color: var(--rt-knowledgebase-fg); } +.result-type.guide { background: var(--rt-guide-bg); color: var(--rt-guide-fg); } .result-type.printer { background: var(--rt-printer-bg); color: var(--rt-printer-fg); } .result-type.network_device { background: var(--rt-network-bg); color: var(--rt-network-fg); } .result-type.measuring_tool { background: var(--rt-measuring-bg); color: var(--rt-measuring-fg); } diff --git a/plugins/guides/frontend/utils/proof.js b/plugins/guides/frontend/utils/proof.js new file mode 100644 index 00000000..5212ea3f --- /dev/null +++ b/plugins/guides/frontend/utils/proof.js @@ -0,0 +1,145 @@ +/** + * Proofing: the mistakes that matter in a PROCEDURE, not a document. + * + * A spellchecker catches "teh". None of these are spelling, and every one has + * been seen for real: + * + * - a smart quote in a command, so the command does not run + * - a checklist item with no text, so a step is silently missing + * - a heading with nothing under it, left when a draft was reorganised + * - an image with no alt text, invisible to anyone who cannot see it + * - a TODO that was never finished, published anyway + * + * Warnings, not blocks. A person publishing a guide at 14:40 with one empty + * bullet should be told, not stopped - the tool does not know that the bullet is + * deliberate, and a procedure that exists imperfectly beats one nobody wrote. + */ + +// Characters an editor or a word processor substitutes silently. In prose they +// are harmless; in a command they are the difference between working and a +// syntax error nobody can explain. +const SMARTCHARS = [ + // Written as escapes, not literals: the repo's style gate forbids non-ASCII in + // source, and a file whose job is detecting smart quotes cannot contain any. + // The escapes are also unambiguous in a diff, where a curly quote and a + // straight one look identical. + ['\u2018', "'"], ['\u2019', "'"], // single quotes + ['\u201C', '"'], ['\u201D', '"'], // double quotes + ['\u2013', '-'], ['\u2014', '-'], // en dash, em dash + ['\u00A0', ' '], // non-breaking space +] + +const UNFINISHED = /\b(TODO|TBD|FIXME|XXX|\?\?\?)\b/i + +function issue (level, blockid, message, fix) { + return { level, blockid, message, fix: fix || null } +} + +export function proof (guide, blocks) { + const issues = [] + + if (!guide.title || !guide.title.trim()) { + issues.push(issue('error', null, 'This guide has no title')) + } + if (!guide.summary || !guide.summary.trim()) { + issues.push(issue('warn', null, + 'No summary. It is what people read in the list and in search results.')) + } + if (!guide.tags || !guide.tags.trim()) { + issues.push(issue('warn', null, 'No tags, so this will be hard to find')) + } + if (!blocks.length) { + issues.push(issue('error', null, 'This guide is empty')) + return issues + } + + blocks.forEach((block, index) => { + const where = 'block ' + (index + 1) + ' (' + block.type + ')' + + if (block.type === 'code') { + // The important one. An autocorrected quote in a command looks right and + // does not run. + // + // Reported ONCE per block, however many offending characters it has: a + // command with an opening and a closing smart quote is one problem with + // one fix, and listing it twice makes the panel look like it is nagging. + const found = SMARTCHARS.filter(([bad]) => (block.text || '').includes(bad)) + if (found.length) { + issues.push(issue('error', block.id, + where + ': contains ' + found.map(([, good]) => 'a curly or long ' + + (good === "'" ? 'quote' : good === '"' ? 'double quote' : + good === '-' ? 'dash' : 'space')).filter( + (v, i, a) => a.indexOf(v) === i).join(' and ') + + ' where a command needs the plain character. Autocorrect does this, ' + + 'and the command will not run.', + { type: 'replaceAll', pairs: found })) + } + if (!(block.text || '').trim()) { + issues.push(issue('error', block.id, where + ' is empty')) + } + } else if (['heading', 'paragraph', 'callout'].includes(block.type)) { + if (!(block.text || '').trim() && !(block.title || '').trim()) { + issues.push(issue('warn', block.id, where + ' is empty')) + } + } + + if (block.type === 'step' && !(block.title || '').trim() + && !(block.text || '').trim()) { + issues.push(issue('warn', block.id, where + ' has no title and no text')) + } + + if (['checklist', 'bullets', 'numbers'].includes(block.type)) { + const empties = (block.items || []).filter( + item => !String(typeof item === 'string' ? item : item.text || '').trim()) + if (empties.length) { + issues.push(issue('warn', block.id, + where + ': ' + empties.length + ' empty item' + + (empties.length > 1 ? 's' : '') + + '. An empty step reads as a step somebody forgot to write.')) + } + } + + if (block.type === 'image' && !(block.alt || '').trim()) { + issues.push(issue('warn', block.id, + where + ': no alt text. Anyone who cannot see the image gets nothing.')) + } + + if (block.type === 'table' && !(block.rows || []).length) { + issues.push(issue('warn', block.id, where + ' has a header but no rows')) + } + + const text = [block.text, block.title].filter(Boolean).join(' ') + if (UNFINISHED.test(text)) { + issues.push(issue('warn', block.id, + where + ': still contains ' + text.match(UNFINISHED)[0])) + } + }) + + // A heading immediately followed by another heading usually means the section + // under the first one was moved or never written. + blocks.forEach((block, index) => { + if (block.type !== 'heading') return + const next = blocks[index + 1] + if (!next || next.type === 'heading') { + issues.push(issue('warn', block.id, + 'Heading "' + (block.text || '').slice(0, 40) + '" has nothing under it')) + } + }) + + return issues +} + +/** Apply a suggested fix in place, for the ones where the answer is obvious. */ +export function applyFix (block, fix) { + if (!fix) return false + const before = block.text || '' + let after = before + if (fix.type === 'replaceAll') { + for (const [bad, good] of fix.pairs) after = after.split(bad).join(good) + } else if (fix.type === 'replace') { + after = after.split(fix.from).join(fix.to) + } + if (after === before) return false + block.text = after + return true +} diff --git a/plugins/guides/frontend/views/GuideEditor.vue b/plugins/guides/frontend/views/GuideEditor.vue index 1413f69a..7105497a 100644 --- a/plugins/guides/frontend/views/GuideEditor.vue +++ b/plugins/guides/frontend/views/GuideEditor.vue @@ -44,6 +44,30 @@ + +
+
+ {{ errorCount }} to fix, {{ issues.length - errorCount }} worth a look + +
+ +
+
proof(guide, blocks.value)) +const errorCount = computed(() => issues.value.filter(i => i.level === 'error').length) + +function jumpTo (blockid) { + focused.value = blockid + const node = textareas[blockid] || itemInputs[blockid] + if (node?.scrollIntoView) { + node.scrollIntoView({ behavior: 'smooth', block: 'center' }) + node.focus?.() + } +} + +function fix (item) { + const block = blocks.value.find(b => b.id === item.blockid) + if (block && applyFix(block, item.fix)) dirty.value = true +} const isNew = computed(() => !guide.guideid) const canpublish = computed(() => auth.hasPermission?.('guides.publish') ?? false) @@ -425,6 +470,12 @@ onBeforeRouteLeave(() => { }) async function publish () { + if (errorCount.value) { + const ok = window.confirm( + errorCount.value + ' problem(s) would go out with this guide, including ' + + 'anything flagged in a command. Publish anyway?') + if (!ok) return + } await save(true) try { const response = await api.post('/guides/' + guide.guideid + '/publish') @@ -608,6 +659,26 @@ onMounted(async () => { .ge-previewtitle { margin: 0 0 0.2rem; font-size: 1.4rem; } .ge-previewsummary { color: var(--text-light); margin: 0 0 1rem; } +.ge-proof { + border: 1px solid var(--border); border-left: 3px solid var(--warning, #d9822b); + border-radius: 4px; padding: 0.5rem 0.7rem; margin-bottom: 1rem; + font-size: 0.82rem; +} +.ge-proofhead { display: flex; align-items: center; gap: 0.5rem; } +.ge-proofhead button { + margin-left: auto; font-size: 0.72rem; background: none; border: none; + color: var(--text-light); cursor: pointer; text-decoration: underline; +} +.ge-proof ul { margin: 0.4rem 0 0; padding: 0; list-style: none; } +.ge-proof li { display: flex; gap: 0.4rem; align-items: baseline; padding: 0.15rem 0; } +.ge-error span { color: var(--danger, #c0392b); } +.ge-warn span { color: var(--text-light); } +.ge-jump, .ge-fix { + font-size: 0.68rem; padding: 0.05rem 0.35rem; border: 1px solid var(--border); + background: transparent; color: var(--text-light); border-radius: 3px; cursor: pointer; +} +.ge-fix { margin-left: auto; border-color: var(--primary); color: var(--primary); } + .ge-revisions { margin-top: 2rem; border-top: 1px solid var(--border); padding-top: 0.8rem; } .ge-rev { display: flex; gap: 0.6rem; align-items: center; font-size: 0.8rem; padding: 0.25rem 0; } .ge-revnum { font-weight: 600; } diff --git a/shopdb/core/api/search.py b/shopdb/core/api/search.py index d023304c..70da6bc1 100644 --- a/shopdb/core/api/search.py +++ b/shopdb/core/api/search.py @@ -255,6 +255,55 @@ def _search_applications(query, search_term): return results +def _search_guides(query, search_term): + """Search guides by title, summary, tags and BODY. + + The body matters more here than in most domains. People look for a procedure + by something written halfway down it - "the one about the drive letter" - + not by a title they never read. The body is a JSON blob of blocks, so this + 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. + """ + results = [] + try: + _require_enabled('guides') + from plugins.guides.models import Guide + + rows = Guide.query.filter( + Guide.isactive == True, + Guide.publishedrevisionid.isnot(None), + _word_match(query, Guide.title, Guide.summary, Guide.tags, + Guide.blocksjson) + ).order_by(Guide.modifieddate.desc()).limit(20).all() + + for guide 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(): + relevance += 20 + if guide.tags and query.lower() in guide.tags.lower(): + relevance += 15 + + results.append({ + 'type': 'guide', + 'id': guide.guideid, + 'title': guide.title, + 'subtitle': guide.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}") + return results + + def _search_knowledgebase(query, search_term): """Search Knowledge Base by description and keywords. @@ -1148,6 +1197,7 @@ def global_search(): # Run all search domains results.extend(_search_applications(query, search_term)) results.extend(_search_knowledgebase(query, search_term)) + results.extend(_search_guides(query, search_term)) results.extend(_search_employees(query, search_term)) results.extend(_search_assets(query, search_term)) results.extend(_search_measuringtools(query, search_term)) diff --git a/tests/test_core/test_search_coverage.py b/tests/test_core/test_search_coverage.py index 57c53115..d80fd3cb 100644 --- a/tests/test_core/test_search_coverage.py +++ b/tests/test_core/test_search_coverage.py @@ -266,3 +266,75 @@ def test_an_ip_search_links_to_the_matched_subnet(client, subnetrow): assert len(rows) == 1 assert rows[0]['id'] == subnetrow assert rows[0]['url'] == f'/networks/{subnetrow}' + + +# --------------------------------------------------------------------------- +# Guides. A guide is not an asset either, so the generic asset search cannot see +# one - and the draft rule makes the omission worse than invisibility: a +# half-written procedure reachable through the search box is one somebody +# follows. +# --------------------------------------------------------------------------- + +@pytest.fixture +def guides(db): + """One published guide and one draft, with distinctive text. + + The published one carries its distinguishing words in the BODY, not the + title, because that is how people look for a procedure - by something + written halfway down it. + """ + from plugins.guides.models import Guide + from plugins.guides.services import authoring + + published = Guide(title='Zzmarker controller swap', slug='zzmarker-swap', + status='draft') + draft = Guide(title='Zzdraft unfinished procedure', slug='zzdraft-unfinished', + status='draft') + db.session.add_all([published, draft]) + db.session.flush() + + authoring.save(published, title=published.title, + summary='Restoring after a swap', tags='zztelesis, zzbackups', + blocks=[{'id': 'g1', 'type': 'paragraph', + 'text': 'Check the zzmappeddrive before starting.'}]) + authoring.publish(published) + authoring.save(draft, title=draft.title, summary='', tags='zztelesis', + blocks=[{'id': 'g2', 'type': 'paragraph', + 'text': 'Half written zzmappeddrive notes.'}]) + db.session.commit() + return {'published': published.guideid, 'draft': draft.guideid} + + +def test_a_published_guide_is_found_by_title(client, guides): + rows = [r for r in search(client, 'Zzmarker') if r['type'] == 'guide'] + assert len(rows) == 1 + assert rows[0]['id'] == guides['published'] + assert rows[0]['url'] == '/guides/zzmarker-swap' + + +def test_a_guide_is_found_by_tag(client, guides): + """Tags are how a technician narrows to a family of procedures.""" + assert 'Zzmarker controller swap' in titles(client, 'zzbackups', 'guide') + + +def test_a_guide_is_found_by_words_in_its_body(client, guides): + """People search for what they remember reading, not for the title. + + The body is stored as a JSON blob of blocks, so this is the check that the + blob is actually reachable rather than merely present. + """ + assert 'Zzmarker controller swap' in titles(client, 'zzmappeddrive', 'guide') + + +def test_a_draft_guide_is_never_returned(client, guides): + """The draft rule has to hold on EVERY path that reaches the row. + + A draft is deliberately invisible to readers, and search is where that rule + would otherwise leak: somebody finds a half-written procedure through the + search box and follows it. The draft here shares a tag and a body word with + the published guide, so it would ride along on both queries if the filter + were missing. + """ + assert 'Zzdraft unfinished procedure' not in titles(client, 'zzmappeddrive', 'guide') + assert 'Zzdraft unfinished procedure' not in titles(client, 'zztelesis', 'guide') + assert titles(client, 'Zzdraft', 'guide') == []