Skip to content

Declarative settings registry: declare each ffc_settings key once #993

Description

@rpgmem

Parked with a trigger, in the style of #788 / #902. This records the full design so it isn't re-derived, and the evidence that argues for it — without committing to build it now. A cheaper guard lands first (see Why not now).

The problem

A single settings key is currently known in up to six places. Tracing cleanup_days:

Where What it declares
Settings::get_default_settings() 'cleanup_days' => 365
SettingsSaveHandler max( 1, absint( … ) ) — the sanitiser
SettingsAjaxEndpoint::allowlist() {option, path, type, cap} for autosave
SettingsReader typed accessor (where one exists) another default
every read site SettingsReader::get_int( 'cleanup_days', **365** ) — the default again
the tab view another literal

Nothing forces those to agree.

Measured evidence

At the time of writing, across includes/:

  • 33 keys declared in get_default_settings().
  • 10 of them repeat their default at a read site.
  • 5 divergences across 3 keys — and 3 of the 5 live inside SettingsReader itself, in the typed accessors CLAUDE.md tells everyone to prefer:
Key Declared Read as Where
obsolete_shortcode_days 90 30 SettingsReader accessor
qr_default_size 200 256 SettingsReader accessor
public_csv_default_limit 1 100 SettingsReader accessor
public_csv_default_limit 1 0 csv-download-validator
public_csv_default_limit 1 0 csv-download-form-info-builder
  • 17 keys are read through SettingsReader but are not declared at all — so get_default_settings() is an incomplete list, not a registry. Their only default is the read-site literal.

The shape

One declaration per key — { default, type, sanitize, cap, autosave } — with every consumer deriving from it:

  • SettingsReader::get( 'cleanup_days' ) takes no second argument; the default comes from the registry.
  • SettingsSaveHandler derives sanitisation instead of hand-writing it per key.
  • SettingsAjaxEndpoint::allowlist() derives from the autosave + cap fields.
  • uninstall.php derives its option list instead of hand-maintaining it.

Divergence stops being detectable and becomes unrepresentable.

Cost

Touches Settings, SettingsReader, SettingsSaveHandler, SettingsAjaxEndpoint, ~17 read sites, and probably the module-boundary baseline. Days, not hours.

Why not now

CLAUDE.md's bad-façade / namespace-churn trap: indirection that narrows nothing isn't worth it. A static guard asserting that every read-site default matches the declared one turns the whole class into a CI failure on the PR that introduces it — hours instead of days, no architectural change, no baseline churn. That guard lands first; with it in place the registry is a genuine improvement rather than a necessity.

Revisit trigger

Any one of:

  1. A new divergence the guard catches that cannot be fixed by aligning a literal — i.e. two consumers legitimately need different defaults for the same key.
  2. A third consumer needing the key list (uninstall + autosave allowlist exist; a third makes derivation clearly cheaper than duplication).
  3. The 17 undeclared keys becoming a live problem — a Bug: auto-delete of old submissions is silently disabled (reads unwritten ffc_cleanup_days option) #936-style dormant feature traced back to a key with no declared default.

Absent a trigger, leave it. Re-deriving this analysis is what the issue exists to prevent.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions