Show the kiosk label prefix, and let a plugin declare the settings it owns
Three defects, all found on printedparts_label_prefix, all one root cause: nothing in the framework knew that setting existed. The parts kiosk runs logged out. An unauthenticated read of a setting is limited to an allowlist, the key was not on it, so the kiosk got a 404 and fell back to no prefix. An admin previewing the same page while logged in saw the prefix, which is why it looked like it worked. 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 the placeholder category the settings API uses for keys it does not recognise, where the plugin's settings page, which lists by category, could no longer see it. The value was in the database the whole time. And the row was missing in the first place because seeding ran from on_install / on_enable, which fire only on a state transition. Neither runs again on an upgrade, so a setting added in a later plugin version never reached a site that installed an earlier one. The comment claiming enable ran every upgrade cycle was simply wrong. A plugin now declares the settings it owns in get_settings_defaults(): key, default, type, category, description, and whether a logged-out page may read it. The framework seeds declared keys at install, at enable, and on every flask plugin upgrade-all; files a first-time write under the declared category; re-homes any row left in the placeholder category, value untouched; and answers an anonymous read for keys marked public. Core carries no list of any plugin's keys. Contract 0.16.0 (additive optional hook). printedparts and printers move to the hook and floor their core_version at 0.16.0. The dev database had two rows in the misfiled state (printedparts_alert_email, employee_db_host); the first repairs itself on the next upgrade pass.
This commit is contained in:
@@ -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}")
|
||||
|
||||
Reference in New Issue
Block a user