diff --git a/CHANGELOG.md b/CHANGELOG.md index 5841183..e128b33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,35 @@ ADR-007 and ADR-002. ## [Unreleased] +### Fixed + +- The 3D parts kiosk label prefix never appeared on the kiosk. The kiosk runs + logged out, and an unauthenticated read of a setting is limited to an + allowlist the key was not on, so the kiosk got a 404 and fell back to no + prefix. An admin previewing the same page while logged in saw it, which is + why it looked like it worked. A plugin now declares which of its settings a + logged-out page may read. +- The same setting also looked like it would not save. The row did not exist on + a site that installed the plugin before the setting was added, so the first + save created it - under a placeholder category, where the settings page, which + lists by category, could no longer see it. The value was in the database the + whole time. A first-time save now files the key under the category its owner + declares. Any row already misfiled is repaired in place on upgrade, value + untouched. The 3D parts alert email and the employee directory host were in + that state. +- Settings added in a later plugin version never reached a site that installed + an earlier one. The seeding ran from `on_install` / `on_enable`, which fire + only on a state transition, so neither ran again on an upgrade - and the + comment claiming enable ran every upgrade cycle was wrong. + `flask plugin upgrade-all` now seeds missing declared settings, without + touching values a site has configured. + +### Added + +- Plugin contract 0.16.0: `get_settings_defaults()` lets a plugin declare the + settings it owns (key, default, type, category, description, and whether an + unauthenticated caller may read it). See `docs/PLUGIN-HOOKS.md`. + ## [0.8.1] - 2026-08-05 Everything here shipped after v0.8.0 was tagged the same morning, driven by two diff --git a/CLAUDE.md b/CLAUDE.md index 4a4bc3b..daf1920 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,7 +46,7 @@ Refactor phases 0-5 landed; phase 6 (multi-site distribution readiness) largely - 1159 tests, naming/style check green, Gitea Actions CI (backend + naming + frontend build + a lean-build job + a migrations-mysql job that runs the real fresh upgrade on utf8mb4 MySQL 8) - GE-Enforce HTTPS cutover: the displays/kiosks cohort now fetches manifest + inline payloads entirely over HTTPS (share-less); the `gea-shopfloor-display` scope is authored in code (`plugins/geenforce/seed_display_scope.py`) and published via `seed_display_scope(publish=True)`. Other fleet PC types still enforce from the SMB share and only report. See `docs/geenforce-api-cutover.md`. -- `__contract_version__` at 0.15.0 (0.12.0 mailer, 0.13.0 User/Role, 0.14.0 send_webhook, 0.15.0 authorized_service_token) (product `__version__` 0.7.0, tags v0.5.0/v0.6.0/v0.7.0 - distinct series, ADR-007) +- `__contract_version__` at 0.16.0 (0.12.0 mailer, 0.13.0 User/Role, 0.14.0 send_webhook, 0.15.0 authorized_service_token, 0.16.0 get_settings_defaults) (product `__version__` 0.7.0, tags v0.5.0/v0.6.0/v0.7.0 - distinct series, ADR-007) - 13 bundled plugins all satisfy contract: computers, employees, geenforce, knowledgebase, machines, measuringtools, network, notifications, printedparts, printers, slides, usb, warranty - Core Alembic chain: baseline `68b3947ae14f` -> head `7d26_settings_description_text` (33 core migrations). Each plugin owns its own chain (ADR-008); deploy runs `flask db upgrade` then `flask plugin upgrade-all`. Reproducible + idempotent from empty (env.py relaxes session sql_mode so the chain runs on strict MySQL 8). - Lean per-site builds (ADR-013 + ADR-014): `scripts/build-site.sh` (backend) + `SITE_PLUGINS` via `scripts/stage-frontend.mjs` (frontend) ship only chosen plugins; `flask plugin prune-schema` drops non-installed plugins' tables at provisioning. Sidebar nav / settings / Displays all gate on staged routes. Manifest-less `plugins//frontend/` dirs (e.g. `applications`) are core and always ship. diff --git a/docs/API-REFERENCE.md b/docs/API-REFERENCE.md index e2c44de..f445f12 100644 --- a/docs/API-REFERENCE.md +++ b/docs/API-REFERENCE.md @@ -76,7 +76,7 @@ keying) are covered there as well. Everything else is the core UI API: the endpoints the Vue frontend calls. As a rule these are JWT-authenticated (a login token or a managed Personal Access Token) and versioned by the plugin contract (`__contract_version__`, currently -0.15.0). Behavior and stability guarantees are in **CONTRACT-STABILITY.md**; +0.16.0). Behavior and stability guarantees are in **CONTRACT-STABILITY.md**; sister sites should pin tight `core_version` ranges until the contract reaches 1.0. @@ -105,6 +105,7 @@ gated by `require_role` or `require_permission`; none are public. | `POST /api/setup/create-admin` | First-run only; creates the first admin, then 403s forever. | | `GET /api/settings/map-blueprint/` | Serve the floor-map blueprint image. | | `GET /api/settings/branding/` | Serve site branding assets (logo, etc.). | +| `GET /api/settings` and `GET /api/settings/` | Read-only, and only the public allowlist: the `branding` and `map` categories, a few named site keys, plus any key a plugin declares `public` in `get_settings_defaults` (e.g. `printedparts_label_prefix`, which the logged-out parts kiosk renders). Every other key answers 404 to an anonymous caller. | | `GET /api/models/image/` | Serve a model image. | | `GET /api/dashboard/navigation` | Public navigation tree. | | `GET /api/dashboard/health` | Liveness / health probe. | diff --git a/docs/CONTRACT-STABILITY.md b/docs/CONTRACT-STABILITY.md index 7a1f76b..ffcd313 100644 --- a/docs/CONTRACT-STABILITY.md +++ b/docs/CONTRACT-STABILITY.md @@ -8,7 +8,7 @@ the live code, not aspiration. The authoritative hook reference is ## Current version -The plugin contract is at **0.13.0**, declared in `shopdb/__init__.py` as +The plugin contract is at **0.16.0**, declared in `shopdb/__init__.py` as `__contract_version__`. It is pre-1.0, which under semver means any 0.x minor bump is allowed to break the contract, and this project has used that latitude. @@ -31,10 +31,12 @@ Recorded in the comment block in `shopdb/__init__.py`: | 0.11.0 | Added `service_token_authorized(scope)` to `shopdb.api` so a plugin's unattended endpoints (e.g. the GE-Enforce fetch API) can authorize a scoped managed service token without importing core token internals | additive surface (minor) | | 0.12.0 | Added the mailer helpers (`send_email`, `send_alert`) to `shopdb.api` | additive surface (minor) | | 0.13.0 | Added the `User` model to the `shopdb.api` surface | additive surface (minor) | +| 0.14.0 | Added `send_webhook` to the `shopdb.api` surface | additive surface (minor) | +| 0.15.0 | Added `authorized_service_token` / the `SupportTeam` model to the `shopdb.api` surface | additive surface (minor) | +| 0.16.0 | Added the `get_settings_defaults` hook so a plugin declares the Setting rows it owns; the framework seeds them at install, at enable, and on `flask plugin upgrade-all`, files a first-time write under the declared category, and honours `public: True` for pages that render before login | additive optional hook (minor) | -The source comment block documents 0.3.0, 0.4.0, 0.6.0, 0.7.0, 0.9.0, 0.10.0, and 0.11.0 (its -last entry); the current `__contract_version__` 0.13.0 is ahead of the last documented comment -entry. Earlier points +The source comment block documents 0.3.0, 0.4.0, 0.6.0, 0.7.0, 0.9.0, 0.10.0, 0.11.0, and +0.16.0; 0.12.0 through 0.15.0 are recorded in this table only. Earlier points (0.1.x / 0.2.x) predate that recorded rationale; `PluginMeta`'s fallback `core_version` default of `>=0.2.0,<1.0.0` is the only remaining trace of the 0.2 baseline. diff --git a/docs/PLUGIN-HOOKS.md b/docs/PLUGIN-HOOKS.md index 6b81979..ef79839 100644 --- a/docs/PLUGIN-HOOKS.md +++ b/docs/PLUGIN-HOOKS.md @@ -9,7 +9,7 @@ The contract is locked in [ADR-001](../docs/adr/ADR-001-asset-as-platform-contra The framework declares its contract version in `shopdb/__init__.py`: ```python -__contract_version__ = '0.15.0' +__contract_version__ = '0.16.0' ``` Each plugin's `manifest.json` declares the range of contract versions it supports: @@ -415,6 +415,46 @@ class PrintersPlugin(BasePlugin): ] ``` +### `get_settings_defaults() -> List[Dict]` (0.16.0) + +Declares the `Setting` rows this plugin owns. Return `[]` (the default) if it +owns none. Each entry is a dict: + +| Key | Meaning | +|-----|---------| +| `key` | the Setting key | +| `value` | default value in string form | +| `valuetype` | `'string'` / `'boolean'` / `'integer'` / `'json'` | +| `category` | grouping the plugin's settings page filters on | +| `description` | what the setting does | +| `public` | `True` if an unauthenticated caller may read it; default `False` | + +```python +class PrintedpartsPlugin(BasePlugin): + def get_settings_defaults(self): + return [ + {'key': 'printedparts_label_prefix', 'value': '', 'valuetype': 'string', + 'category': 'printedparts', 'public': True, + 'description': 'Leading text on the physical labels, shown at the kiosk'}, + ] +``` + +The framework seeds declared keys at install, at enable, and on every +`flask plugin upgrade-all`, so a key added in a later plugin version reaches a +site that installed an earlier one. Existing values are never overwritten. + +Declaring a key is also what tells the settings API which category and type to +use when an admin's save creates the row for the first time. Do not seed +settings by hand in `on_install` / `on_enable`: those hooks fire only on a state +transition, so a hand-seeded key added later never reaches an existing site, and +the row the first save creates lands in the placeholder `plugin` category where +the plugin's own settings page (which filters by category) cannot see it. + +`public: True` puts the key on the unauthenticated read allowlist of +`GET /api/settings/` and `GET /api/settings`. Use it only for cosmetic +values that a page rendering before login needs (a kiosk, a print page). Never +mark a credential, a hostname, or an integration URL public. + ### `get_collector_schema() -> Optional[Dict]` Declares the JSON Schema for an external collector pushing to `/api/collector/`. See [ADR-006](../docs/adr/ADR-006-collector-contract.md) for the contract. diff --git a/plugins/printedparts/manifest.json b/plugins/printedparts/manifest.json index dfb0231..1343ef2 100644 --- a/plugins/printedparts/manifest.json +++ b/plugins/printedparts/manifest.json @@ -5,7 +5,7 @@ "display_name": "3D Printed Parts", "author": "", "dependencies": ["employees"], - "core_version": ">=0.12.0,<1.0.0", + "core_version": ">=0.16.0,<1.0.0", "api_prefix": "/api/printedparts", "default_enabled": false } diff --git a/plugins/printedparts/plugin.py b/plugins/printedparts/plugin.py index b3473bc..1ceaa15 100644 --- a/plugins/printedparts/plugin.py +++ b/plugins/printedparts/plugin.py @@ -14,7 +14,6 @@ from typing import List, Optional, Type from flask import Flask, Blueprint from shopdb.plugins.base import BasePlugin, PluginMeta -from shopdb.api import db, Setting from .models import PrintedItem, PrintedItemTransaction, PrintedItemFile from .api import printedparts_bp @@ -112,43 +111,79 @@ class PrintedpartsPlugin(BasePlugin): ] def on_install(self, app: Flask) -> None: - with app.app_context(): - self._seed_settings() - logger.info('Printedparts plugin installed') + logger.info('Printedparts plugin installed') - def on_enable(self, app: Flask) -> None: - # Idempotent re-seed so settings added in later versions reach sites - # that installed earlier (enable runs on every upgrade cycle). - with app.app_context(): - self._seed_settings() - - def _seed_settings(self) -> None: - defaults = [ - ('printedparts_code_prefix', '3DP', 'string', - 'Prefix for generated item codes'), - ('printedparts_label_prefix', '', 'string', - 'Leading text on the physical gage-lab labels, shown at the kiosk ' - 'before the number box so operators type only the digits. Empty ' - 'shows no prefix. Site-specific: West Jefferson labels read WJ'), - ('printedparts_default_threshold', '5', 'integer', - 'Default low-stock threshold for new items'), - ('printedparts_unknown_badge', 'deny', 'string', - 'Kiosk policy when a badge resolves to no employee: allow or deny'), - ('printedparts_alert_email', '', 'string', - 'Comma-separated low-stock alert recipients; empty uses the ' - 'site alert_recipients'), - ('printedparts_alert_userids', '', 'string', - 'Comma-separated shopdb user ids whose account emails receive ' - 'low-stock alerts'), - ('printedparts_alert_roleids', '', 'string', - 'Comma-separated role ids; every active member of these roles ' - 'receives low-stock alerts'), - ('printedparts_alert_supportteamid', '', 'string', - 'Support team whose webhook receives low-stock alerts; empty uses ' - 'the site alert_webhook_url'), + def get_settings_defaults(self) -> List[dict]: + # The framework seeds these at install, at enable, and on every + # `flask plugin upgrade-all`, so a key added in a later version reaches + # a site that installed an earlier one. + return [ + { + 'key': 'printedparts_code_prefix', + 'value': '3DP', + 'valuetype': 'string', + 'category': 'printedparts', + 'description': 'Prefix for generated item codes', + }, + { + 'key': 'printedparts_label_prefix', + 'value': '', + 'valuetype': 'string', + 'category': 'printedparts', + # Public: the parts kiosk runs logged out, so the anonymous + # read has to be allowed or the prefix never renders there. + 'public': True, + 'description': 'Leading text on the physical gage-lab labels, ' + 'shown at the kiosk before the number box so ' + 'operators type only the digits. Empty shows no ' + 'prefix. Site-specific: West Jefferson labels ' + 'read WJ', + }, + { + 'key': 'printedparts_default_threshold', + 'value': '5', + 'valuetype': 'integer', + 'category': 'printedparts', + 'description': 'Default low-stock threshold for new items', + }, + { + 'key': 'printedparts_unknown_badge', + 'value': 'deny', + 'valuetype': 'string', + 'category': 'printedparts', + 'description': 'Kiosk policy when a badge resolves to no ' + 'employee: allow or deny', + }, + { + 'key': 'printedparts_alert_email', + 'value': '', + 'valuetype': 'string', + 'category': 'printedparts', + 'description': 'Comma-separated low-stock alert recipients; ' + 'empty uses the site alert_recipients', + }, + { + 'key': 'printedparts_alert_userids', + 'value': '', + 'valuetype': 'string', + 'category': 'printedparts', + 'description': 'Comma-separated shopdb user ids whose account ' + 'emails receive low-stock alerts', + }, + { + 'key': 'printedparts_alert_roleids', + 'value': '', + 'valuetype': 'string', + 'category': 'printedparts', + 'description': 'Comma-separated role ids; every active member ' + 'of these roles receives low-stock alerts', + }, + { + 'key': 'printedparts_alert_supportteamid', + 'value': '', + 'valuetype': 'string', + 'category': 'printedparts', + 'description': 'Support team whose webhook receives low-stock ' + 'alerts; empty uses the site alert_webhook_url', + }, ] - for key, value, valuetype, description in defaults: - if Setting.get(key) is None: - Setting.set(key, value, valuetype=valuetype, - category='printedparts', description=description) - db.session.commit() diff --git a/plugins/printers/manifest.json b/plugins/printers/manifest.json index bb043cc..9d45007 100644 --- a/plugins/printers/manifest.json +++ b/plugins/printers/manifest.json @@ -4,7 +4,7 @@ "description": "Printer management plugin with Zabbix integration, supply tracking, and QR codes", "author": "ShopDB Team", "dependencies": [], - "core_version": ">=0.1.0,<1.0.0", + "core_version": ">=0.16.0,<1.0.0", "api_prefix": "/api/printers", "provides": { "machine_category": "Printer", diff --git a/plugins/printers/plugin.py b/plugins/printers/plugin.py index 153fa7a..fdabfd6 100644 --- a/plugins/printers/plugin.py +++ b/plugins/printers/plugin.py @@ -9,7 +9,7 @@ from flask import Flask, Blueprint import click from shopdb.plugins.base import BasePlugin, PluginMeta -from shopdb.api import db, AssetType, Setting +from shopdb.api import db, AssetType from .models import ( Printer, PrinterType, ModelSupply, PrinterDriver, PrinterSupplyAlert @@ -106,40 +106,65 @@ class PrintersPlugin(BasePlugin): with app.app_context(): self._ensure_asset_type() self._ensure_printer_types() - self._seed_settings() logger.info("Printers plugin installed") - def on_enable(self, app: Flask) -> None: - # Idempotent re-seed so settings added in later versions reach sites - # that installed earlier (enable runs on every upgrade cycle). - with app.app_context(): - self._seed_settings() + def get_settings_defaults(self) -> List[dict]: + """Low-toner alert settings. - def _seed_settings(self) -> None: - """Seed low-toner alert recipient settings (idempotent).""" - defaults = [ - ('printers_alert_email', '', 'string', - 'Comma-separated low-toner alert recipients; empty uses the ' - 'site alert_recipients'), - ('printers_alert_userids', '', 'string', - 'Comma-separated shopdb user ids whose account emails receive ' - 'low-toner alerts'), - ('printers_alert_roleids', '', 'string', - 'Comma-separated role ids; every active member of these roles ' - 'receives low-toner alerts'), - ('printers_alert_supportteamid', '', 'string', - 'Support team whose webhook receives low-toner alerts; empty ' - 'uses the site alert_webhook_url'), - ('printers_alert_warning_threshold', '5', 'integer', - 'Toner percent remaining at or below which a warning email fires'), - ('printers_alert_critical_threshold', '0', 'integer', - 'Toner percent remaining at or below which a critical email fires'), + The framework seeds these at install, at enable, and on every + `flask plugin upgrade-all`, so a key added in a later version reaches a + site that installed an earlier one. + """ + return [ + { + 'key': 'printers_alert_email', + 'value': '', + 'valuetype': 'string', + 'category': 'printers', + 'description': 'Comma-separated low-toner alert recipients; ' + 'empty uses the site alert_recipients', + }, + { + 'key': 'printers_alert_userids', + 'value': '', + 'valuetype': 'string', + 'category': 'printers', + 'description': 'Comma-separated shopdb user ids whose account ' + 'emails receive low-toner alerts', + }, + { + 'key': 'printers_alert_roleids', + 'value': '', + 'valuetype': 'string', + 'category': 'printers', + 'description': 'Comma-separated role ids; every active member ' + 'of these roles receives low-toner alerts', + }, + { + 'key': 'printers_alert_supportteamid', + 'value': '', + 'valuetype': 'string', + 'category': 'printers', + 'description': 'Support team whose webhook receives low-toner ' + 'alerts; empty uses the site alert_webhook_url', + }, + { + 'key': 'printers_alert_warning_threshold', + 'value': '5', + 'valuetype': 'integer', + 'category': 'printers', + 'description': 'Toner percent remaining at or below which a ' + 'warning email fires', + }, + { + 'key': 'printers_alert_critical_threshold', + 'value': '0', + 'valuetype': 'integer', + 'category': 'printers', + 'description': 'Toner percent remaining at or below which a ' + 'critical email fires', + }, ] - for key, value, valuetype, description in defaults: - if Setting.get(key) is None: - Setting.set(key, value, valuetype=valuetype, - category='printers', description=description) - db.session.commit() def _ensure_asset_type(self) -> None: """Ensure printer asset type exists.""" diff --git a/shopdb/__init__.py b/shopdb/__init__.py index c428366..82f1821 100644 --- a/shopdb/__init__.py +++ b/shopdb/__init__.py @@ -36,7 +36,13 @@ from .plugins import plugin_manager # unattended endpoints (e.g. the GE-Enforce fetch API) can authorize a scoped # managed service token without importing core token internals. Additive name # on the import surface, minor bump. -__contract_version__ = '0.15.0' +# 0.16.0: added the get_settings_defaults hook so a plugin declares the Setting +# rows it owns (key, value, type, category, description, public). The framework +# seeds them at install, at enable, and on `flask plugin upgrade-all`, files a +# first-time write under the declared category, and lets a plugin mark a key +# readable without auth for pages that run logged out. Additive optional hook, +# minor bump. +__contract_version__ = '0.16.0' # Product release version (see ADR-007). The product version and the # plugin-contract version above are distinct series with independent diff --git a/shopdb/core/api/settings.py b/shopdb/core/api/settings.py index a356686..2f1acdb 100644 --- a/shopdb/core/api/settings.py +++ b/shopdb/core/api/settings.py @@ -52,6 +52,8 @@ SECRET_MASK = '********' # frontend/src/utils/siteSettings.js + mapConfig.js + setupState.js read before # login. Whole categories that are purely presentation are allowed wholesale; # the rest are named keys so a new integration key does not leak by default. +# A plugin adds its own public keys by declaring public=True in +# get_settings_defaults - core does not carry a list of every plugin's keys. PUBLIC_SETTING_CATEGORIES = {'branding', 'map'} PUBLIC_SETTING_KEYS = { 'site_base_url', 'facility_name', 'printer_hostname_template', @@ -60,9 +62,25 @@ PUBLIC_SETTING_KEYS = { } +def _plugin_declared_settings() -> dict: + """Declared settings ({key: entry}) of every loaded plugin, or {}.""" + pm = current_app.extensions.get('plugin_manager') + if not pm: + return {} + try: + return pm.get_declared_plugin_settings() + except Exception: + current_app.logger.exception('Could not read plugin setting declarations') + return {} + + def _is_public_setting(setting) -> bool: - return (setting.category in PUBLIC_SETTING_CATEGORIES - or setting.key in PUBLIC_SETTING_KEYS) + # A plugin declares its own public keys (get_settings_defaults, public=True) + # so a kiosk or print page that renders before login can read them without + # core carrying a list of every plugin's keys. + if setting.category in PUBLIC_SETTING_CATEGORIES or setting.key in PUBLIC_SETTING_KEYS: + return True + return bool(_plugin_declared_settings().get(setting.key, {}).get('public')) # Optional asset identifiers and the asset types they can be toggled on. # Drives per-type seed keys and the Settings matrix UI. The asset type names @@ -92,6 +110,17 @@ SEARCH_DOMAINS = { 'subnet': 'Subnets', } +def _declared_default(key: str) -> dict: + """Return the declared default for a key (core defaults, then plugins). + + Empty dict when nobody declares it - a genuinely ad-hoc key. + """ + for entry in build_default_settings(): + if entry['key'] == key: + return entry + return _plugin_declared_settings().get(key, {}) + + def _is_secret(key: str) -> bool: return 'password' in key or 'token' in key or 'secret' in key @@ -256,9 +285,19 @@ def update_setting(key: str): setting = Setting.query.filter_by(key=key).first() # Upsert: create the row on first write (e.g. plugin config keys the setup - # wizard saves). New keys default to a plugin-scoped string setting. + # wizard saves). Take the category, type and description from whoever + # declares the key - core defaults or a plugin's get_settings_defaults. + # Filing a declared key under a placeholder category hid it from the + # settings page that had just written it, which read as "did not save". if not setting: - setting = Setting(key=key, value='', valuetype='string', category='plugin') + declared = _declared_default(key) + setting = Setting( + key=key, + value='', + valuetype=declared.get('valuetype', 'string'), + category=declared.get('category', 'plugin'), + description=declared.get('description'), + ) db.session.add(setting) # Track old value for audit diff --git a/shopdb/plugins/__init__.py b/shopdb/plugins/__init__.py index 64ad849..0e35dd7 100644 --- a/shopdb/plugins/__init__.py +++ b/shopdb/plugins/__init__.py @@ -114,6 +114,11 @@ class PluginManager: per-plugin migrations extend that chain. See ADR-008. Idempotent. """ results: Dict[str, str] = {} + # Settings declared by a later plugin version reach an already-installed + # site here. on_install/on_enable fire only on a state transition, so an + # upgrade is the only moment left to seed them. Runs even with no + # migration manager - it is independent of the Alembic chains. + self.sync_all_plugin_settings() if not self.migration_manager: return results # Only ADOPTED plugins (those in the registry) get migrated. A plugin @@ -134,6 +139,50 @@ class PluginManager: results[name] = f'error: {ex}' return results + def sync_all_plugin_settings(self) -> int: + """Seed every enabled plugin's declared settings. Idempotent. + + Returns the number of plugins whose settings were touched. Best-effort + per plugin: one bad plugin must not abort a deploy's upgrade pass. + """ + touched = 0 + for name in list(self.registry.get_all().keys()): + if not self.registry.is_enabled(name): + continue + try: + plugin = self.loader.load_plugin(name, self._app, self._db) + except Exception: + logger.exception("Could not load %s to sync its settings", name) + continue + if plugin and self._seed_plugin_settings(plugin): + touched += 1 + return touched + + def get_declared_plugin_settings(self) -> Dict[str, dict]: + """Return {key: declared entry} across all loaded plugins. + + The settings API uses this to file a first-time write under the owning + plugin's category instead of a placeholder, and to decide which keys an + unauthenticated caller may read. + """ + declared: Dict[str, dict] = {} + for name, plugin in self.loader.get_all_loaded().items(): + try: + entries = plugin.get_settings_defaults() or [] + except Exception: + logger.exception("get_settings_defaults failed for %s", name) + continue + for entry in entries: + key = entry.get('key') + if key: + declared[key] = entry + return declared + + def get_public_setting_keys(self) -> set: + """Setting keys plugins declare readable without authentication.""" + return {key for key, entry in self.get_declared_plugin_settings().items() + if entry.get('public')} + def _register_plugin_components(self, plugin: BasePlugin) -> None: """Register plugin's blueprint, models, CLI commands, etc.""" # Register blueprint @@ -260,6 +309,7 @@ class PluginManager: if plugin: self._register_plugin_components(plugin) self._seed_plugin_permissions(plugin) + self._seed_plugin_settings(plugin) plugin.on_install(self._app) logger.info(f"Installed plugin: {name} v{manifest_version}") @@ -288,6 +338,58 @@ class PluginManager: "Seeded %d permission(s) for plugin %s", created, plugin.meta.name) + def _seed_plugin_settings(self, plugin: BasePlugin) -> bool: + """Idempotently create Setting rows for a plugin's declared settings. + + Runs at install, at enable, and on `flask plugin upgrade-all`. Existing + values are never overwritten - only missing rows are created, and a row + whose category does not match the declaration is re-homed. That repair + matters: before a key was declared, the first save created it under the + settings API's placeholder category, where the owning plugin's settings + page (which filters by category) could not see it again, so the value + looked like it never saved. + + Returns True if anything was created or repaired. Best-effort: a plugin + that raises must not abort the lifecycle. + """ + try: + entries = plugin.get_settings_defaults() or [] + except Exception: + logger.exception( + "get_settings_defaults failed for %s", plugin.meta.name) + return False + if not entries: + return False + from shopdb.core.models import Setting + created = 0 + repaired = 0 + with self._app.app_context(): + for entry in entries: + key = entry.get('key') + if not key: + continue + setting = Setting.query.filter_by(key=key).first() + if setting is None: + self._db.session.add(Setting( + key=key, + value=entry.get('value', ''), + valuetype=entry.get('valuetype', 'string'), + category=entry.get('category', 'plugin'), + description=entry.get('description'), + )) + created += 1 + continue + category = entry.get('category') + if category and setting.category != category: + setting.category = category + repaired += 1 + if created or repaired: + self._db.session.commit() + logger.info( + "Plugin %s settings: %d created, %d re-homed", + plugin.meta.name, created, repaired) + return bool(created or repaired) + def _is_core_tier(self, name: str) -> bool: """True when the plugin's manifest marks it tier=core (mandatory). @@ -406,6 +508,7 @@ class PluginManager: plugin = self.loader.load_plugin(name, self._app, self._db) if plugin: self._seed_plugin_permissions(plugin) + self._seed_plugin_settings(plugin) plugin.on_enable(self._app) except Exception: logger.exception(f"on_enable hook failed for plugin {name}") diff --git a/shopdb/plugins/base.py b/shopdb/plugins/base.py index b423ece..bb4ebc2 100644 --- a/shopdb/plugins/base.py +++ b/shopdb/plugins/base.py @@ -104,6 +104,28 @@ class BasePlugin(ABC): """ return [] + def get_settings_defaults(self) -> List[Dict]: + """Declare the Setting rows this plugin owns. + + The framework seeds these at install, at enable, and on every + `flask plugin upgrade-all`, so a setting added in a later plugin + version reaches a site that installed an earlier one. Declaring a key + here is also what tells the settings API which category and type the + key belongs to, so a save never has to invent one. + + Each entry is a dict: + key - the Setting key + value - default value (string form) + valuetype - 'string' | 'boolean' | 'integer' | 'json' + category - grouping the plugin's settings page filters on + description - what the setting does + public - True if an UNAUTHENTICATED caller may read it (kiosk + and print pages render before login); default False. + Never mark a credential or an integration URL public. + Return [] (default) if the plugin owns no settings. + """ + return [] + def get_setting(self, key: str, default=None): """Read a plugin-scoped setting from the core Setting store. diff --git a/shopdb/plugins/cli.py b/shopdb/plugins/cli.py index ca6709b..e4bd81e 100644 --- a/shopdb/plugins/cli.py +++ b/shopdb/plugins/cli.py @@ -869,7 +869,10 @@ def upgrade_all_plugins(): Idempotent. Run this after `flask db upgrade` on every deploy and upgrade. It stamps each bundled plugin's anchor revision into alembic_version_ and applies any per-plugin migrations added - after the ownership cutover (ADR-008). Safe to re-run at head. + after the ownership cutover (ADR-008). It also seeds any settings a + plugin declares (get_settings_defaults) that this site is missing, so a + setting added in a later version reaches a site that installed earlier. + Safe to re-run at head. """ pm = current_app.extensions.get('plugin_manager') if not pm: diff --git a/tests/test_core/test_settings_plugin_declared.py b/tests/test_core/test_settings_plugin_declared.py new file mode 100644 index 0000000..59ea063 --- /dev/null +++ b/tests/test_core/test_settings_plugin_declared.py @@ -0,0 +1,148 @@ +"""Tests for plugin-declared settings (contract 0.16.0). + +Three regressions are pinned here, all found on the 3D-parts kiosk label +prefix: + +1. The kiosk runs logged out, so a setting it renders must be readable by an + anonymous caller when the owning plugin declares it public - and only then. +2. A save that creates the row for the first time must file it under the + declared category, or the plugin's own settings page (which lists by + category) stops seeing the value and the save looks lost. +3. Settings declared by a later plugin version must reach a site that installed + an earlier one; on_install/on_enable fire only on a state transition, so the + upgrade pass has to seed them. +""" + +import pytest + +from shopdb.core.models import Setting + +KIOSK_KEY = 'printedparts_label_prefix' +PRIVATE_KEY = 'printedparts_alert_email' + + +def _plugin(app, name='printedparts'): + pm = app.extensions.get('plugin_manager') + plugin = pm.loader.get_loaded_plugin(name) if pm else None + if plugin is None: + pytest.skip(f'{name} plugin not loaded in this build') + return pm, plugin + + +def test_plugin_declares_the_kiosk_prefix_public(app): + _, plugin = _plugin(app) + declared = {entry['key']: entry for entry in plugin.get_settings_defaults()} + assert declared[KIOSK_KEY]['public'] is True + assert declared[KIOSK_KEY]['category'] == 'printedparts' + # Everything else the plugin owns stays behind auth. + assert not any(entry.get('public') for key, entry in declared.items() + if key != KIOSK_KEY) + + +def test_anon_can_read_a_public_plugin_setting(client, db): + """The kiosk reads this with no token; a 404 leaves the prefix blank.""" + Setting.set(KIOSK_KEY, 'WJ', valuetype='string', category='printedparts') + + resp = client.get(f'/api/settings/{KIOSK_KEY}') + + assert resp.status_code == 200, resp.get_json() + assert resp.get_json()['data']['value'] == 'WJ' + + +def test_anon_cannot_read_a_nonpublic_plugin_setting(client, db): + Setting.set(PRIVATE_KEY, 'lead@example.com', valuetype='string', + category='printedparts') + + resp = client.get(f'/api/settings/{PRIVATE_KEY}') + + assert resp.status_code == 404 + body = resp.get_data(as_text=True) + assert 'lead@example.com' not in body + + +def test_anon_list_includes_public_plugin_settings_only(client, db): + Setting.set(KIOSK_KEY, 'WJ', valuetype='string', category='printedparts') + Setting.set(PRIVATE_KEY, 'lead@example.com', valuetype='string', + category='printedparts') + + resp = client.get('/api/settings?category=printedparts') + + assert resp.status_code == 200 + keys = {row['key'] for row in resp.get_json()['data']} + assert keys == {KIOSK_KEY} + + +def test_first_save_files_the_key_under_its_declared_category(client, db, + auth_headers): + """Save then reload, the way the settings page does it. + + The row does not exist yet (the site installed before the key was added), + so the PUT creates it. Filed under a placeholder category it would vanish + from the page's category-filtered reload - the "it does not save" bug. + """ + assert Setting.query.filter_by(key=KIOSK_KEY).first() is None + + resp = client.put(f'/api/settings/{KIOSK_KEY}', json={'value': 'WJ'}, + headers=auth_headers) + assert resp.status_code == 200, resp.get_json() + + setting = Setting.query.filter_by(key=KIOSK_KEY).first() + assert setting.category == 'printedparts' + + reload = client.get('/api/settings?category=printedparts', + headers=auth_headers) + values = {row['key']: row['value'] for row in reload.get_json()['data']} + assert values[KIOSK_KEY] == 'WJ' + + +def test_first_save_of_a_core_key_keeps_its_core_category(client, db, + auth_headers): + resp = client.put('/api/settings/facility_name', + json={'value': 'Test Plant'}, headers=auth_headers) + assert resp.status_code == 200, resp.get_json() + + assert Setting.query.filter_by(key='facility_name').first().category == 'site' + + +def test_undeclared_key_still_upserts(client, db, auth_headers): + """An ad-hoc key nobody declares keeps the old placeholder behavior.""" + resp = client.put('/api/settings/some_adhoc_key', json={'value': 'x'}, + headers=auth_headers) + + assert resp.status_code == 200, resp.get_json() + assert Setting.query.filter_by(key='some_adhoc_key').first().category == 'plugin' + + +def test_upgrade_seeds_settings_missing_from_an_existing_site(app, db): + """`flask plugin upgrade-all` reaches a site that installed earlier.""" + pm, _ = _plugin(app) + assert Setting.query.filter_by(key=KIOSK_KEY).first() is None + + pm.sync_all_plugin_settings() + + setting = Setting.query.filter_by(key=KIOSK_KEY).first() + assert setting is not None + assert setting.category == 'printedparts' + assert setting.value == '' + + +def test_sync_never_overwrites_a_configured_value(app, db): + pm, _ = _plugin(app) + Setting.set(KIOSK_KEY, 'WJ', valuetype='string', category='printedparts') + + pm.sync_all_plugin_settings() + + assert Setting.query.filter_by(key=KIOSK_KEY).first().value == 'WJ' + + +def test_sync_rehomes_a_row_left_in_the_placeholder_category(app, db): + """Repairs rows an earlier save created before the key was declared.""" + pm, _ = _plugin(app) + Setting.set(PRIVATE_KEY, 'lead@example.com', valuetype='string', + category='plugin') + + pm.sync_all_plugin_settings() + + setting = Setting.query.filter_by(key=PRIVATE_KEY).first() + assert setting.category == 'printedparts' + assert setting.value == 'lead@example.com'