From f221fba54076157c7b11cb45dd20dbf615dd8409 Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Wed, 26 Aug 2026 12:42:24 -0400 Subject: [PATCH 1/3] Guides: procedures written and read inside ShopDB --- frontend/src/composables/systemSettings.js | 1 + frontend/src/views/SearchResults.vue | 5 + plugins/guides/__init__.py | 3 + plugins/guides/api/__init__.py | 3 + plugins/guides/api/routes.py | 442 +++++++++++ .../frontend/components/GuideBlocks.vue | 257 +++++++ plugins/guides/frontend/routes.js | 42 ++ plugins/guides/frontend/utils/inline.js | 82 +++ plugins/guides/frontend/utils/proof.js | 145 ++++ plugins/guides/frontend/utils/templates.js | 74 ++ plugins/guides/frontend/views/GuideEditor.vue | 691 ++++++++++++++++++ plugins/guides/frontend/views/GuideView.vue | 271 +++++++ plugins/guides/frontend/views/GuidesList.vue | 163 +++++ plugins/guides/migrations/env.py | 16 + plugins/guides/migrations/script.py.mako | 24 + .../versions/0001_guides_baseline.py | 114 +++ plugins/guides/models/__init__.py | 4 +- plugins/guides/models/guide.py | 120 ++- plugins/guides/plugin.py | 92 +++ plugins/guides/services/authoring.py | 133 ++++ plugins/guides/services/blocks.py | 21 + shopdb/core/api/search.py | 50 ++ shopdb/core/api/settings.py | 1 + shopdb/plugins/alembic_template.py | 1 + tests/test_core/test_search_coverage.py | 72 ++ tests/test_plugin_migrations.py | 3 + tests/test_plugins/test_guides.py | 334 +++++++++ 27 files changed, 3152 insertions(+), 12 deletions(-) create mode 100644 plugins/guides/__init__.py create mode 100644 plugins/guides/api/__init__.py create mode 100644 plugins/guides/api/routes.py create mode 100644 plugins/guides/frontend/components/GuideBlocks.vue create mode 100644 plugins/guides/frontend/routes.js create mode 100644 plugins/guides/frontend/utils/inline.js create mode 100644 plugins/guides/frontend/utils/proof.js create mode 100644 plugins/guides/frontend/utils/templates.js create mode 100644 plugins/guides/frontend/views/GuideEditor.vue create mode 100644 plugins/guides/frontend/views/GuideView.vue create mode 100644 plugins/guides/frontend/views/GuidesList.vue create mode 100644 plugins/guides/migrations/env.py create mode 100644 plugins/guides/migrations/script.py.mako create mode 100644 plugins/guides/migrations/versions/0001_guides_baseline.py create mode 100644 plugins/guides/plugin.py create mode 100644 plugins/guides/services/authoring.py create mode 100644 tests/test_plugins/test_guides.py diff --git a/frontend/src/composables/systemSettings.js b/frontend/src/composables/systemSettings.js index 4f9fa842..69b090a9 100644 --- a/frontend/src/composables/systemSettings.js +++ b/frontend/src/composables/systemSettings.js @@ -45,6 +45,7 @@ export const assetTypeCols = [ export const searchDomains = [ { key: 'application', label: 'Applications' }, { key: 'knowledgebase', label: 'Knowledge Base' }, + { key: 'guide', label: 'Guides' }, { key: 'employee', label: 'Employees' }, { key: 'machine', label: 'Machines' }, { key: 'computer', label: 'PCs' }, 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/__init__.py b/plugins/guides/__init__.py new file mode 100644 index 00000000..e14bbc43 --- /dev/null +++ b/plugins/guides/__init__.py @@ -0,0 +1,3 @@ +from .plugin import GuidesPlugin + +__all__ = ['GuidesPlugin'] diff --git a/plugins/guides/api/__init__.py b/plugins/guides/api/__init__.py new file mode 100644 index 00000000..422e0a08 --- /dev/null +++ b/plugins/guides/api/__init__.py @@ -0,0 +1,3 @@ +from .routes import guides_bp + +__all__ = ['guides_bp'] diff --git a/plugins/guides/api/routes.py b/plugins/guides/api/routes.py new file mode 100644 index 00000000..ac59426c --- /dev/null +++ b/plugins/guides/api/routes.py @@ -0,0 +1,442 @@ +"""Guides API. + +TWO WAYS TO READ A GUIDE, on purpose: + + GET /api/guides/ what was PUBLISHED + GET /api/guides//working the working copy, for editors + +A reader following a procedure must not have it change under them because +somebody upstairs is editing. The published pointer is what makes that true, and +serving the working copy from the reader's route would quietly undo it. +""" + +from datetime import datetime +import re + +from flask import Blueprint, request, Response +from flask_jwt_extended import jwt_required, get_jwt_identity, current_user + +from shopdb.api import ( + db, + success_response, error_response, ErrorCodes, + require_permission, +) + +from ..models import Guide, GuideRevision, GuideImage +from ..services import authoring +from ..services.blocks import BlockError, checklistitemcount + +guides_bp = Blueprint('guides', __name__) + + +# Screenshots of a settings dialog, not photographs. Generous for the former and +# small enough that a pasted phone picture is refused rather than silently +# making a guide take ten seconds to open on a shopfloor PC. +MAXIMAGEBYTES = 8 * 1024 * 1024 +ALLOWEDIMAGETYPES = { + 'image/png': '.png', + 'image/jpeg': '.jpg', + 'image/gif': '.gif', + 'image/webp': '.webp', +} + + +def _userid(): + if current_user is not None: + return current_user.userid + try: + return int(get_jwt_identity()) + except (TypeError, ValueError): + return None + + +def _has(permission): + """Whether the caller holds a permission, without raising. + + require_permission decorates a route; this is for the in-route decisions - + may this person see drafts, may they edit this particular guide. Same check + the decorator makes, including the admin bypass that lives inside + haspermission. + """ + return bool(current_user is not None + and current_user.haspermission(permission)) + + +def _mayedit(guide): + """Edit rights for one guide. + + guides.edit covers everything. Otherwise an author may edit their OWN guide + while it is unpublished - which is what makes drafting useful without + handing everyone rights over published procedures. Once it is published it + takes guides.edit, because by then other people are following it. + """ + if _has('guides.edit'): + return True + if not _has('guides.create'): + return False + return guide.createdby == _userid() and not guide.publishedrevisionid + + +def _slugify(title, guideid=None): + """A readable, stable, unique slug. + + Stored rather than derived on read, so renaming a guide cannot break a link + somebody pasted into a work order months ago. + """ + base = re.sub(r'[^a-z0-9]+', '-', (title or '').lower()).strip('-') or 'guide' + base = base[:180] # room for a '-2' style suffix inside the 191 cap + candidate = base + suffix = 2 + while True: + clash = Guide.query.filter(Guide.slug == candidate) + if guideid: + clash = clash.filter(Guide.guideid != guideid) + if not clash.first(): + return candidate + candidate = '{}-{}'.format(base, suffix) + suffix += 1 + + +# ---------------------------------------------------------------- reading --- + +@guides_bp.route('', methods=['GET']) +@jwt_required() +@require_permission('guides.view') +def list_guides(): + """Published guides, plus the caller's own drafts if they can author. + + A half-written procedure on a shop floor is worse than no procedure, so + drafts are invisible to readers - but an author must be able to find their + own, or drafting is a trap door. + """ + query = Guide.query + + search = (request.args.get('q') or '').strip() + if search: + like = '%{}%'.format(search) + query = query.filter(db.or_( + Guide.title.ilike(like), + Guide.summary.ilike(like), + Guide.tags.ilike(like), + # The body too: people look for "the one about the drive letter", + # not for a title they never read. + Guide.blocksjson.ilike(like), + )) + + tag = (request.args.get('tag') or '').strip() + if tag: + query = query.filter(Guide.tags.ilike('%{}%'.format(tag))) + + caneditall = _has('guides.edit') + if not caneditall: + mine = _userid() + query = query.filter(db.or_( + Guide.publishedrevisionid.isnot(None), + Guide.createdby == mine, + )) + + rows = query.order_by(Guide.modifieddate.desc()).all() + + out = [] + for guide in rows: + data = guide.to_dict() + data['ispublished'] = bool(guide.publishedrevisionid) + data['checklistitems'] = checklistitemcount(guide.blocks) + data['canedit'] = _mayedit(guide) + out.append(data) + return success_response(out) + + +@guides_bp.route('/tags', methods=['GET']) +@jwt_required() +@require_permission('guides.view') +def list_tags(): + """Every tag in use, with a count, for the filter chips.""" + counts = {} + for (tags,) in db.session.query(Guide.tags).filter(Guide.tags.isnot(None)).all(): + for tag in [t.strip() for t in tags.split(',') if t.strip()]: + counts[tag] = counts.get(tag, 0) + 1 + return success_response([ + {'tag': tag, 'count': count} + for tag, count in sorted(counts.items(), key=lambda kv: (-kv[1], kv[0].lower())) + ]) + + +@guides_bp.route('/', methods=['GET']) +@jwt_required() +@require_permission('guides.view') +def read_guide(slug): + """What a reader gets: the published revision. + + An unpublished guide is 404 to anyone who cannot edit it - not 403, which + would confirm it exists. + """ + guide = Guide.query.filter_by(slug=slug).first() + if guide is None: + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + + revision = authoring.readable(guide) + if revision is None: + if not _mayedit(guide): + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', + http_code=404) + # An editor previewing their own unpublished draft. + data = guide.to_dict(includeblocks=True) + data['ispublished'] = False + data['isdraftpreview'] = True + data['canedit'] = True + return success_response(data) + + data = guide.to_dict() + data.update({ + 'title': revision.title, + 'summary': revision.summary, + 'tags': [t.strip() for t in (revision.tags or '').split(',') if t.strip()], + 'blocks': revision.blocks, + 'revision': revision.revision, + 'ispublished': True, + 'isdraftpreview': False, + 'canedit': _mayedit(guide), + # So an editor can see there is newer unpublished work without leaving + # the page. + 'hasunpublishedchanges': guide.revision != revision.revision, + }) + return success_response(data) + + +@guides_bp.route('//working', methods=['GET']) +@jwt_required() +@require_permission('guides.view') +def read_working(guideid): + """The working copy, for the editor.""" + guide = db.session.get(Guide, guideid) + if guide is None: + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + if not _mayedit(guide): + return error_response(ErrorCodes.FORBIDDEN, + 'You do not have permission to edit this guide', + http_code=403) + data = guide.to_dict(includeblocks=True) + data['ispublished'] = bool(guide.publishedrevisionid) + data['canedit'] = True + return success_response(data) + + +@guides_bp.route('//revisions', methods=['GET']) +@jwt_required() +@require_permission('guides.view') +def list_revisions(guideid): + guide = db.session.get(Guide, guideid) + if guide is None: + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + rows = (GuideRevision.query.filter_by(guideid=guideid) + .order_by(GuideRevision.revision.desc()).all()) + published = guide.publishedrevisionid + return success_response([ + dict(row.to_dict(), ispublished=(row.guiderevisionid == published)) + for row in rows + ]) + + +@guides_bp.route('/revisions/', methods=['GET']) +@jwt_required() +@require_permission('guides.view') +def read_revision(revisionid): + revision = db.session.get(GuideRevision, revisionid) + if revision is None: + return error_response(ErrorCodes.NOT_FOUND, 'Revision not found', + http_code=404) + return success_response(revision.to_dict(includeblocks=True)) + + +# ---------------------------------------------------------------- writing --- + +@guides_bp.route('', methods=['POST']) +@jwt_required() +@require_permission('guides.create') +def create_guide(): + data = request.get_json(silent=True) or {} + title = (data.get('title') or '').strip() + if not title: + return error_response(ErrorCodes.VALIDATION_ERROR, 'A guide needs a title') + + guide = Guide(title=title, slug=_slugify(title), status='draft', + createdby=_userid(), updatedby=_userid()) + db.session.add(guide) + db.session.flush() + + try: + authoring.save(guide, + title=title, + summary=data.get('summary', ''), + tags=data.get('tags', ''), + blocks=data.get('blocks', []), + note=data.get('note'), + userid=_userid()) + except BlockError as exc: + db.session.rollback() + return error_response(ErrorCodes.VALIDATION_ERROR, str(exc)) + + db.session.commit() + return success_response(guide.to_dict(includeblocks=True), + message='Guide created') + + +@guides_bp.route('/', methods=['PUT']) +@jwt_required() +@require_permission('guides.view') +def update_guide(guideid): + guide = db.session.get(Guide, guideid) + if guide is None: + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + if not _mayedit(guide): + return error_response(ErrorCodes.FORBIDDEN, + 'You do not have permission to edit this guide', + http_code=403) + + data = request.get_json(silent=True) or {} + if 'title' in data and not (data.get('title') or '').strip(): + return error_response(ErrorCodes.VALIDATION_ERROR, 'A guide needs a title') + + try: + guide, revision = authoring.save( + guide, + title=data.get('title'), + summary=data.get('summary'), + tags=data.get('tags'), + blocks=data.get('blocks'), + note=data.get('note'), + userid=_userid()) + except BlockError as exc: + db.session.rollback() + return error_response(ErrorCodes.VALIDATION_ERROR, str(exc)) + + if 'title' in data: + guide.slug = _slugify(guide.title, guide.guideid) + + db.session.commit() + result = guide.to_dict(includeblocks=True) + # 'unchanged' is a real outcome, not a failure: an autosave where the author + # changed nothing must not manufacture a version. + result['saved'] = revision is not None + return success_response(result, + message='Saved' if revision else 'No changes to save') + + +@guides_bp.route('//publish', methods=['POST']) +@jwt_required() +@require_permission('guides.publish') +def publish_guide(guideid): + guide = db.session.get(Guide, guideid) + if guide is None: + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + try: + revision = authoring.publish(guide, userid=_userid()) + except ValueError as exc: + return error_response(ErrorCodes.VALIDATION_ERROR, str(exc)) + db.session.commit() + return success_response(guide.to_dict(), + message='Published version {}'.format(revision.revision)) + + +@guides_bp.route('//unpublish', methods=['POST']) +@jwt_required() +@require_permission('guides.publish') +def unpublish_guide(guideid): + guide = db.session.get(Guide, guideid) + if guide is None: + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + authoring.unpublish(guide, userid=_userid()) + db.session.commit() + return success_response(guide.to_dict(), message='Withdrawn from readers') + + +@guides_bp.route('//restore/', methods=['POST']) +@jwt_required() +@require_permission('guides.view') +def restore_revision(guideid, revisionid): + guide = db.session.get(Guide, guideid) + revision = db.session.get(GuideRevision, revisionid) + if guide is None or revision is None or revision.guideid != guideid: + return error_response(ErrorCodes.NOT_FOUND, 'Revision not found', + http_code=404) + if not _mayedit(guide): + return error_response(ErrorCodes.FORBIDDEN, + 'You do not have permission to edit this guide', + http_code=403) + + authoring.restore(guide, revision, userid=_userid()) + db.session.commit() + return success_response(guide.to_dict(includeblocks=True), + message='Restored version {} as a new draft version' + .format(revision.revision)) + + +@guides_bp.route('/', methods=['DELETE']) +@jwt_required() +@require_permission('guides.delete') +def delete_guide(guideid): + guide = db.session.get(Guide, guideid) + if guide is None: + return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + db.session.delete(guide) + db.session.commit() + return success_response({'guideid': guideid}, message='Guide deleted') + + +# ----------------------------------------------------------------- images --- + +@guides_bp.route('/images', methods=['POST']) +@jwt_required() +@require_permission('guides.create') +def upload_image(): + """Take a pasted or dropped image. + + Unattached at first: an image is pasted into a NEW guide before that guide + has an id. The editor attaches it on save; anything never attached is + orphaned and prunable, which is a tidier problem than refusing to accept a + paste until the author has saved. + """ + upload = request.files.get('file') + if upload is None: + return error_response(ErrorCodes.VALIDATION_ERROR, 'No file provided') + + mimetype = (upload.mimetype or '').lower() + if mimetype not in ALLOWEDIMAGETYPES: + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'Unsupported image type {!r}. Allowed: {}'.format( + mimetype, ', '.join(sorted(ALLOWEDIMAGETYPES)))) + + data = upload.read() + if not data: + return error_response(ErrorCodes.VALIDATION_ERROR, 'That file is empty') + if len(data) > MAXIMAGEBYTES: + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'That image is {:.1f} MB; the limit is {} MB'.format( + len(data) / 1048576, MAXIMAGEBYTES // 1048576)) + + guideid = request.form.get('guideid', type=int) + image = GuideImage(guideid=guideid, filename=upload.filename, + mimetype=mimetype, bytesize=len(data), data=data, + uploadedby=_userid()) + db.session.add(image) + db.session.commit() + return success_response(image.to_dict(), message='Image uploaded') + + +@guides_bp.route('/images/', methods=['GET']) +@jwt_required() +@require_permission('guides.view') +def serve_image(imageid): + image = db.session.get(GuideImage, imageid) + if image is None: + return error_response(ErrorCodes.NOT_FOUND, 'Image not found', http_code=404) + return Response(image.data, mimetype=image.mimetype, headers={ + # Immutable: an image is never edited in place, only replaced by a new + # upload with a new id, so it can be cached hard. + 'Cache-Control': 'private, max-age=31536000, immutable', + 'Content-Disposition': 'inline; filename="{}"'.format( + image.filename or 'image{}'.format(imageid)), + }) diff --git a/plugins/guides/frontend/components/GuideBlocks.vue b/plugins/guides/frontend/components/GuideBlocks.vue new file mode 100644 index 00000000..ebc24551 --- /dev/null +++ b/plugins/guides/frontend/components/GuideBlocks.vue @@ -0,0 +1,257 @@ + + + + + diff --git a/plugins/guides/frontend/routes.js b/plugins/guides/frontend/routes.js new file mode 100644 index 00000000..5958a0e3 --- /dev/null +++ b/plugins/guides/frontend/routes.js @@ -0,0 +1,42 @@ +/** + * Guides plugin routes. + * + * Three views, and the split between the last two is deliberate: reading a + * guide and editing one are different jobs with different rights, and the + * checkboxes are live in exactly one of them. + * + * meta.plugin = 'guides' so the ADR-009 router guard redirects when the backend + * plugin is disabled. Every route requires auth because every guides API route + * is behind JWT plus a guides.* permission. + */ +export default [ + { + path: 'guides', + name: 'guides', + component: () => import('./views/GuidesList.vue'), + meta: { requiresAuth: true, plugin: 'guides' } + }, + { + path: 'guides/new', + name: 'guide-new', + component: () => import('./views/GuideEditor.vue'), + meta: { requiresAuth: true, plugin: 'guides' } + }, + { + path: 'guides/:slug', + name: 'guide-view', + component: () => import('./views/GuideView.vue'), + meta: { requiresAuth: true, plugin: 'guides' } + }, + { + path: 'guides/:guideid/edit', + name: 'guide-edit', + component: () => import('./views/GuideEditor.vue'), + meta: { requiresAuth: true, plugin: 'guides' } + } +] + +// No full-screen routes: every view renders inside AppLayout. Declared so +// routes.gen.js can read `.toplevel` off the module without Rollup warning +// about a missing export. +export const toplevel = [] diff --git a/plugins/guides/frontend/utils/inline.js b/plugins/guides/frontend/utils/inline.js new file mode 100644 index 00000000..b2ce2356 --- /dev/null +++ b/plugins/guides/frontend/utils/inline.js @@ -0,0 +1,82 @@ +/** + * Inline text rendering for guide blocks. + * + * A guide stores its text as a RESTRICTED Markdown subset - bold, italic, + * inline code, links, line breaks - and nothing else. This turns that into + * HTML. + * + * ESCAPE FIRST, THEN FORMAT. The order is the whole security model: every + * angle bracket and ampersand becomes an entity before any pattern is applied, + * so no markup an author pasted can survive into the DOM. There is no path here + * that emits a tag the code below did not write itself. + * + * The restriction is not only about safety. Pasted rich text carries fonts and + * hardcoded colours, which then fight the theme - black text on a dark + * background, and nobody can say why. Emphasis is semantic; the stylesheet + * decides how it looks. That is what makes dark mode work without an author + * thinking about it, and why a guide written next year still matches one + * written today. + */ + +function escapeHtml (text) { + return String(text == null ? '' : text) + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, ''') +} + +// Only http(s) and mailto. A javascript: or data: URL in a link is the one way +// an author could otherwise smuggle script past the escaping above. +function safeHref (url) { + const trimmed = String(url || '').trim() + if (/^(https?:|mailto:)/i.test(trimmed)) return trimmed + if (trimmed.startsWith('/')) return trimmed // in-app link + return null +} + +export function renderInline (text) { + let html = escapeHtml(text) + + // Inline code first, so formatting characters inside `code` stay literal - + // a path like C:\**temp** should not turn half of itself bold. + const codeSpans = [] + html = html.replace(/`([^`]+)`/g, (match, code) => { + codeSpans.push(code) + return '\u0000CODE' + (codeSpans.length - 1) + '\u0000' + }) + + // [label](url). The URL pattern allows ONE level of balanced parentheses so + // real links survive - Wikipedia-style /wiki/Foo_(bar) is the common case, + // and a naive [^)]+ silently truncates it into a broken link. + html = html.replace(/\[([^\]]+)\]\(((?:[^()\s]|\([^()\s]*\))+)\)/g, (match, label, url) => { + const href = safeHref(url) + if (!href) return label + const external = /^https?:/i.test(href) + return '' + label + '' + }) + + // **bold** then *italic*, longest marker first so **x** is not read as + // *(*x*)*. + html = html.replace(/\*\*([^*]+)\*\*/g, '$1') + html = html.replace(/(^|[^*])\*([^*]+)\*(?!\*)/g, '$1$2') + + html = html.replace(/\n/g, '
') + + html = html.replace(/\u0000CODE(\d+)\u0000/g, (match, index) => + '' + codeSpans[Number(index)] + '') + + return html +} + +/** Plain text, for titles and anywhere markup would be noise. */ +export function stripInline (text) { + return String(text == null ? '' : text) + .replace(/`([^`]+)`/g, '$1') + .replace(/\[([^\]]+)\]\([^)\s]+\)/g, '$1') + .replace(/\*\*([^*]+)\*\*/g, '$1') + .replace(/\*([^*]+)\*/g, '$1') +} 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/utils/templates.js b/plugins/guides/frontend/utils/templates.js new file mode 100644 index 00000000..1a166f81 --- /dev/null +++ b/plugins/guides/frontend/utils/templates.js @@ -0,0 +1,74 @@ +/** + * Starting points for a new guide. + * + * The blank page is the real barrier to somebody writing a procedure down. A + * skeleton with the right SHAPE - what you need first, the steps, how you know + * it worked, what to do when it does not - gets a half-decent guide out of + * somebody who would otherwise have written nothing. + * + * The shape is lifted from the KB articles the team already writes, so guides + * written here read like the ones written before them. + */ + +function id (prefix, n) { + return prefix + n +} + +export const templates = [ + { + key: 'blank', + name: 'Blank', + description: 'Start with nothing', + blocks: () => [], + }, + { + key: 'procedure', + name: 'Procedure', + description: 'Before you start, numbered steps, how to check it worked', + blocks: () => [ + { id: id('t', 1), type: 'paragraph', + text: 'What this covers, and when somebody would need it.' }, + { id: id('t', 2), type: 'callout', variant: 'warning', title: 'Before you start', + text: 'Anything that has to be true first, or that will bite halfway through.' }, + { id: id('t', 3), type: 'heading', text: 'Steps' }, + { id: id('t', 4), type: 'step', title: 'First step', text: '' }, + { id: id('t', 5), type: 'step', title: 'Second step', text: '' }, + { id: id('t', 6), type: 'heading', text: 'Check it worked' }, + { id: id('t', 7), type: 'checklist', items: [ + { id: id('ti', 1), text: '' }, + { id: id('ti', 2), text: '' }, + ] }, + ], + }, + { + key: 'troubleshooting', + name: 'Troubleshooting', + description: 'Symptom, likely causes, what to check first', + blocks: () => [ + { id: id('t', 1), type: 'paragraph', text: 'The symptom, as somebody would describe it.' }, + { id: id('t', 2), type: 'heading', text: 'Check these first' }, + { id: id('t', 3), type: 'checklist', items: [ + { id: id('ti', 1), text: '' }, + { id: id('ti', 2), text: '' }, + ] }, + { id: id('t', 4), type: 'heading', text: 'Likely causes' }, + { id: id('t', 5), type: 'table', + header: ['What you see', 'What it usually means'], rows: [['', '']] }, + { id: id('t', 6), type: 'callout', variant: 'info', + text: 'The thing that looks like the cause and is not.' }, + ], + }, + { + key: 'checklist', + name: 'Checklist', + description: 'A list to work through, nothing else', + blocks: () => [ + { id: id('t', 1), type: 'paragraph', text: 'When to use this list.' }, + { id: id('t', 2), type: 'checklist', items: [ + { id: id('ti', 1), text: '' }, + { id: id('ti', 2), text: '' }, + { id: id('ti', 3), text: '' }, + ] }, + ], + }, +] diff --git a/plugins/guides/frontend/views/GuideEditor.vue b/plugins/guides/frontend/views/GuideEditor.vue new file mode 100644 index 00000000..7105497a --- /dev/null +++ b/plugins/guides/frontend/views/GuideEditor.vue @@ -0,0 +1,691 @@ + + + + + diff --git a/plugins/guides/frontend/views/GuideView.vue b/plugins/guides/frontend/views/GuideView.vue new file mode 100644 index 00000000..e0ff3686 --- /dev/null +++ b/plugins/guides/frontend/views/GuideView.vue @@ -0,0 +1,271 @@ + + + + + + + diff --git a/plugins/guides/frontend/views/GuidesList.vue b/plugins/guides/frontend/views/GuidesList.vue new file mode 100644 index 00000000..052d9eea --- /dev/null +++ b/plugins/guides/frontend/views/GuidesList.vue @@ -0,0 +1,163 @@ + + + + + diff --git a/plugins/guides/migrations/env.py b/plugins/guides/migrations/env.py new file mode 100644 index 00000000..f3de904d --- /dev/null +++ b/plugins/guides/migrations/env.py @@ -0,0 +1,16 @@ +"""Alembic environment for the guides plugin migration chain. + +Delegates to the shared runner in shopdb.plugins.alembic_template, which filters +the metadata to this plugin's tables and drives Alembic against the per-plugin +version table alembic_version_guides. See ADR-008 for the ownership model. + +Like the backups plugin and unlike the cutover ones, guides is NEW: its 0001 +baseline really CREATES its tables, because the core chain never built them. +""" +import os + +os.environ['PLUGIN_NAME'] = 'guides' + +from shopdb.plugins.alembic_template import run_migrations # noqa: E402 + +run_migrations() diff --git a/plugins/guides/migrations/script.py.mako b/plugins/guides/migrations/script.py.mako new file mode 100644 index 00000000..2c015630 --- /dev/null +++ b/plugins/guides/migrations/script.py.mako @@ -0,0 +1,24 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision = ${repr(up_revision)} +down_revision = ${repr(down_revision)} +branch_labels = ${repr(branch_labels)} +depends_on = ${repr(depends_on)} + + +def upgrade(): + ${upgrades if upgrades else "pass"} + + +def downgrade(): + ${downgrades if downgrades else "pass"} diff --git a/plugins/guides/migrations/versions/0001_guides_baseline.py b/plugins/guides/migrations/versions/0001_guides_baseline.py new file mode 100644 index 00000000..8964c3a7 --- /dev/null +++ b/plugins/guides/migrations/versions/0001_guides_baseline.py @@ -0,0 +1,114 @@ +"""guides plugin baseline: guides, guiderevisions, guideimages. + +A new plugin, so this builds its tables rather than stamping an anchor over +core-owned ones (contrast the cutover plugins, whose 0001 is a no-op). + +Idempotent: each table is created only if it is not already there, so a database +where an earlier run half-succeeded can be upgraded again without hand-editing +alembic_version_guides. +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy import inspect + + +revision = 'guides0001baseline' +down_revision = None +branch_labels = None +depends_on = None + + +def _tables(): + return set(inspect(op.get_bind()).get_table_names()) + + +def upgrade(): + existing = _tables() + + if 'guides' not in existing: + op.create_table( + 'guides', + sa.Column('guideid', sa.Integer(), nullable=False), + sa.Column('title', sa.String(length=300), nullable=False), + # 191 because this is UNIQUE-indexed and MySQL caps an index key at + # 767 bytes; utf8mb4 is 4 bytes per character. + sa.Column('slug', sa.String(length=191), nullable=False), + sa.Column('summary', sa.String(length=1000)), + sa.Column('tags', sa.String(length=500)), + sa.Column('status', sa.String(length=20), nullable=False, + server_default='draft'), + sa.Column('blocksjson', sa.dialects.mysql.MEDIUMTEXT() + if op.get_bind().dialect.name == 'mysql' else sa.Text(), + nullable=False), + sa.Column('publishedrevisionid', sa.Integer()), + sa.Column('createdby', sa.Integer()), + sa.Column('updatedby', sa.Integer()), + sa.Column('publishedat', sa.DateTime()), + # BaseModel contributes these to every model in the app; a plugin + # migration has to spell them out or the table is missing columns + # the ORM selects on every query. + sa.Column('createddate', sa.DateTime(), nullable=False), + sa.Column('modifieddate', sa.DateTime(), nullable=False), + sa.Column('isactive', sa.Boolean(), nullable=False), + sa.Column('revision', sa.Integer(), nullable=False, server_default='1'), + sa.PrimaryKeyConstraint('guideid'), + sa.UniqueConstraint('slug', name='uq_guides_slug'), + ) + op.create_index('idx_guides_status', 'guides', ['status']) + + if 'guiderevisions' not in existing: + op.create_table( + 'guiderevisions', + sa.Column('guiderevisionid', sa.Integer(), nullable=False), + sa.Column('guideid', sa.Integer(), nullable=False), + sa.Column('revision', sa.Integer(), nullable=False), + sa.Column('title', sa.String(length=300), nullable=False), + sa.Column('summary', sa.String(length=1000)), + sa.Column('tags', sa.String(length=500)), + sa.Column('blocksjson', sa.dialects.mysql.MEDIUMTEXT() + if op.get_bind().dialect.name == 'mysql' else sa.Text(), + nullable=False), + sa.Column('contenthash', sa.String(length=64), nullable=False), + sa.Column('note', sa.String(length=500)), + sa.Column('createdby', sa.Integer()), + # BaseModel contributes these to every model in the app; a plugin + # migration has to spell them out or the table is missing columns + # the ORM selects on every query. + sa.Column('createddate', sa.DateTime(), nullable=False), + sa.Column('modifieddate', sa.DateTime(), nullable=False), + sa.Column('isactive', sa.Boolean(), nullable=False), + sa.PrimaryKeyConstraint('guiderevisionid'), + sa.ForeignKeyConstraint(['guideid'], ['guides.guideid'], + ondelete='CASCADE'), + ) + op.create_index('idx_guiderevisions_guide', 'guiderevisions', ['guideid']) + op.create_index('idx_guiderevisions_hash', 'guiderevisions', ['contenthash']) + + if 'guideimages' not in existing: + op.create_table( + 'guideimages', + sa.Column('guideimageid', sa.Integer(), nullable=False), + sa.Column('guideid', sa.Integer()), + sa.Column('filename', sa.String(length=255)), + sa.Column('mimetype', sa.String(length=100), nullable=False), + sa.Column('bytesize', sa.Integer(), nullable=False), + sa.Column('data', sa.dialects.mysql.MEDIUMBLOB() + if op.get_bind().dialect.name == 'mysql' else sa.LargeBinary(), + nullable=False), + sa.Column('uploadedby', sa.Integer()), + # BaseModel contributes these to every model in the app; a plugin + # migration has to spell them out or the table is missing columns + # the ORM selects on every query. + sa.Column('createddate', sa.DateTime(), nullable=False), + sa.Column('modifieddate', sa.DateTime(), nullable=False), + sa.Column('isactive', sa.Boolean(), nullable=False), + sa.PrimaryKeyConstraint('guideimageid'), + sa.ForeignKeyConstraint(['guideid'], ['guides.guideid'], + ondelete='CASCADE'), + ) + op.create_index('idx_guideimages_guide', 'guideimages', ['guideid']) + + +def downgrade(): + for table in ('guideimages', 'guiderevisions', 'guides'): + op.drop_table(table) diff --git a/plugins/guides/models/__init__.py b/plugins/guides/models/__init__.py index c32b970e..02a44397 100644 --- a/plugins/guides/models/__init__.py +++ b/plugins/guides/models/__init__.py @@ -1,3 +1,3 @@ -from .guide import Guide, GuideImage, BLOCKTYPES +from .guide import Guide, GuideRevision, GuideImage, BLOCKTYPES -__all__ = ['Guide', 'GuideImage', 'BLOCKTYPES'] +__all__ = ['Guide', 'GuideRevision', 'GuideImage', 'BLOCKTYPES'] diff --git a/plugins/guides/models/guide.py b/plugins/guides/models/guide.py index 5df9ef4b..a3587798 100644 --- a/plugins/guides/models/guide.py +++ b/plugins/guides/models/guide.py @@ -74,7 +74,12 @@ class Guide(BaseModel): # derived on read so an edited title cannot silently break a link somebody # pasted into a work order. title = db.Column(db.String(300), nullable=False) - slug = db.Column(db.String(300), nullable=False, unique=True, index=True) + # 191, not 300: this column is UNIQUE-indexed, and MySQL caps an index key + # at 767 bytes. utf8mb4 stores up to 4 bytes per character, so 191 * 4 = + # 764 is the longest indexable string - 300 produces + # "Specified key was too long" the moment the migration runs on a real + # database, having passed every SQLite test. + slug = db.Column(db.String(191), nullable=False, unique=True, index=True) # One line, shown in the list and in search results. summary = db.Column(db.String(1000)) @@ -93,15 +98,39 @@ class Guide(BaseModel): # the same reason the backups plugin stores its projection as text. blocksjson = db.Column(db.Text(length=16777215), nullable=False, default='[]') + # What READERS see. Points at an immutable snapshot in guiderevisions, not + # at the working copy above. + # + # This is the whole point of separating the two: without it, editing a live + # procedure changes it under a technician who is halfway through following + # it. With it, they keep reading the published version until somebody + # deliberately publishes a new one. NULL means never published - a draft + # nobody outside the authors can see. + # + # use_alter, because this and guiderevisions.guideid form a cycle: a guide + # points at its published revision, and every revision points back at its + # guide. Without it SQLAlchemy cannot order CREATE/DROP and warns that it + # cannot sort the tables. The database-level constraint is deliberately not + # created by the migration either - the cycle buys nothing there, and the + # pointer is maintained in one place (services/authoring.py). + publishedrevisionid = db.Column( + db.Integer, db.ForeignKey('guiderevisions.guiderevisionid', + ondelete='SET NULL', + use_alter=True, + name='fk_guides_publishedrevision')) + + # createddate / modifieddate / isactive come from BaseModel - no second set + # of timestamps on top of them, or the next person has to work out which + # pair is authoritative. publishedat is ours because "when did readers start + # seeing this" is a different fact from "when was the row last touched". createdby = db.Column(db.Integer, db.ForeignKey('users.userid')) updatedby = db.Column(db.Integer, db.ForeignKey('users.userid')) - createdat = db.Column(db.DateTime, nullable=False, default=_utcnow) - updatedat = db.Column(db.DateTime, nullable=False, default=_utcnow, - onupdate=_utcnow) publishedat = db.Column(db.DateTime) - # Bumped on every save. Cheap, and it gives a printed copy something - # honest to put in its footer: "version 7, printed 26 Aug 2026". + # The working copy's version number. Bumped only when a save actually + # changes something, so it counts real edits rather than keystrokes, and a + # printed copy has something honest for its footer: "version 7 (published), + # printed 26 Aug 2026". revision = db.Column(db.Integer, nullable=False, default=1) @property @@ -138,8 +167,10 @@ class Guide(BaseModel): 'tags': self.taglist(), 'status': self.status, 'revision': self.revision, - 'createdat': self.createdat.isoformat() + 'Z' if self.createdat else None, - 'updatedat': self.updatedat.isoformat() + 'Z' if self.updatedat else None, + # API names stay createdat/updatedat: the column names are a + # storage detail, and renaming a payload field breaks callers. + 'createdat': self.createddate.isoformat() + 'Z' if self.createddate else None, + 'updatedat': self.modifieddate.isoformat() + 'Z' if self.modifieddate else None, 'publishedat': (self.publishedat.isoformat() + 'Z' if self.publishedat else None), } @@ -148,6 +179,76 @@ class Guide(BaseModel): return data +class GuideRevision(BaseModel): + """An immutable snapshot of a guide, written when a save changes something. + + Full snapshots rather than diffs. A guide is small, and a snapshot answers + the question people actually ask - "what did this say in March, when the + job last ran" - without replaying a chain of deltas to find out. The same + reasoning the part-marker backups follow, for the same reason: the version + you need is the one nobody thought to keep. + + DEDUP: a save whose content hashes identically to the latest revision writes + nothing. Without that, autosave produces a hundred identical versions a day + and the history becomes noise that hides the three edits that mattered. + + Never updated after insert. A revision that can be edited is not a record of + anything. + """ + + __tablename__ = 'guiderevisions' + + guiderevisionid = db.Column(db.Integer, primary_key=True) + guideid = db.Column(db.Integer, + db.ForeignKey('guides.guideid', ondelete='CASCADE'), + nullable=False, index=True) + + # Monotonic per guide, and shown to people: "version 7". + revision = db.Column(db.Integer, nullable=False) + + # The whole guide as it stood, not just the blocks: a title or tag change is + # a change worth being able to see and roll back. + title = db.Column(db.String(300), nullable=False) + summary = db.Column(db.String(1000)) + tags = db.Column(db.String(500)) + blocksjson = db.Column(db.Text(length=16777215), nullable=False) + + # sha256 of the canonical content, and the dedup key. + contenthash = db.Column(db.String(64), nullable=False, index=True) + + # Optional "what changed and why", the way a commit message earns its keep. + note = db.Column(db.String(500)) + + createdby = db.Column(db.Integer, db.ForeignKey('users.userid')) + + @property + def blocks(self): + if not self.blocksjson: + return [] + try: + data = json.loads(self.blocksjson) + except (ValueError, TypeError): + return [] + return data if isinstance(data, list) else [] + + def to_dict(self, includeblocks=False): + data = { + 'guiderevisionid': self.guiderevisionid, + 'guideid': self.guideid, + 'revision': self.revision, + 'title': self.title, + 'summary': self.summary, + 'tags': [t.strip() for t in (self.tags or '').split(',') if t.strip()], + 'note': self.note, + 'createdby': self.createdby, + 'createdat': self.createddate.isoformat() + 'Z' if self.createddate else None, + 'shorthash': (self.contenthash or '')[:12], + } + if includeblocks: + data['blocks'] = self.blocks + return data + + class GuideImage(BaseModel): """An image pasted or dropped into a guide. @@ -174,7 +275,6 @@ class GuideImage(BaseModel): data = db.Column(db.LargeBinary(length=(2 ** 24) - 1), nullable=False) uploadedby = db.Column(db.Integer, db.ForeignKey('users.userid')) - createdat = db.Column(db.DateTime, nullable=False, default=_utcnow) def to_dict(self): return { @@ -184,5 +284,5 @@ class GuideImage(BaseModel): 'mimetype': self.mimetype, 'bytesize': self.bytesize, 'url': '/api/guides/images/{}'.format(self.guideimageid), - 'createdat': self.createdat.isoformat() + 'Z' if self.createdat else None, + 'createdat': self.createddate.isoformat() + 'Z' if self.createddate else None, } diff --git a/plugins/guides/plugin.py b/plugins/guides/plugin.py new file mode 100644 index 00000000..c8b0e387 --- /dev/null +++ b/plugins/guides/plugin.py @@ -0,0 +1,92 @@ +"""Guides plugin. + +Step-by-step procedures written and read inside ShopDB: images, working +checklists, dark mode and printable output, with drafts and revision history. + +Distinct from the knowledgebase plugin, which is a LINK REGISTRY - a +description, a URL and keywords pointing at a document held somewhere else. +This holds the document. They share a tag vocabulary on purpose, so one search +can cover both. +""" + +import json +import logging +from pathlib import Path +from typing import List, Dict, Optional, Type + +from flask import Flask, Blueprint + +from shopdb.plugins.base import BasePlugin, PluginMeta + +from .models import Guide, GuideRevision, GuideImage +from .api import guides_bp + +logger = logging.getLogger(__name__) + + +class GuidesPlugin(BasePlugin): + + def __init__(self): + self._manifest = self._load_manifest() + + def _load_manifest(self) -> Dict: + manifest_path = Path(__file__).parent / 'manifest.json' + if manifest_path.exists(): + with open(manifest_path, 'r') as fh: + return json.load(fh) + return {} + + @property + def meta(self) -> PluginMeta: + return PluginMeta( + name=self._manifest.get('name', 'guides'), + version=self._manifest.get('version', '1.0.0'), + description=self._manifest.get('description', 'Step-by-step guides'), + author=self._manifest.get('author', 'ShopDB Team'), + dependencies=self._manifest.get('dependencies', []), + core_version=self._manifest.get('core_version', '>=0.1.0,<1.0.0'), + api_prefix=self._manifest.get('api_prefix', '/api/guides'), + ) + + def get_blueprint(self) -> Optional[Blueprint]: + return guides_bp + + def get_models(self) -> List[Type]: + return [Guide, GuideRevision, GuideImage] + + def get_permissions(self) -> List: + """Five permissions, and the split between the last two is the point. + + publish is separate from edit so a technician can write up a procedure + while somebody else decides it is fit for the floor. Without that split, + anyone who can fix a typo can also put an unreviewed procedure in front + of an operator. + + create without edit is a working combination on purpose: an author may + edit their OWN guide while it is unpublished, and needs guides.edit once + other people are following it (see _mayedit in the API). + """ + return [ + ('guides.view', 'View guides', 'guides'), + ('guides.create', 'Create guides and edit your own drafts', 'guides'), + ('guides.edit', 'Edit any guide, including published ones', 'guides'), + ('guides.publish', 'Publish or withdraw a guide', 'guides'), + ('guides.delete', 'Delete guides', 'guides'), + ] + + def get_navigation_items(self) -> List[Dict]: + # Position 8, beside Slides (7), rather than down in the Information + # section with the reference lists. A guide is something you go and DO + # something with, like a slide deck you put on a screen - not a + # catalogue you look things up in. + return [ + { + 'name': 'Guides', + 'icon': 'book-open', + 'route': '/guides', + 'position': 8, + }, + ] + + def init_app(self, app: Flask, db_instance) -> None: + logger.info('Guides plugin initialized (v%s)', self.meta.version) diff --git a/plugins/guides/services/authoring.py b/plugins/guides/services/authoring.py new file mode 100644 index 00000000..1afd4389 --- /dev/null +++ b/plugins/guides/services/authoring.py @@ -0,0 +1,133 @@ +"""Saving, revisioning and publishing a guide. + +Kept out of the API layer so the rules are testable without a request, and so +there is exactly ONE path that writes a guide. A second write path is how a +system ends up with revisions that skipped the dedup, or a published pointer +that points at nothing. +""" + +from datetime import datetime + +from shopdb.api import db + +from ..models import Guide, GuideRevision +from .blocks import validate, contenthash + + +def _utcnow(): + return datetime.utcnow() + + +def save(guide, title=None, summary=None, tags=None, blocks=None, note=None, + userid=None): + """Apply an edit to the working copy, writing a revision if it changed. + + Returns (guide, revision_or_None). None means the save was a no-op: the + content hashed identically to the last revision, so nothing was recorded. + That is the normal outcome of an autosave where the author changed nothing, + and it is what keeps the history readable. + """ + if title is not None: + guide.title = title.strip() + if summary is not None: + guide.summary = summary.strip() or None + if tags is not None: + # Normalised once, here: trimmed, empties dropped, order preserved. + # Tags are compared case-insensitively when hashing, but stored as the + # author typed them - "PXE" should not become "pxe" on screen. + guide.tags = ', '.join(t.strip() for t in tags.split(',') if t.strip()) or None + if blocks is not None: + guide.blocks = validate(blocks) + + incoming = contenthash(guide.title, guide.summary, guide.tags, guide.blocks) + + latest = (GuideRevision.query + .filter_by(guideid=guide.guideid) + .order_by(GuideRevision.revision.desc()) + .first()) + if latest is not None and latest.contenthash == incoming: + # Nothing a reader would notice has changed. Touch nothing else: an + # updatedat that moves on every autosave makes "last edited" a lie. + return guide, None + + guide.revision = (latest.revision + 1) if latest else 1 + guide.updatedby = userid + + revision = GuideRevision( + guideid=guide.guideid, + revision=guide.revision, + title=guide.title, + summary=guide.summary, + tags=guide.tags, + blocksjson=guide.blocksjson, + contenthash=incoming, + note=(note or '').strip() or None, + createdby=userid, + ) + db.session.add(revision) + db.session.flush() + return guide, revision + + +def publish(guide, userid=None): + """Point readers at the current working copy. + + Publishing does NOT snapshot: it promotes the latest existing revision. If + it snapshotted, a publish with unsaved edits would create a version nobody + reviewed - which is exactly the accident this workflow exists to prevent. + """ + latest = (GuideRevision.query + .filter_by(guideid=guide.guideid) + .order_by(GuideRevision.revision.desc()) + .first()) + if latest is None: + raise ValueError('nothing to publish: this guide has no saved content') + + guide.publishedrevisionid = latest.guiderevisionid + guide.status = 'published' + guide.publishedat = _utcnow() + guide.updatedby = userid + db.session.flush() + return latest + + +def unpublish(guide, userid=None): + """Withdraw a guide from readers without deleting anything. + + For a procedure that turns out to be wrong. The revisions stay, so it can be + corrected and published again, and so there is still a record of what people + were being told in the meantime. + """ + guide.publishedrevisionid = None + guide.status = 'draft' + guide.updatedby = userid + db.session.flush() + return guide + + +def readable(guide): + """The revision a READER should be served, or None. + + Never the working copy: a published guide shows what was published, even + while somebody is editing the next version. Editors get the working copy + through the editor, which is a different route on purpose. + """ + if not guide.publishedrevisionid: + return None + return db.session.get(GuideRevision, guide.publishedrevisionid) + + +def restore(guide, revision, userid=None): + """Copy an old revision back onto the working copy. + + Deliberately NOT a rollback of history: restoring writes a NEW revision with + the old content. The record of what was published between then and now stays + intact, which is the difference between a history and a rewrite. + """ + return save(guide, + title=revision.title, + summary=revision.summary or '', + tags=revision.tags or '', + blocks=revision.blocks, + note='Restored from version {}'.format(revision.revision), + userid=userid) diff --git a/plugins/guides/services/blocks.py b/plugins/guides/services/blocks.py index ef07608b..08f06c0c 100644 --- a/plugins/guides/services/blocks.py +++ b/plugins/guides/services/blocks.py @@ -17,6 +17,7 @@ Storing text verbatim keeps what the author typed recoverable even if the rendering rules change later. """ +import hashlib import json from ..models import BLOCKTYPES @@ -129,3 +130,23 @@ def checklistitemcount(blocks): if isinstance(block, dict) and block.get('type') == 'checklist': total += len(block.get('items') or []) return total + + +def contenthash(title, summary, tags, blocks): + """sha256 over everything a reader would notice changing. + + Title, summary and tags are included, not just the blocks: renaming a guide + or retagging it is a change worth a revision, and hashing only the body + would silently drop it from the history. + + Canonical by construction - the blocks are re-serialized with sorted keys + and no whitespace, so the same content saved twice hashes the same however + the editor happened to order its JSON that day. + """ + canonical = json.dumps({ + 'title': (title or '').strip(), + 'summary': (summary or '').strip(), + 'tags': sorted(t.strip().lower() for t in (tags or '').split(',') if t.strip()), + 'blocks': blocks or [], + }, sort_keys=True, separators=(',', ':')) + return hashlib.sha256(canonical.encode('utf-8')).hexdigest() 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/shopdb/core/api/settings.py b/shopdb/core/api/settings.py index 6a17a338..c38d7bdb 100644 --- a/shopdb/core/api/settings.py +++ b/shopdb/core/api/settings.py @@ -101,6 +101,7 @@ IDENTIFIER_ASSETTYPES = ['machine', 'computer', 'printer', 'network_device', SEARCH_DOMAINS = { 'application': 'Applications', 'knowledgebase': 'Knowledge Base', + 'guide': 'Guides', 'employee': 'Employees', 'machine': 'Machines', 'computer': 'PCs', diff --git a/shopdb/plugins/alembic_template.py b/shopdb/plugins/alembic_template.py index 70e272fe..07de998b 100644 --- a/shopdb/plugins/alembic_template.py +++ b/shopdb/plugins/alembic_template.py @@ -58,6 +58,7 @@ PLUGIN_TABLE_OWNERS: dict[str, Iterable[str]] = { 'manifestblobs', 'manifestenforcementreports', 'manifestenforcementresults', 'pctypealiases'), + 'guides': ('guides', 'guiderevisions', 'guideimages'), 'knowledgebase': ('knowledgebase',), 'machines': ('machinetypes', 'machines'), 'measuringtools': ('measuringtooltypes', 'measuringtools'), 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') == [] diff --git a/tests/test_plugin_migrations.py b/tests/test_plugin_migrations.py index e4ef3976..5c58e74c 100644 --- a/tests/test_plugin_migrations.py +++ b/tests/test_plugin_migrations.py @@ -50,6 +50,9 @@ EXPECTED_HEAD_REVISION['measuringtools'] = 'measuringtools0001baseline' # 0002 adds lastseenat - when a config was last CONFIRMED unchanged, which dedup # otherwise throws away. EXPECTED_HEAD_REVISION['backups'] = 'backups0003clearlastseen' +# guides is a NEW plugin, so its 0001 builds tables rather than stamping an +# anchor over core-owned ones. +EXPECTED_HEAD_REVISION['guides'] = 'guides0001baseline' # geenforce adds the content-addressed blob store (manifestblobs) on top of its # baseline. EXPECTED_HEAD_REVISION['geenforce'] = 'geenforce0004minlib' diff --git a/tests/test_plugins/test_guides.py b/tests/test_plugins/test_guides.py new file mode 100644 index 00000000..5c78bf92 --- /dev/null +++ b/tests/test_plugins/test_guides.py @@ -0,0 +1,334 @@ +"""Guides: block validation, revisioning, and the draft/publish split. + +The behaviour worth protecting hardest is the last one. A technician following a +procedure on a shop floor must not have it change under them because somebody is +editing it upstairs, and the only thing standing between them and that is the +published pointer. +""" + +import pytest + +from shopdb import create_app +from shopdb.extensions import db as _db +from shopdb.plugins import plugin_manager + + +# --------------------------------------------------------------------------- +# Block validation - pure, no app context needed. These rules exist to stop a +# BAD SAVE becoming a guide that will not render weeks later, when the author +# has moved on and the only evidence is a blank page. +# --------------------------------------------------------------------------- + +def _block(blockid='b1', blocktype='paragraph', **extra): + return dict({'id': blockid, 'type': blocktype}, **extra) + + +def test_a_well_formed_block_list_passes(): + from plugins.guides.services.blocks import validate + blocks = [ + _block('b1', 'heading', text='Before you start'), + _block('b2', 'paragraph', text='Check the **cable** first.'), + _block('b3', 'checklist', items=[{'id': 'i1', 'text': 'Power on'}]), + ] + assert validate(blocks) == blocks + + +def test_a_block_without_an_id_is_refused(): + """Ids are what make a tick rememberable. + + Without one the checkbox silently loses its state on the next visit, which + reads to a user as "the checkboxes do not work" - a bug report with no + stack trace and no obvious cause. + """ + from plugins.guides.services.blocks import validate, BlockError + with pytest.raises(BlockError) as exc: + validate([{'type': 'paragraph', 'text': 'no id'}]) + assert 'stable id' in str(exc.value) + + +def test_duplicate_ids_are_refused(): + from plugins.guides.services.blocks import validate, BlockError + with pytest.raises(BlockError) as exc: + validate([_block('same'), _block('same')]) + assert 'duplicate' in str(exc.value) + + +def test_an_unknown_block_type_is_refused_and_lists_the_known_ones(): + from plugins.guides.services.blocks import validate, BlockError + with pytest.raises(BlockError) as exc: + validate([_block('b1', 'carousel')]) + message = str(exc.value) + assert 'carousel' in message and 'checklist' in message + + +def test_checklist_items_need_their_own_ids(): + """Same reason as blocks: the item id is what a tick is stored against.""" + from plugins.guides.services.blocks import validate, BlockError + with pytest.raises(BlockError) as exc: + validate([_block('b1', 'checklist', items=[{'text': 'no id'}])]) + assert 'item' in str(exc.value) + + +def test_the_error_names_which_block_is_wrong(): + """An author fixing a rejected save should not have to count blocks.""" + from plugins.guides.services.blocks import validate, BlockError + with pytest.raises(BlockError) as exc: + validate([_block('b1'), _block('b2'), {'type': 'paragraph'}]) + assert str(exc.value).startswith('block 3:') + + +def test_content_hash_covers_title_and_tags_not_just_blocks(): + """Renaming or retagging a guide is a change a reader notices. + + Hashing only the body would drop those edits out of the history entirely. + """ + from plugins.guides.services.blocks import contenthash + blocks = [_block('b1', 'paragraph', text='same body')] + base = contenthash('Title', 'Summary', 'pxe, share', blocks) + assert contenthash('Different', 'Summary', 'pxe, share', blocks) != base + assert contenthash('Title', 'Summary', 'pxe', blocks) != base + assert contenthash('Title', 'Different', 'pxe, share', blocks) != base + + +def test_content_hash_ignores_tag_order_case_and_spacing(): + """Otherwise reordering tags in the editor would manufacture a version.""" + from plugins.guides.services.blocks import contenthash + blocks = [_block()] + assert (contenthash('T', '', 'PXE, Share', blocks) + == contenthash('T', '', 'share ,pxe', blocks)) + + +def test_checklist_item_count_counts_only_checklists(): + from plugins.guides.services.blocks import checklistitemcount + blocks = [ + _block('b1', 'checklist', items=[{'id': 'i1'}, {'id': 'i2'}]), + _block('b2', 'bullets', items=['not tickable']), + _block('b3', 'checklist', items=[{'id': 'i3'}]), + ] + assert checklistitemcount(blocks) == 3 + + +def test_plaintext_flattens_a_guide_for_search(): + """People search for something written halfway down, not for the title.""" + from plugins.guides.services.blocks import plaintext + blocks = [ + _block('b1', 'heading', text='Mapped drive'), + _block('b2', 'checklist', items=[{'id': 'i1', 'text': 'Run the sync'}]), + _block('b3', 'table', rows=[['left', 'right']]), + ] + flat = plaintext(blocks) + assert 'Mapped drive' in flat and 'Run the sync' in flat and 'right' in flat + + +# --------------------------------------------------------------------------- +# Authoring: revisions, dedup and publishing. Needs an app context. +# --------------------------------------------------------------------------- + +@pytest.fixture(scope='module') +def gd_app(): + saved = (plugin_manager._app, plugin_manager._db, plugin_manager.registry, + plugin_manager.loader, plugin_manager.migration_manager, + plugin_manager._registered_prefixes) + application = create_app('testing') + + from plugins.guides.plugin import GuidesPlugin + plugin = GuidesPlugin() + pm = application.extensions['plugin_manager'] + if plugin.meta.api_prefix not in pm._registered_prefixes: + pm._register_plugin_components(plugin) + + with application.app_context(): + _db.create_all() + yield application + _db.session.remove() + _db.drop_all() + + (plugin_manager._app, plugin_manager._db, plugin_manager.registry, + plugin_manager.loader, plugin_manager.migration_manager, + plugin_manager._registered_prefixes) = saved + + +@pytest.fixture +def gd_clean(gd_app): + from plugins.guides.models import Guide, GuideRevision + with gd_app.app_context(): + _db.session.query(GuideRevision).delete() + _db.session.query(Guide).delete() + _db.session.commit() + yield + + +def _newguide(title='Replacing a marker controller'): + from plugins.guides.models import Guide + guide = Guide(title=title, slug=title.lower().replace(' ', '-'), + status='draft') + _db.session.add(guide) + _db.session.flush() + return guide + + +def test_the_first_save_creates_version_one(gd_app, gd_clean): + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + guide, revision = authoring.save( + guide, title='Replacing a marker controller', + blocks=[_block('b1', 'paragraph', text='Power it off first.')]) + _db.session.commit() + assert revision is not None + assert revision.revision == 1 + assert guide.revision == 1 + + +def test_saving_identical_content_writes_no_revision(gd_app, gd_clean): + """Autosave would otherwise produce a hundred versions a day. + + A history full of identical entries hides the three edits that mattered, + which is the same failure the backups plugin dedups away. + """ + from plugins.guides.models import GuideRevision + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + blocks = [_block('b1', 'paragraph', text='Unchanged.')] + authoring.save(guide, title='T', blocks=blocks) + _db.session.commit() + + guide, second = authoring.save(guide, title='T', blocks=blocks) + _db.session.commit() + + assert second is None, 'an unchanged save must not manufacture a version' + assert GuideRevision.query.filter_by(guideid=guide.guideid).count() == 1 + assert guide.revision == 1 + + +def test_a_real_edit_creates_the_next_version(gd_app, gd_clean): + from plugins.guides.models import GuideRevision + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + authoring.save(guide, title='T', + blocks=[_block('b1', 'paragraph', text='One.')]) + _db.session.commit() + guide, second = authoring.save( + guide, blocks=[_block('b1', 'paragraph', text='Two.')]) + _db.session.commit() + + assert second.revision == 2 + assert GuideRevision.query.filter_by(guideid=guide.guideid).count() == 2 + + +def test_readers_see_nothing_until_it_is_published(gd_app, gd_clean): + """A half-written procedure on a shop floor is worse than no procedure.""" + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + authoring.save(guide, title='T', blocks=[_block()]) + _db.session.commit() + assert authoring.readable(guide) is None + + +def test_editing_a_published_guide_does_not_change_what_readers_see(gd_app, gd_clean): + """THE point of the draft/publish split. + + A technician halfway through a procedure keeps reading the version that was + published, even while the next one is being written. + """ + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + authoring.save(guide, title='T', + blocks=[_block('b1', 'paragraph', text='Published text.')]) + authoring.publish(guide) + _db.session.commit() + + authoring.save(guide, + blocks=[_block('b1', 'paragraph', text='Work in progress.')]) + _db.session.commit() + + served = authoring.readable(guide) + assert served.blocks[0]['text'] == 'Published text.' + assert guide.revision == 2, 'the working copy moved on' + assert served.revision == 1, 'what readers get did not' + + +def test_publishing_promotes_the_latest_saved_revision(gd_app, gd_clean): + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + authoring.save(guide, title='T', blocks=[_block('b1', 'paragraph', text='v1')]) + authoring.save(guide, blocks=[_block('b1', 'paragraph', text='v2')]) + authoring.publish(guide) + _db.session.commit() + assert authoring.readable(guide).blocks[0]['text'] == 'v2' + assert guide.status == 'published' + + +def test_publishing_an_empty_guide_is_refused(gd_app, gd_clean): + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + _db.session.commit() + with pytest.raises(ValueError): + authoring.publish(guide) + + +def test_unpublishing_withdraws_without_destroying_the_history(gd_app, gd_clean): + """For a procedure that turns out to be wrong. + + The versions stay, so it can be corrected and published again, and so there + is still a record of what people were being told in the meantime. + """ + from plugins.guides.models import GuideRevision + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + authoring.save(guide, title='T', blocks=[_block()]) + authoring.publish(guide) + _db.session.commit() + + authoring.unpublish(guide) + _db.session.commit() + + assert authoring.readable(guide) is None + assert guide.status == 'draft' + assert GuideRevision.query.filter_by(guideid=guide.guideid).count() == 1 + + +def test_restoring_writes_a_new_version_rather_than_rewriting_history(gd_app, gd_clean): + """A history you can edit is not a record of anything. + + Restoring brings old content forward as a NEW version, so what was + published in between stays visible. + """ + from plugins.guides.models import GuideRevision + from plugins.guides.services import authoring + with gd_app.app_context(): + guide = _newguide() + _, first = authoring.save(guide, title='T', + blocks=[_block('b1', 'paragraph', text='original')]) + authoring.save(guide, blocks=[_block('b1', 'paragraph', text='mistake')]) + _db.session.commit() + + authoring.restore(guide, first) + _db.session.commit() + + rows = (GuideRevision.query.filter_by(guideid=guide.guideid) + .order_by(GuideRevision.revision).all()) + assert [r.revision for r in rows] == [1, 2, 3] + assert rows[2].blocks[0]['text'] == 'original' + assert 'Restored from version 1' in (rows[2].note or '') + assert rows[1].blocks[0]['text'] == 'mistake', 'the mistake stays on record' + + +def test_a_corrupt_blocks_column_renders_as_empty_rather_than_erroring(gd_app, gd_clean): + """A guide that will not render is a page a person cannot read. + + An empty guide with its title and tags intact is a better failure than an + error page, and the corruption is still visible - the editor shows nothing. + """ + with gd_app.app_context(): + guide = _newguide() + guide.blocksjson = 'not json at all' + _db.session.commit() + assert guide.blocks == [] From 4e38303326fe0b004ed6b434f886837acbf4da94 Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Wed, 26 Aug 2026 13:31:13 -0400 Subject: [PATCH 2/3] Tab titles name the record; Guides reach a site and the contract surface --- CHANGELOG.md | 18 + deploy/site-profile-universal.json | 1 + frontend/src/composables/pageTitle.js | 91 ++++ frontend/src/composables/pageTitle.spec.js | 177 ++++++++ frontend/src/router/index.js | 16 +- .../frontend/views/ApplicationDetail.vue | 2 + plugins/computers/frontend/routes.js | 2 +- plugins/computers/frontend/views/PCDetail.vue | 2 + .../frontend/views/EmployeeDetail.vue | 2 + plugins/guides/frontend/views/GuideView.vue | 2 + plugins/guides/models/guide.py | 3 +- .../frontend/views/KnowledgeBaseDetail.vue | 426 +++++++++--------- .../machines/frontend/views/MachineDetail.vue | 2 + plugins/measuringtools/frontend/routes.js | 2 +- .../frontend/views/MeasuringToolDetail.vue | 2 + .../frontend/views/NetworkDeviceDetail.vue | 2 + .../network/frontend/views/SubnetDetail.vue | 2 + .../frontend/views/PrintedItemDetail.vue | 2 + plugins/printers/frontend/routes.js | 4 +- .../printers/frontend/views/PrinterDetail.vue | 4 + plugins/usb/frontend/views/USBDetail.vue | 2 + plugins/warranty/frontend/routes.js | 2 +- 22 files changed, 538 insertions(+), 228 deletions(-) create mode 100644 frontend/src/composables/pageTitle.js create mode 100644 frontend/src/composables/pageTitle.spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d46109c..8cc81df7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,24 @@ ADR-007 and ADR-002. ### Fixed +- **Every detail page had the same browser tab title.** A machine, a PC and a + printer all read "West Jefferson ShopDB - Machine" or the equivalent, so two + tabs on two different records were indistinguishable, a bookmark said nothing + about what it pointed at, and a link shared into Teams previewed as the + section rather than the thing. Detail pages now name the record: machines, + PCs, printers, network devices, subnets, measuring tools, USB devices, + printed parts, applications, employees, knowledge base articles and guides. + A record with no usable name, or one that failed to load, keeps the section + name, and a long name is cut at a word boundary. Report pages gained proper + titles too, which also fixes "Pc Relationships". + +- **Guides would have been dropped at provisioning.** The plugin was missing + from `deploy/site-profile-universal.json`, the profile the released Windows + installer is built from, so the wizard would not have offered it and + `flask plugin prune-schema` drops the tables of any plugin absent from the + profile (ADR-014). Its model also imported core directly rather than through + `shopdb.api`, which the contract test catches. + - **The shipped catalog offered two ways to spell the same vendor, and one row that should never have left a test.** `HP Inc.` and `Zebra Technologies` were empty rows duplicating `HP` and `Zebra`, which carry every model between diff --git a/deploy/site-profile-universal.json b/deploy/site-profile-universal.json index 64b7539e..7308f77d 100644 --- a/deploy/site-profile-universal.json +++ b/deploy/site-profile-universal.json @@ -5,6 +5,7 @@ "computers", "employees", "geenforce", + "guides", "knowledgebase", "machines", "measuringtools", diff --git a/frontend/src/composables/pageTitle.js b/frontend/src/composables/pageTitle.js new file mode 100644 index 00000000..b466eb6f --- /dev/null +++ b/frontend/src/composables/pageTitle.js @@ -0,0 +1,91 @@ +// The browser tab title, including WHICH record is open. +// +// The router can only name the KIND of page, because it runs before the view +// has fetched anything: every application read "West Jefferson ShopDB - +// Application", so two tabs on two different machines were indistinguishable, +// and a shared link previewed as the section rather than the thing. +// +// So a detail view names itself once its record arrives: +// +// setRecordTitle(machine.assetnumber) +// +// THE ORDERING IS THE WHOLE PROBLEM. The router's afterEach awaits the site +// settings before it can build " ShopDB - ", so on a cold load +// it finishes AFTER a view that fetched quickly has already set a good title, +// and would overwrite it with the generic one. Both writers therefore go +// through here, and the generic write is dropped when a record title already +// belongs to the route being landed on. The key is the route's fullPath, so +// navigating away invalidates the previous record's claim. + +import { getFacilityName } from '@/utils/siteSettings' + +let activeKey = null // fullPath of the route the tab is currently showing +let recordTitle = null // record name set by a view for activeKey, if any +let sitePrefix = 'ShopDB' // ' ShopDB', resolved once and cached + +function compose(page) { + return page && page !== 'Home' ? `${sitePrefix} - ${page}` : sitePrefix +} + +async function resolveSitePrefix() { + try { + const facility = await getFacilityName() + if (facility && facility !== 'ShopDB') sitePrefix = `${facility} ShopDB` + } catch (settingsError) { + // settings unavailable: plain ShopDB is a fine tab title + } + return sitePrefix +} + +// Called by the router on every navigation, BEFORE it awaits anything, so a +// view that resolves first is not treated as belonging to the previous route. +export function beginNavigation(key) { + activeKey = key + recordTitle = null +} + +// Called by the router once it knows the site prefix. Skipped when a view has +// already named the record for this route. +export async function applyGenericTitle(key, page) { + await resolveSitePrefix() + if (key !== activeKey) return // navigated on while we awaited + // A view that named the record before the facility name arrived composed its + // title against the placeholder prefix, so rewrite it rather than leaving + // 'ShopDB - CSF16' on a site called West Jefferson. + document.title = compose(recordTitle !== null ? recordTitle : page) +} + +// A tab shows very little, and the browser truncates mid-word wherever it runs +// out. Cut at a word boundary instead, so what survives is readable. Knowledge +// base articles are the case that forces this: their name is a whole sentence +// of description, not a short label. +const MAXLENGTH = 60 + +function shorten(text) { + if (text.length <= MAXLENGTH) return text + const cut = text.slice(0, MAXLENGTH) + const space = cut.lastIndexOf(' ') + return `${(space > MAXLENGTH / 2 ? cut.slice(0, space) : cut).trimEnd()}...` +} + +// Called by a detail view once its record has loaded. An empty or missing name +// leaves the generic title in place rather than showing a bare id: "Machine" +// beats "Machine 4192" for somebody scanning a row of tabs, and a record that +// failed to load has no name to show anyway. +export function setRecordTitle(name) { + const text = (name === null || name === undefined) ? '' : String(name).trim() + if (!text) return + recordTitle = shorten(text) + document.title = compose(recordTitle) +} + +// Exposed for tests. +export function _state() { + return { activeKey, recordTitle, sitePrefix } +} + +export function _reset() { + activeKey = null + recordTitle = null + sitePrefix = 'ShopDB' +} diff --git a/frontend/src/composables/pageTitle.spec.js b/frontend/src/composables/pageTitle.spec.js new file mode 100644 index 00000000..ae7aab07 --- /dev/null +++ b/frontend/src/composables/pageTitle.spec.js @@ -0,0 +1,177 @@ +// The tab title names the record, and the two writers do not fight. +// +// The router can only name the KIND of page - it runs before the view has +// fetched anything - so a detail view names the record itself once it loads. +// That leaves two writers for one string, and the router's is asynchronous +// (it awaits the site settings), so on a cold load it can finish LAST and +// overwrite a good title with the generic one. These pin that ordering. + +import { describe, it, expect, beforeEach, vi } from 'vitest' + +// The real getFacilityName caches, so only the first call is slow and every +// later one is instant. Mirror that: pending calls are held until the test +// releases them, and the value is served synchronously from then on. A mock +// that hands out a fresh unresolved promise per call hangs the moment a test +// navigates twice. +let pending = [] +let settled = null + +function resolveFacility(value) { + settled = value + const waiting = pending + pending = [] + waiting.forEach((resolve) => resolve(value)) +} + +vi.mock('@/utils/siteSettings', () => ({ + getFacilityName: () => (settled !== null + ? Promise.resolve(settled) + : new Promise((resolve) => { pending.push(resolve) })), +})) + +import { + beginNavigation, applyGenericTitle, setRecordTitle, _reset, +} from './pageTitle' + +beforeEach(() => { + _reset() + document.title = '' + pending = [] + settled = null +}) + +describe('the generic title', () => { + it('names the section when no record has been set', async () => { + beginNavigation('/machines') + const done = applyGenericTitle('/machines', 'Machines') + resolveFacility('West Jefferson') + await done + + expect(document.title).toBe('West Jefferson ShopDB - Machines') + }) + + it('falls back to plain ShopDB when the facility is unset', async () => { + beginNavigation('/machines') + const done = applyGenericTitle('/machines', 'Machines') + resolveFacility('ShopDB') + await done + + expect(document.title).toBe('ShopDB - Machines') + }) + + it('drops the site name entirely on the dashboard', async () => { + beginNavigation('/') + const done = applyGenericTitle('/', 'Home') + resolveFacility('West Jefferson') + await done + + expect(document.title).toBe('West Jefferson ShopDB') + }) +}) + +describe('a record title', () => { + it('names the record once the view has loaded it', async () => { + beginNavigation('/machines/12') + const done = applyGenericTitle('/machines/12', 'Machines') + resolveFacility('West Jefferson') + await done + + setRecordTitle('CSF16') + + expect(document.title).toBe('West Jefferson ShopDB - CSF16') + }) + + it('SURVIVES a slow router that resolves afterwards', async () => { + // The regression this file exists for. The view wins the race, the + // settings fetch lands second, and the generic write must stand down. + beginNavigation('/machines/12') + const done = applyGenericTitle('/machines/12', 'Machines') + + setRecordTitle('CSF16') // view got there first + resolveFacility('West Jefferson') + await done // router finishes last + + expect(document.title).toBe('West Jefferson ShopDB - CSF16') + }) + + it('is ignored when the record has no usable name', async () => { + beginNavigation('/machines/12') + const done = applyGenericTitle('/machines/12', 'Machines') + resolveFacility('West Jefferson') + await done + + setRecordTitle('') + setRecordTitle(null) + setRecordTitle(undefined) + setRecordTitle(' ') + + // the section name is a better tab than a blank or a bare id + expect(document.title).toBe('West Jefferson ShopDB - Machines') + }) + + it('shortens a name too long for a tab, at a word boundary', async () => { + // Knowledge base articles are named with a sentence of description, and a + // browser truncates those mid-word wherever it happens to run out. + beginNavigation('/knowledgebase/2') + const done = applyGenericTitle('/knowledgebase/2', 'Knowledge Base') + resolveFacility('West Jefferson') + await done + + const full = 'Documentation on how to image a Standard or Business PC ' + + 'in GCC High using MediaCreator Lite' + setRecordTitle(full) + + expect(document.title.length).toBeLessThan(90) + expect(document.title.startsWith('West Jefferson ShopDB - ')).toBe(true) + expect(document.title.endsWith('...')).toBe(true) + + // what survives is a whole-word prefix of the original: the character + // following the cut in the source is a space, so no word was split + const kept = document.title + .replace('West Jefferson ShopDB - ', '') + .replace(/\.\.\.$/, '') + expect(full.startsWith(kept)).toBe(true) + expect(full[kept.length]).toBe(' ') + }) + + it('leaves a name that already fits alone', async () => { + beginNavigation('/machines/14') + const done = applyGenericTitle('/machines/14', 'Machines') + resolveFacility('West Jefferson') + await done + + setRecordTitle('0600') + + expect(document.title).toBe('West Jefferson ShopDB - 0600') + }) + + it('does not leak onto the next page', async () => { + beginNavigation('/machines/12') + const first = applyGenericTitle('/machines/12', 'Machines') + resolveFacility('West Jefferson') + await first + setRecordTitle('CSF16') + + beginNavigation('/printers') + const second = applyGenericTitle('/printers', 'Printers') + await second + + expect(document.title).toBe('West Jefferson ShopDB - Printers') + }) + + it('is discarded when the user navigates away mid-flight', async () => { + // Two navigations overlap; the first one's late generic write must not + // land on the second page. + beginNavigation('/machines/12') + const stale = applyGenericTitle('/machines/12', 'Machines') + + beginNavigation('/printers/3') + const fresh = applyGenericTitle('/printers/3', 'Printers') + setRecordTitle('WJPRT04') + + resolveFacility('West Jefferson') + await Promise.all([stale, fresh]) + + expect(document.title).toBe('West Jefferson ShopDB - WJPRT04') + }) +}) diff --git a/frontend/src/router/index.js b/frontend/src/router/index.js index f01a802c..37562394 100644 --- a/frontend/src/router/index.js +++ b/frontend/src/router/index.js @@ -6,7 +6,7 @@ import { setupComplete, setupSkipped, isSetupLoaded, refreshSetupState, needsAdmin, isNeedsAdminLoaded, refreshNeedsAdmin } from '../composables/setupState' import { loadEnabledPlugins, isPluginEnabled } from '../composables/enabledPlugins' import { useToast } from '../composables/toast' -import { getFacilityName } from '../utils/siteSettings' +import { beginNavigation, applyGenericTitle } from '../composables/pageTitle' import { stagedChildren, stagedToplevel } from './routes.gen' // Auto-discover all route modules from routes/ directory. A module's default @@ -147,14 +147,12 @@ function pageTitleFor(route) { word.charAt(0).toUpperCase() + word.slice(1)).join(' ') } -router.afterEach(async (to) => { - let siteTitle = 'ShopDB' - try { - const facility = await getFacilityName() - if (facility && facility !== 'ShopDB') siteTitle = `${facility} ShopDB` - } catch (titleError) { /* settings unavailable - plain ShopDB */ } - const page = pageTitleFor(to) - document.title = page && page !== 'Home' ? `${siteTitle} - ${page}` : siteTitle +router.afterEach((to) => { + // Claim the route BEFORE awaiting anything: a detail view that resolves + // quickly sets the record title, and the generic write below must not + // overwrite it. See composables/pageTitle.js. + beginNavigation(to.fullPath) + applyGenericTitle(to.fullPath, pageTitleFor(to)) }) // A deploy replaces the hashed chunk files, so a tab that was open across it diff --git a/plugins/applications/frontend/views/ApplicationDetail.vue b/plugins/applications/frontend/views/ApplicationDetail.vue index 88fb5734..2f31b286 100644 --- a/plugins/applications/frontend/views/ApplicationDetail.vue +++ b/plugins/applications/frontend/views/ApplicationDetail.vue @@ -168,6 +168,7 @@ import { applicationsApi } from '@/api' import { getContactEmailDomain } from '@/utils/siteSettings' import { sanitizeNotesHtml } from '@/utils/sanitizeHtml' import { fileHref } from '@/utils/basePath' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() @@ -202,6 +203,7 @@ onMounted(async () => { // Load application details const response = await applicationsApi.get(route.params.id) app.value = response.data.data + setRecordTitle(app.value?.appname) // Load versions try { diff --git a/plugins/computers/frontend/routes.js b/plugins/computers/frontend/routes.js index cd724a09..828d9122 100644 --- a/plugins/computers/frontend/routes.js +++ b/plugins/computers/frontend/routes.js @@ -6,7 +6,7 @@ export default [ path: 'reports/pc-relationships', name: 'report-pc-relationships', component: () => import('./views/PCRelationshipsReport.vue'), - meta: { plugin: 'computers' } + meta: { plugin: 'computers', title: 'PC Relationships Report' } }, { path: 'settings/pctypemapping', diff --git a/plugins/computers/frontend/views/PCDetail.vue b/plugins/computers/frontend/views/PCDetail.vue index 633aa2e4..09f7a0ba 100644 --- a/plugins/computers/frontend/views/PCDetail.vue +++ b/plugins/computers/frontend/views/PCDetail.vue @@ -263,6 +263,7 @@ import CustomFieldsSection from '@/components/CustomFieldsSection.vue' import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import AssetRelationships from '@/components/AssetRelationships.vue' import { useIdentifierFlags } from '@/composables/identifierSettings' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() const { isEnabled } = useIdentifierFlags() @@ -280,6 +281,7 @@ onMounted(async () => { try { const response = await computersApi.get(route.params.id) computer.value = response.data.data + setRecordTitle(computer.value?.assetnumber || computer.value?.name) // Load installed applications try { diff --git a/plugins/employees/frontend/views/EmployeeDetail.vue b/plugins/employees/frontend/views/EmployeeDetail.vue index 56dc9674..c9ab54fc 100644 --- a/plugins/employees/frontend/views/EmployeeDetail.vue +++ b/plugins/employees/frontend/views/EmployeeDetail.vue @@ -153,6 +153,7 @@ import { withBase } from '@/utils/basePath' import { employeesApi, usbApi, notificationsApi } from '@/api' import { isPluginEnabled, loadEnabledPlugins } from '@/composables/enabledPlugins' import { useToast } from '@/composables/toast' +import { setRecordTitle } from '@/composables/pageTitle' const toast = useToast() const route = useRoute() @@ -223,6 +224,7 @@ async function loadEmployee() { try { const response = await employeesApi.lookup(route.params.sso) employee.value = response.data.data + setRecordTitle(employee.value?.name || employee.value?.sso) } catch (err) { console.error('Error loading employee:', err) error.value = 'Employee not found' diff --git a/plugins/guides/frontend/views/GuideView.vue b/plugins/guides/frontend/views/GuideView.vue index e0ff3686..8b32133a 100644 --- a/plugins/guides/frontend/views/GuideView.vue +++ b/plugins/guides/frontend/views/GuideView.vue @@ -59,6 +59,7 @@ import { useRoute } from 'vue-router' import api from '@/api' import { apiError } from '@/utils/apiError' import GuideBlocks from '../components/GuideBlocks.vue' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() const guide = ref(null) @@ -146,6 +147,7 @@ async function load () { try { const response = await api.get('/guides/' + route.params.slug) guide.value = response.data?.data || response.data + setRecordTitle(guide.value?.title) loadProgress() } catch (err) { error.value = apiError(err, 'Could not load this guide') diff --git a/plugins/guides/models/guide.py b/plugins/guides/models/guide.py index a3587798..809445b8 100644 --- a/plugins/guides/models/guide.py +++ b/plugins/guides/models/guide.py @@ -33,8 +33,7 @@ without an author thinking about it. import json from datetime import datetime -from shopdb.api import db -from shopdb.core.models.base import BaseModel +from shopdb.api import db, BaseModel def _utcnow(): diff --git a/plugins/knowledgebase/frontend/views/KnowledgeBaseDetail.vue b/plugins/knowledgebase/frontend/views/KnowledgeBaseDetail.vue index 2d2d8b85..25364a19 100644 --- a/plugins/knowledgebase/frontend/views/KnowledgeBaseDetail.vue +++ b/plugins/knowledgebase/frontend/views/KnowledgeBaseDetail.vue @@ -1,212 +1,214 @@ - - - - - + + + + + diff --git a/plugins/machines/frontend/views/MachineDetail.vue b/plugins/machines/frontend/views/MachineDetail.vue index 4e7810c0..464484c7 100644 --- a/plugins/machines/frontend/views/MachineDetail.vue +++ b/plugins/machines/frontend/views/MachineDetail.vue @@ -257,6 +257,7 @@ import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import AssetRelationships from '@/components/AssetRelationships.vue' import { useWarrantyBadge } from '@/composables/warrantyBadge' import { useIdentifierFlags } from '@/composables/identifierSettings' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() const { isEnabled } = useIdentifierFlags() @@ -271,6 +272,7 @@ onMounted(async () => { try { const response = await machinesApi.get(route.params.id) machine.value = response.data.data + setRecordTitle(machine.value?.assetnumber || machine.value?.name) } catch (error) { console.error('Error loading machine:', error) } finally { diff --git a/plugins/measuringtools/frontend/routes.js b/plugins/measuringtools/frontend/routes.js index 95d6094e..7e0fc3d5 100644 --- a/plugins/measuringtools/frontend/routes.js +++ b/plugins/measuringtools/frontend/routes.js @@ -43,7 +43,7 @@ export default [ path: 'reports/calibration', name: 'calibration-report', component: () => import('./views/CalibrationReport.vue'), - meta: { plugin: 'measuringtools' } + meta: { plugin: 'measuringtools', title: 'Calibration Report' } }, { path: 'settings/measuringtooltypes', diff --git a/plugins/measuringtools/frontend/views/MeasuringToolDetail.vue b/plugins/measuringtools/frontend/views/MeasuringToolDetail.vue index 5a9da8ea..d64a8b55 100644 --- a/plugins/measuringtools/frontend/views/MeasuringToolDetail.vue +++ b/plugins/measuringtools/frontend/views/MeasuringToolDetail.vue @@ -172,6 +172,7 @@ import CustomFieldsSection from '@/components/CustomFieldsSection.vue' import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import AssetRelationships from '@/components/AssetRelationships.vue' import { useIdentifierFlags } from '@/composables/identifierSettings' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() @@ -193,6 +194,7 @@ onMounted(async () => { ? await measuringtoolsApi.getByAsset(route.params.assetid) : await measuringtoolsApi.get(route.params.id) tool.value = response.data.data + setRecordTitle(tool.value?.assetnumber || tool.value?.name) } catch (err) { console.error('Error loading measuring tool:', err) } finally { diff --git a/plugins/network/frontend/views/NetworkDeviceDetail.vue b/plugins/network/frontend/views/NetworkDeviceDetail.vue index 6921b403..f947549d 100644 --- a/plugins/network/frontend/views/NetworkDeviceDetail.vue +++ b/plugins/network/frontend/views/NetworkDeviceDetail.vue @@ -228,6 +228,7 @@ import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import { useWarrantyBadge } from '@/composables/warrantyBadge' import { useIdentifierFlags } from '@/composables/identifierSettings' import { useToast } from '@/composables/toast' +import { setRecordTitle } from '@/composables/pageTitle' const toast = useToast() const { isEnabled } = useIdentifierFlags() @@ -250,6 +251,7 @@ async function loadDevice() { try { const response = await networkApi.get(deviceId) device.value = response.data.data + setRecordTitle(device.value?.assetnumber || device.value?.name) } catch (error) { console.error('Error loading device:', error) device.value = null diff --git a/plugins/network/frontend/views/SubnetDetail.vue b/plugins/network/frontend/views/SubnetDetail.vue index 6975044c..ca121ad0 100644 --- a/plugins/network/frontend/views/SubnetDetail.vue +++ b/plugins/network/frontend/views/SubnetDetail.vue @@ -59,6 +59,7 @@ import { ref, computed, onMounted } from 'vue' import { useRoute } from 'vue-router' import { networkApi } from '@/api' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() const subnet = ref(null) @@ -79,6 +80,7 @@ async function load() { try { const response = await networkApi.subnets.get(route.params.id) subnet.value = response.data?.data || response.data + setRecordTitle(subnet.value?.name || subnet.value?.cidr) } finally { loading.value = false } diff --git a/plugins/printedparts/frontend/views/PrintedItemDetail.vue b/plugins/printedparts/frontend/views/PrintedItemDetail.vue index e60d7345..e7fcbdc2 100644 --- a/plugins/printedparts/frontend/views/PrintedItemDetail.vue +++ b/plugins/printedparts/frontend/views/PrintedItemDetail.vue @@ -187,6 +187,7 @@ import { useRoute } from 'vue-router' import { printedpartsApi } from '@/api' import { withBase } from '@/utils/basePath' import Modal from '@/components/Modal.vue' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() const item = ref(null) @@ -196,6 +197,7 @@ onMounted(async () => { try { const response = await printedpartsApi.get(route.params.id) item.value = response.data.data + setRecordTitle(item.value?.itemname || item.value?.itemcode) await loadFiles() } catch (loadError) { console.error('Error loading printed item:', loadError) diff --git a/plugins/printers/frontend/routes.js b/plugins/printers/frontend/routes.js index 83b4f23d..f5428243 100644 --- a/plugins/printers/frontend/routes.js +++ b/plugins/printers/frontend/routes.js @@ -6,13 +6,13 @@ export default [ path: 'reports/toner', name: 'toner-report', component: () => import('./views/TonerReport.vue'), - meta: { plugin: 'printers' } + meta: { plugin: 'printers', title: 'Toner Report' } }, { path: 'reports/toner-forecast', name: 'toner-forecast', component: () => import('./views/TonerForecast.vue'), - meta: { plugin: 'printers' } + meta: { plugin: 'printers', title: 'Toner Forecast' } }, { path: 'settings/printertypes', diff --git a/plugins/printers/frontend/views/PrinterDetail.vue b/plugins/printers/frontend/views/PrinterDetail.vue index 88e05507..751bcaf6 100644 --- a/plugins/printers/frontend/views/PrinterDetail.vue +++ b/plugins/printers/frontend/views/PrinterDetail.vue @@ -305,6 +305,7 @@ import PluginAssetPanels from '@/components/PluginAssetPanels.vue' import AssetRelationships from '@/components/AssetRelationships.vue' import { useWarrantyBadge } from '@/composables/warrantyBadge' import { useIdentifierFlags } from '@/composables/identifierSettings' +import { setRecordTitle } from '@/composables/pageTitle' const route = useRoute() const { isEnabled } = useIdentifierFlags() @@ -344,6 +345,9 @@ onMounted(async () => { ]) printer.value = printerRes.data.data + // the page's own heading logic, which already discards the literal + // name 'NONE' that a discovered queue can carry + setRecordTitle(displayTitle.value) // supplies endpoint returns {ipaddress, pingstatus, supplies:[...]} supplies.value = suppliesRes.data.data?.supplies || [] // drivers are attached to the printer detail, matched by model diff --git a/plugins/usb/frontend/views/USBDetail.vue b/plugins/usb/frontend/views/USBDetail.vue index 0a687cd3..cf504f64 100644 --- a/plugins/usb/frontend/views/USBDetail.vue +++ b/plugins/usb/frontend/views/USBDetail.vue @@ -160,6 +160,7 @@ import { usbApi } from '@/api' import Modal from '@/components/Modal.vue' import { useToast } from '@/composables/toast' import { apiError } from '@/utils/apiError' +import { setRecordTitle } from '@/composables/pageTitle' const toast = useToast() const route = useRoute() @@ -181,6 +182,7 @@ async function loadDevice() { try { const response = await usbApi.get(route.params.id) device.value = response.data.data + setRecordTitle(device.value?.label || device.value?.serialnumber) } catch (error) { console.error('Error loading USB device:', error) } finally { diff --git a/plugins/warranty/frontend/routes.js b/plugins/warranty/frontend/routes.js index a535b415..54661f4c 100644 --- a/plugins/warranty/frontend/routes.js +++ b/plugins/warranty/frontend/routes.js @@ -18,7 +18,7 @@ export default [ path: 'reports/warranty', name: 'warranty-report', component: () => import('./views/WarrantyReport.vue'), - meta: { plugin: 'warranty' } + meta: { plugin: 'warranty', title: 'Warranty Report' } } ] From 0738d7152556c623eb8ae035e255d0bd475dd92d Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Wed, 26 Aug 2026 15:08:53 -0400 Subject: [PATCH 3/3] Guides image + draft gating, audit retention, ADR-015 gate, backup credential redaction --- CHANGELOG.md | 98 ++++++++ frontend/src/api/index.js | 5 + frontend/src/composables/pageTitle.js | 4 +- frontend/src/utils/siteSettings.js | 26 +- frontend/src/views/SearchResults.vue | 27 +++ .../src/views/settings/ServiceNowSettings.vue | 2 +- frontend/src/views/settings/SiteSettings.vue | 107 ++++++++- .../versions/0002_backups_lastseenat.py | 12 +- plugins/backups/plugin.py | 10 +- plugins/backups/services/ntlars.py | 64 +++++ plugins/computers/frontend/views/PCDetail.vue | 24 +- plugins/guides/api/routes.py | 163 +++++++++++-- .../frontend/components/GuideBlocks.vue | 33 ++- plugins/guides/frontend/utils/images.js | 77 ++++++ plugins/guides/frontend/utils/inline.js | 32 ++- plugins/guides/frontend/utils/inline.spec.js | 50 ++++ plugins/guides/frontend/views/GuideEditor.vue | 34 ++- .../machines/frontend/views/MachineDetail.vue | 24 +- .../frontend/views/MeasuringToolDetail.vue | 24 +- .../migrations/versions/0003_gagelabtag.py | 39 ++- .../printers/frontend/views/PrinterDetail.vue | 24 +- scripts/check-naming-and-style.sh | 21 +- scripts/redact_backup_secrets.py | 136 +++++++++++ shopdb/__init__.py | 3 +- shopdb/cli/__init__.py | 72 ++++++ shopdb/core/api/auditlogs.py | 74 +++++- shopdb/core/api/search.py | 132 +++++++--- shopdb/core/api/settings.py | 25 +- shopdb/core/models/user.py | 3 + shopdb/core/services/auditretention.py | 133 ++++++++++ tests/test_core/test_audit_retention.py | 135 +++++++++++ tests/test_core/test_search_degraded.py | 65 +++++ tests/test_core/test_search_integrations.py | 30 ++- tests/test_core/test_settings_branding.py | 15 +- tests/test_plugin_migrations.py | 23 +- tests/test_plugins/test_backups.py | 109 +++++++++ tests/test_plugins/test_guides.py | 227 ++++++++++++++++++ 37 files changed, 1953 insertions(+), 129 deletions(-) create mode 100644 plugins/guides/frontend/utils/images.js create mode 100644 plugins/guides/frontend/utils/inline.spec.js create mode 100644 scripts/redact_backup_secrets.py create mode 100644 shopdb/core/services/auditretention.py create mode 100644 tests/test_core/test_audit_retention.py create mode 100644 tests/test_core/test_search_degraded.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cc81df7..af07bb30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,104 @@ ADR-007 and ADR-002. ### Added +- **Audit log retention.** The audit log is append-only and nothing ever removed + a row, so it grew for as long as a site ran - here to the point where the + audit page was tens of thousands of pages deep and therefore useless, because + nobody pages through that to find anything. Site & Facility gains + `audit_retentiondays` plus a card showing the log's size and exactly how many + entries a purge would remove, and there is `flask audit stats` / + `flask audit purge --apply` for the server. **Opt-in: the default is 0, which + keeps everything.** An upgrade that silently started deleting a site's audit + history would be indefensible. Deletion runs in committed batches, because one + `DELETE` over a million rows holds table locks long enough to block the + collectors posting inventory; a run that stops early reports how much is left + rather than claiming it finished. Purging needs its own `audit.purge` + permission - reading the log and destroying it are different acts. + +- **Guides: images, and a reader who cannot see drafts.** Guide images are + fetched through the authenticated API client instead of being pointed at by + ``. An image tag sends no Authorization header and this app keeps its + JWT in one, so every guide image was a broken icon for every user; the same + change makes stored image links survive a subpath mount. A failed image now + says so in words rather than showing a broken icon. Revision and image routes + are gated on their parent guide - revision ids are sequential, so reading one + guide used to be enough to walk the ids and read every unpublished draft on + the site. Search, the guide list and the tag chips now match the PUBLISHED + text, not the working copy, so a reader can no longer get a hit on wording + that is not in the page when they open it. + +### Fixed + +- **A guide's URL no longer moves when its title is edited.** The slug was + regenerated on every save that carried a title, and the editor autosaves every + three seconds - so fixing a typo in a heading silently moved the guide and + broke exactly the pasted links the stored slug exists to protect. The slug is + now set once, at create. + +- **Publishing a guide after a failed save reported success.** `publish()` ran + regardless of whether the pre-publish save reached the server, so the author + saw "Published version N" while readers got the previous revision. A failed + save now stops the publish and says why. + +- **The ADR-015 site-literal check was enforcing nothing.** Its patterns are + assembled from fragments so the published script does not contain the strings + it greps for, but they used the apostrophe-escape form (`'a'\''b'`) instead of + concatenation (`'a''b'`), embedding a literal apostrophe in every pattern. + Nothing could ever match and the check passed everything from the day it was + written. Fixed, and it immediately found 25 real violations - among them a QA + ServiceNow tenant URL sitting in the shipped defaults in seven places. + +- **ServiceNow ships off and blank.** The defaults pointed at one organisation's + QA ServiceNow tenant with the integration enabled, so a site that had never + configured ServiceNow still rendered ticket links - to a tenant its + technicians cannot reach, and for an adopting site, into somebody else's + instance. There is no cross-site default for "which ServiceNow is yours". + +- **Credentials are no longer stored in backup revisions.** The NTLARS/DNC + registry carries an FTP password in cleartext, and it was stored verbatim - + rendered by the revision panel and handed back by the download route as a + ready-to-import `.reg`. Credential values are now redacted at parse, so they + never reach the database, and the rendered file NAMES each withheld value + instead of carrying a placebo one: importing a fake password would produce a + machine that looks restored and cannot connect. `scripts/redact_backup_secrets.py` + scrubs revisions recorded before this (dry run by default). Rotate any + password that was stored. + +- **A search domain that fails is no longer indistinguishable from one that + found nothing.** Each domain catches broadly and returns no rows so one broken + table cannot take the whole search box down - but the technician was then told + they might have misspelled a query that was right, and concluded the record + did not exist. Failed domains are named in the response, shown as a banner, + and suppress the did-you-mean. + +- **An empty `sourcehostname` defeated backup dedup permanently.** The dedup + query maps `''` to `IS NULL` while the insert stored the raw `''`, so those + rows could never match their own chain: every post inserted, the table grew + without bound and the history filled with identical entries. Normalized once + at ingest, whitespace included. + +- **`printedparts` migration 0003 was unguarded.** Unlike its twenty siblings it + ran a bare `ALTER`, so `flask plugin upgrade-all` failed on any database built + by `create_all` and took 0004 down with it. The migration test that should + have caught this only covered single-migration chains; it now covers every + post-cutover chain. + +- **Guide links with several parameters opened the wrong page.** Text is escaped + before formatting, so a captured URL arrives carrying entities and was then + escaped a second time - `&amp;` in every multi-parameter ServiceNow and + SharePoint link. Emphasis could also reach inside an `href`, and + `//host` passed as an in-app link when the browser reads it as off-site. + +- **Retire an asset from the interface.** Machines, PCs, printers and measuring + tools had no way to take a record out of ShopDB without calling the API, so a + scrapped tool stayed in every list and report. The endpoints already existed + and were already soft deletes: the record leaves the lists while its + calibration history, backups and relationships stay put, and it can be + restored by setting `isactive` back. Called Retire rather than Delete for + that reason, and the confirm says what is kept. USB is not included - it + retires through a status column in the external CMMC database and needs its + own decision. + - **`flask catalog dedupe`, `merge` and `rename`: catalog hygiene.** The legacy data carried space-stripped twins - `DellInc.` beside `Dell Inc.`, `Windows10` beside `Windows 10` - which rode into the shipped seed catalog diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js index 0e9e54bb..d6bb4013 100644 --- a/frontend/src/api/index.js +++ b/frontend/src/api/index.js @@ -343,6 +343,11 @@ export const printersApi = { update(id, data) { return api.put(`/printers/${id}`, data) }, + // Soft delete: the server deactivates the asset, so the printer leaves every + // list while its history and relationships stay intact. + remove(id) { + return api.delete(`/printers/${id}`) + }, updateExtension(id, data) { return api.put(`/printers/${id}/printerdata`, data) }, diff --git a/frontend/src/composables/pageTitle.js b/frontend/src/composables/pageTitle.js index b466eb6f..0b5777b0 100644 --- a/frontend/src/composables/pageTitle.js +++ b/frontend/src/composables/pageTitle.js @@ -1,7 +1,7 @@ // The browser tab title, including WHICH record is open. // // The router can only name the KIND of page, because it runs before the view -// has fetched anything: every application read "West Jefferson ShopDB - +// has fetched anything: every application read " ShopDB - // Application", so two tabs on two different machines were indistinguishable, // and a shared link previewed as the section rather than the thing. // @@ -51,7 +51,7 @@ export async function applyGenericTitle(key, page) { if (key !== activeKey) return // navigated on while we awaited // A view that named the record before the facility name arrived composed its // title against the placeholder prefix, so rewrite it rather than leaving - // 'ShopDB - CSF16' on a site called West Jefferson. + // 'ShopDB - CSF16' without the facility name in front of it. document.title = compose(recordTitle !== null ? recordTitle : page) } diff --git a/frontend/src/utils/siteSettings.js b/frontend/src/utils/siteSettings.js index c7ec67ef..a333a950 100644 --- a/frontend/src/utils/siteSettings.js +++ b/frontend/src/utils/siteSettings.js @@ -98,22 +98,20 @@ function darkenHexColor(hex, fraction) { // ServiceNow ticket-link config. Returns the enabled flag, incident/change URL // templates, and the global-search URL template (all with a {ticket} -// placeholder). Defaults mirror the previously-hardcoded GE ServiceNow URLs. +// placeholder). +// +// The defaults are BLANK and the integration ships OFF, matching the server. +// They used to be one organisation's QA ServiceNow tenant, so any site that had +// not configured ServiceNow still rendered ticket links - pointing its +// technicians at a tenant they cannot reach and, for an adopting site, at +// somebody else's. A blank URL renders tickets as plain text, which is the +// correct behaviour for "not configured". export async function getServicenowUrls() { - const enabledRaw = await getSetting('servicenow_enabled', 'true') + const enabledRaw = await getSetting('servicenow_enabled', 'false') const enabled = enabledRaw !== 'false' && enabledRaw !== '0' && enabledRaw !== false - const incidentUrl = await getSetting( - 'servicenow_incident_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) - const changeUrl = await getSetting( - 'servicenow_change_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) - const searchUrl = await getSetting( - 'servicenow_search_url', - 'https://geaerospaceqa.service-now.com/now/nav/ui/search/0f8b85d0c7922010099a308dc7c2606a/params/search-term/{ticket}/global-search-data-config-id/c861cea2c7022010099a308dc7c26041/' - ) + const incidentUrl = await getSetting('servicenow_incident_url', '') + const changeUrl = await getSetting('servicenow_change_url', '') + const searchUrl = await getSetting('servicenow_search_url', '') return { enabled, incidentUrl, changeUrl, searchUrl } } diff --git a/frontend/src/views/SearchResults.vue b/frontend/src/views/SearchResults.vue index 1eb53316..3e3d4b41 100644 --- a/frontend/src/views/SearchResults.vue +++ b/frontend/src/views/SearchResults.vue @@ -35,6 +35,18 @@
Searching...
diff --git a/plugins/backups/migrations/versions/0002_backups_lastseenat.py b/plugins/backups/migrations/versions/0002_backups_lastseenat.py index b4f5c70a..e71501a8 100644 --- a/plugins/backups/migrations/versions/0002_backups_lastseenat.py +++ b/plugins/backups/migrations/versions/0002_backups_lastseenat.py @@ -27,8 +27,10 @@ depends_on = None def upgrade(): - columns = {c['name'] for c in - sa.inspect(op.get_bind()).get_columns('backuprevisions')} + inspector = sa.inspect(op.get_bind()) + if 'backuprevisions' not in inspector.get_table_names(): + return + columns = {c['name'] for c in inspector.get_columns('backuprevisions')} if 'lastseenat' not in columns: op.add_column('backuprevisions', sa.Column('lastseenat', sa.DateTime(), nullable=True)) @@ -37,7 +39,9 @@ def upgrade(): def downgrade(): - columns = {c['name'] for c in - sa.inspect(op.get_bind()).get_columns('backuprevisions')} + inspector = sa.inspect(op.get_bind()) + if 'backuprevisions' not in inspector.get_table_names(): + return + columns = {c['name'] for c in inspector.get_columns('backuprevisions')} if 'lastseenat' in columns: op.drop_column('backuprevisions', 'lastseenat') diff --git a/plugins/backups/plugin.py b/plugins/backups/plugin.py index b2e59cc4..cb6f6575 100644 --- a/plugins/backups/plugin.py +++ b/plugins/backups/plugin.py @@ -410,7 +410,13 @@ class BackupsPlugin(BasePlugin): # with_for_update serializes concurrent posts for the same chain: the # fleet collects on one GE-Enforce cycle, so a retry could otherwise # pass the hash check twice and insert the same revision. - sourcehostname = payload.get('sourcehostname') + # Normalize ONCE, and use the same value to query and to store. + # samesource() maps '' to IS NULL, but the insert below used to store + # the raw '' - so a client posting an empty hostname wrote rows that its + # own next post could never match. Every post then deduped against + # nothing, inserting forever: unbounded growth, and a history no + # technician can read. Whitespace does the same thing more quietly. + sourcehostname = (payload.get('sourcehostname') or '').strip() or None latest = (db.session.query(BackupRevision) .filter(BackupRevision.assetid == assetid, BackupRevision.backupkind == kindkey, @@ -476,7 +482,7 @@ class BackupsPlugin(BasePlugin): sharepath=sharepath, sourcefilename=sourcefilename, bytesize=bytesize, - sourcehostname=payload.get('sourcehostname'), + sourcehostname=sourcehostname, collectedat=collectedat or datetime.utcnow(), lastseenat=datetime.utcnow(), ) diff --git a/plugins/backups/services/ntlars.py b/plugins/backups/services/ntlars.py index 6e65ff9d..d0d992c4 100644 --- a/plugins/backups/services/ntlars.py +++ b/plugins/backups/services/ntlars.py @@ -32,6 +32,31 @@ import re SCHEMA = 'ntlars/1' +# SECRETS ARE NOT BACKED UP. +# +# The DNC registry carries an FTP password in cleartext. Stored verbatim it +# reached three places it must not: the backuprevisions table, the revision +# panel (which renders values), and the download route (which hands back a +# ready-to-import .reg). Anyone with backups.view could read the fleet's FTP +# password off a web page. +# +# Redaction happens at PARSE, so the secret never enters the database at all. +# Redacting only on display would leave it at rest and one query away. +# +# What is deliberately NOT done: dropping the value silently. This module's +# parser refuses to discard an unparseable value precisely because a backup that +# looks complete and restores a machine with a setting missing is the worst +# outcome. A redacted value keeps its NAME and records itself in +# `redacted`, so both the panel and a restore can say plainly that this one +# field must be set by hand. +REDACTEDMARKER = '***REDACTED***' +SECRETNAMEPATTERN = re.compile(r'passw|pwd|secret|apikey|token', re.IGNORECASE) + + +def issecretname(name): + """Whether a registry value name holds a credential.""" + return bool(SECRETNAMEPATTERN.search(name or '')) + REGROOT = 'HKEY_LOCAL_MACHINE' DNCPATH = r'SOFTWARE\GE Aircraft Engines\DNC' DNCPATHWOW = r'SOFTWARE\WOW6432Node\GE Aircraft Engines\DNC' @@ -245,6 +270,20 @@ def parse(raw): dialect = 'wow6432node' if sawwow else ('ntlars' if sawplain else 'unknown') + # Strip credentials before the projection is returned - i.e. before anything + # persists it. Recorded by path\name so the omission is visible rather than + # discovered during a restore. + redacted = [] + for path in keys: + for name, value in keys[path].items(): + if not issecretname(name): + continue + if value.get('data') in ('', None): + continue # nothing to hide + keys[path][name] = {'type': value.get('type', 'REG_SZ'), + 'data': REDACTEDMARKER} + redacted.append('{}\\{}'.format(path, name) if path else name) + return { 'schema': SCHEMA, 'sourcedialect': dialect, @@ -252,6 +291,7 @@ def parse(raw): {'path': path, 'values': dict(sorted(keys[path].items()))} for path in sorted(keys) ], + 'redacted': sorted(redacted), } @@ -291,6 +331,12 @@ def render(projection, dialect='ntlars', comments=None): dialect='ntlars' omits WOW6432Node - use with the NTLARS Load... button dialect='wow6432node' includes it - use with `reg import` on 64-bit + + Redacted credentials are rendered as a comment naming the value, NOT as the + marker string. Importing '***REDACTED***' as the FTP password would leave a + machine that looks restored and cannot connect, and the technician would be + debugging the network. A commented-out value fails loudly at the one moment + somebody is looking at the file. """ if dialect not in ('ntlars', 'wow6432node'): raise ValueError('unknown dialect: {!r}'.format(dialect)) @@ -302,10 +348,28 @@ def render(projection, dialect='ntlars', comments=None): if comments: out.append('') + redacted = projection.get('redacted') or [] + if redacted: + out.append('; ---------------------------------------------------------------') + out.append('; {} credential value(s) were NOT backed up and are NOT in this' + .format(len(redacted))) + out.append('; file. Set them by hand after importing:') + for name in redacted: + out.append('; {}'.format(name)) + out.append('; ---------------------------------------------------------------') + out.append('') + for key in projection.get('keys', []): path = key.get('path', '') out.append('[{}]'.format(root + ('\\' + path if path else ''))) for name, entry in (key.get('values') or {}).items(): + # A redacted credential is commented out, never emitted as the + # marker. Importing the marker as a password produces a machine that + # looks restored and cannot log in. + if (entry or {}).get('data') == REDACTEDMARKER: + out.append('; "{}" was not backed up - set it by hand'.format( + _escape(name))) + continue out.append(_rendervalue(name, entry)) out.append('') diff --git a/plugins/computers/frontend/views/PCDetail.vue b/plugins/computers/frontend/views/PCDetail.vue index 09f7a0ba..b288c17c 100644 --- a/plugins/computers/frontend/views/PCDetail.vue +++ b/plugins/computers/frontend/views/PCDetail.vue @@ -5,6 +5,7 @@
Print Label Edit + Back to List
@@ -254,7 +255,7 @@