From 271d558f8b9a55491c2cade85d72a1169bc18a43 Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Wed, 26 Aug 2026 10:07:51 -0400 Subject: [PATCH] An unmeasurable backup must not hide a working one --- plugins/geenforce/api/routes.py | 26 ++- plugins/guides/manifest.json | 13 ++ plugins/guides/models/__init__.py | 3 + plugins/guides/models/guide.py | 188 ++++++++++++++++++ plugins/guides/services/__init__.py | 0 plugins/guides/services/blocks.py | 131 ++++++++++++ .../test_geenforce_publish_gate.py | 22 +- 7 files changed, 372 insertions(+), 11 deletions(-) create mode 100644 plugins/guides/manifest.json create mode 100644 plugins/guides/models/__init__.py create mode 100644 plugins/guides/models/guide.py create mode 100644 plugins/guides/services/__init__.py create mode 100644 plugins/guides/services/blocks.py diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index ec473ccb..2470df98 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -1111,17 +1111,29 @@ def _attach_backup_state(facts, hostnames): # rows are already newest-confirmed-first, so the first is freshest. freshest[kindkey] = revision + # Among kinds that can be JUDGED, the stalest decides. A kind whose newest + # revision has never been confirmed (lastseenat NULL - a backfilled row, or + # one written before the column existed) is not stale; it is unmeasurable, + # and it must not win. + # + # Treating NULL as "worst" looked right and was wrong: a host with a healthy + # ntlars backup and one never-confirmed row showed NO BADGE, because the + # verdict for a NULL confirmation is tri-state None. A working backup then + # renders as nothing at all, which is the one thing the badge must never do. + # This is the same rule staleness.py already applies: a chain becomes + # measurable the first time its PC posts, and says nothing until then. stalest = {} for (key, _kind), revision in freshest.items(): - current = stalest.get(key) - if current is None: - stalest[key] = revision - continue - # None (never confirmed) is worse than any date. if revision.lastseenat is None: + continue + current = stalest.get(key) + if current is None or revision.lastseenat < current.lastseenat: stalest[key] = revision - elif current.lastseenat is not None and revision.lastseenat < current.lastseenat: - stalest[key] = revision + + # Every kind unmeasurable: fall back to any revision so the host still + # reports which kind it has, with a None verdict and no badge. + for (key, _kind), revision in freshest.items(): + stalest.setdefault(key, revision) for key, revision in stalest.items(): entry = facts.get(key) diff --git a/plugins/guides/manifest.json b/plugins/guides/manifest.json new file mode 100644 index 00000000..d9234e98 --- /dev/null +++ b/plugins/guides/manifest.json @@ -0,0 +1,13 @@ +{ + "name": "guides", + "display_name": "Guides", + "version": "1.0.0", + "description": "Step-by-step guides written and read inside ShopDB, with images, working checklists and printable output", + "author": "ShopDB Team", + "dependencies": [], + "core_version": ">=0.1.0,<1.0.0", + "api_prefix": "/api/guides", + "provides": { + "features": ["guides"] + } +} diff --git a/plugins/guides/models/__init__.py b/plugins/guides/models/__init__.py new file mode 100644 index 00000000..c32b970e --- /dev/null +++ b/plugins/guides/models/__init__.py @@ -0,0 +1,3 @@ +from .guide import Guide, GuideImage, BLOCKTYPES + +__all__ = ['Guide', 'GuideImage', 'BLOCKTYPES'] diff --git a/plugins/guides/models/guide.py b/plugins/guides/models/guide.py new file mode 100644 index 00000000..5df9ef4b --- /dev/null +++ b/plugins/guides/models/guide.py @@ -0,0 +1,188 @@ +"""Guides: step-by-step procedures written and read inside ShopDB. + +WHY THIS IS NOT THE KNOWLEDGE BASE PLUGIN + + The knowledgebase plugin is a LINK REGISTRY - a short description, a URL and + some keywords, pointing at a document that lives somewhere else. That is a + different job. This holds the document itself. + +WHY BLOCKS RATHER THAN A BLOB OF HTML + + A guide is stored as an ordered list of typed blocks, each with a stable id, + serialized as JSON. Free-form HTML would be easier to write and would defeat + three of the four things this feature exists for: + + checklists a tickable step has to be identifiable to be remembered, and + an buried in pasted markup has no identity + dark mode pasted content carries hardcoded colours, which then fight the + theme - black text on a dark background, and nobody can say + why + printing print has to know where a step begins and ends to avoid + splitting one across a page + + Blocks also mean a new block type later is additive: old guides keep + rendering, because the renderer skips what it does not know. + +INLINE TEXT IS A RESTRICTED MARKDOWN SUBSET - bold, italic, inline code, links, +line breaks, and nothing else. No fonts, sizes or colours: emphasis is semantic +and the theme decides how it looks. That single rule is what keeps a guide +written next year looking like one written today, and what makes dark mode work +without an author thinking about it. +""" + +import json +from datetime import datetime + +from shopdb.api import db +from shopdb.core.models.base import BaseModel + + +def _utcnow(): + return datetime.utcnow() + + +# Block types the renderer knows. Stored as data, not as markup, so the same +# guide can render to screen, to dark mode and to paper differently. +BLOCKTYPES = ( + 'heading', + 'paragraph', + 'step', # a numbered step, with its own body + 'checklist', # tickable items - the reason progress is a feature + 'bullets', + 'numbers', + 'callout', # info / warning / success, per the KB house style + 'code', + 'image', + 'table', +) + + +class Guide(BaseModel): + """One guide. Blocks live in a JSON column rather than a child table. + + A guide is edited and saved WHOLE - nobody reorders one paragraph in + isolation - so a child table would buy ordering machinery, a second + migration and N+1 reads, in exchange for nothing this feature needs. The + same reasoning the geenforce manifests and backup payloads already follow. + """ + + __tablename__ = 'guides' + + guideid = db.Column(db.Integer, primary_key=True) + + # Human title, and the slug the URL uses. The slug is stored rather than + # 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) + + # One line, shown in the list and in search results. + summary = db.Column(db.String(1000)) + + # Comma-separated, matching how knowledgebase.keywords already works, so one + # search can cover both and there is one tag vocabulary rather than two. + tags = db.Column(db.String(500)) + + # 'draft' or 'published'. A draft is visible to people who can edit, and + # invisible to everyone else: a half-written procedure on a shop floor is + # worse than no procedure. + status = db.Column(db.String(20), nullable=False, default='draft', index=True) + + # MEDIUMTEXT: serialized blocks. Text rather than a native JSON column so the + # bytes come back exactly as written and a diff between versions is stable - + # the same reason the backups plugin stores its projection as text. + blocksjson = db.Column(db.Text(length=16777215), nullable=False, default='[]') + + 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". + revision = db.Column(db.Integer, nullable=False, default=1) + + @property + def blocks(self): + """The decoded block list, or [] on a corrupt row. + + Deliberately forgiving, unlike the backups payload getter: a guide that + will not render is a page a person cannot read, and an empty guide with + its title and tags intact is a better failure than an error page. The + corruption is still visible - the editor shows no blocks. + """ + if not self.blocksjson: + return [] + try: + data = json.loads(self.blocksjson) + except (ValueError, TypeError): + return [] + return data if isinstance(data, list) else [] + + @blocks.setter + def blocks(self, value): + self.blocksjson = json.dumps(value or [], separators=(',', ':')) + + def taglist(self): + """Tags as a clean list, empties dropped.""" + return [t.strip() for t in (self.tags or '').split(',') if t.strip()] + + def to_dict(self, includeblocks=False): + data = { + 'guideid': self.guideid, + 'title': self.title, + 'slug': self.slug, + 'summary': self.summary, + '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, + 'publishedat': (self.publishedat.isoformat() + 'Z' + if self.publishedat else None), + } + if includeblocks: + data['blocks'] = self.blocks + return data + + +class GuideImage(BaseModel): + """An image pasted or dropped into a guide. + + Bytes live in the database, not on disk. Guides are small, the images are + screenshots rather than photographs, and a row travels with a database + backup - whereas a file under instance/ has been lost to a container rebuild + on this very deployment before. If a site ever puts thousands of these in, + the storage decision is one column and one route to revisit. + """ + + __tablename__ = 'guideimages' + + guideimageid = db.Column(db.Integer, primary_key=True) + + # Nullable: an image is uploaded while a NEW guide is still unsaved, so it + # exists before the guide it belongs to does. The editor attaches it on + # save; anything still unattached is orphaned and prunable. + guideid = db.Column(db.Integer, db.ForeignKey('guides.guideid', + ondelete='CASCADE'), index=True) + + filename = db.Column(db.String(255)) + mimetype = db.Column(db.String(100), nullable=False) + bytesize = db.Column(db.Integer, nullable=False) + 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 { + 'guideimageid': self.guideimageid, + 'guideid': self.guideid, + 'filename': self.filename, + 'mimetype': self.mimetype, + 'bytesize': self.bytesize, + 'url': '/api/guides/images/{}'.format(self.guideimageid), + 'createdat': self.createdat.isoformat() + 'Z' if self.createdat else None, + } diff --git a/plugins/guides/services/__init__.py b/plugins/guides/services/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/plugins/guides/services/blocks.py b/plugins/guides/services/blocks.py new file mode 100644 index 00000000..ef07608b --- /dev/null +++ b/plugins/guides/services/blocks.py @@ -0,0 +1,131 @@ +"""Block validation. + +The editor is the only thing that writes blocks today, so this is not defending +against a hostile author - it is defending against a BAD SAVE. A block list that +is wrong in a way nobody notices at save time becomes a guide that will not +render weeks later, when the person who wrote it has moved on and the only +evidence is a blank page. + +So: validate at the door, name the block that is wrong, and refuse the save. +The alternative - storing whatever arrives and coping at render time - was tried +in the backups plugin with a manifest and produced exactly the failure this +avoids: a client bug turning into a broken page that outlived it. + +WHAT IS NOT VALIDATED HERE: the inline text. It is a restricted Markdown subset +rendered through a sanitiser in the browser, which is where a renderer belongs. +Storing text verbatim keeps what the author typed recoverable even if the +rendering rules change later. +""" + +import json + +from ..models import BLOCKTYPES + +# A guide is a procedure, not a document repository. This cap is generous for +# anything a person will read on a shop floor and small enough that a runaway +# paste cannot fill a MEDIUMTEXT column. +MAXBLOCKSBYTES = 2 * 1024 * 1024 +MAXBLOCKS = 500 +MAXTEXT = 20000 + + +class BlockError(ValueError): + """Raised with a message naming the offending block, for the API to relay.""" + + +def _fail(index, message): + raise BlockError('block {}: {}'.format(index + 1, message)) + + +def validate(blocks): + """Check a block list, returning it unchanged, or raise BlockError. + + Kept pure - no database, no request - so the rules are testable directly and + the same function can be reused by an importer later. + """ + if not isinstance(blocks, list): + raise BlockError('blocks must be a list') + if len(blocks) > MAXBLOCKS: + raise BlockError('a guide is limited to {} blocks'.format(MAXBLOCKS)) + + seen = set() + for index, block in enumerate(blocks): + if not isinstance(block, dict): + _fail(index, 'must be an object') + + blockid = block.get('id') + if not isinstance(blockid, str) or not blockid.strip(): + # Ids are what make a checklist item rememberable between visits. + # A block without one silently loses its tick the next time the + # guide is opened, which reads as "the checkboxes do not work". + _fail(index, 'needs a stable id') + if blockid in seen: + _fail(index, 'duplicate id {!r} - ticks would follow the wrong item' + .format(blockid)) + seen.add(blockid) + + blocktype = block.get('type') + if blocktype not in BLOCKTYPES: + _fail(index, 'unknown type {!r}. Known types: {}'.format( + blocktype, ', '.join(BLOCKTYPES))) + + text = block.get('text') + if text is not None: + if not isinstance(text, str): + _fail(index, 'text must be a string') + if len(text) > MAXTEXT: + _fail(index, 'text is longer than {} characters'.format(MAXTEXT)) + + items = block.get('items') + if items is not None: + if not isinstance(items, list): + _fail(index, 'items must be a list') + for item in items: + # Checklist items carry their own id for the same reason blocks + # do: that id is the thing a tick is stored against. + if isinstance(item, dict): + if not isinstance(item.get('id'), str) or not item['id'].strip(): + _fail(index, 'every item needs an id') + if not isinstance(item.get('text', ''), str): + _fail(index, 'item text must be a string') + elif not isinstance(item, str): + _fail(index, 'items must be strings or objects') + + encoded = json.dumps(blocks, separators=(',', ':')) + if len(encoded.encode('utf-8')) > MAXBLOCKSBYTES: + raise BlockError('this guide is larger than {} MB'.format( + MAXBLOCKSBYTES // (1024 * 1024))) + return blocks + + +def plaintext(blocks): + """Flatten a guide to searchable text. + + Search should find a guide by something written halfway down it, not only by + its title - that is how people actually look for a procedure ("the one about + the drive letter"). Cheap because it runs on save, not on every query. + """ + parts = [] + for block in blocks or []: + if not isinstance(block, dict): + continue + if isinstance(block.get('text'), str): + parts.append(block['text']) + for item in block.get('items') or []: + if isinstance(item, str): + parts.append(item) + elif isinstance(item, dict) and isinstance(item.get('text'), str): + parts.append(item['text']) + for row in block.get('rows') or []: + if isinstance(row, list): + parts.extend(cell for cell in row if isinstance(cell, str)) + return '\n'.join(parts) + + +def checklistitemcount(blocks): + """How many tickable items a guide has, for the list and the progress bar.""" + total = 0 + for block in blocks or []: + if isinstance(block, dict) and block.get('type') == 'checklist': + total += len(block.get('items') or []) + return total diff --git a/tests/test_plugins/test_geenforce_publish_gate.py b/tests/test_plugins/test_geenforce_publish_gate.py index 88e98e90..6b8255c5 100644 --- a/tests/test_plugins/test_geenforce_publish_gate.py +++ b/tests/test_plugins/test_geenforce_publish_gate.py @@ -155,9 +155,20 @@ def test_hosts_below_libversion_reports_the_floor(db): # -- backup state on the reports table --------------------------------------- -def test_backup_state_prefers_the_newest_confirmed_revision(db): - """A host with several revisions shows the most recently CONFIRMED one, and - a revision that has never been confirmed (lastseenat NULL) never wins. +def test_backup_state_reports_the_stalest_measurable_kind(db): + """A host is judged by the WORST backup it has, not the best. + + This used to prefer the most recently confirmed revision. That was right + while a PC reported one kind, and became wrong when part markers arrived: a + marker bay reports ntlars AND partmarker, so a daily ntlars post kept the + badge green while the marker backup had been failing for a month. A host is + healthy only when everything it backs up is current, and the badge names the + kind that is not. + + A revision that has never been confirmed (lastseenat NULL) still never wins. + It is not stale, it is unmeasurable - and letting it win hid a working + backup entirely, because the verdict for a NULL confirmation is tri-state + None, which renders as no badge at all. Ordering happens in Python on purpose. Doing it in SQL needs ORDER BY ... NULLS LAST, which SQLite accepts and MySQL rejects outright - @@ -191,7 +202,10 @@ def test_backup_state_prefers_the_newest_confirmed_revision(db): facts = {'wjpc01': {'backupkind': None, 'backuplastseen': None}} _attach_backup_state(facts, {'WJPC01'}) - assert facts['wjpc01']['backupkind'] == 'udc' + # ntlars is 9 days old, udc was confirmed just now: the host is judged by + # ntlars, and the badge names it so somebody knows WHICH backup stopped. + assert facts['wjpc01']['backupkind'] == 'ntlars' + # 'files' has never been confirmed, so it neither wins nor blanks the badge. assert facts['wjpc01']['backuplastseen'] is not None