fix(settings): implement settings#update — PUT /api/settings returned 500

[rubenvdlinde]

The defect

\OCA\OpenRegister\AppHost\Routes::standard() ships the canonical route
['name' => 'settings#update', 'url' => '/api/settings', 'verb' => 'PUT'].
decidesk's appinfo/routes.php calls Routes::standard($extra) behind a
class_exists() guard and its openregister-absent fallback re-declares
settings#update verbatim (decidesk#377) — so the route is live on both
code paths.

AppHost\Bootstrap::aliasControllerUnlessLeafDefinesIt() only substitutes
OpenRegister's GenericSettingsController when the leaf does not ship a
class of that name. decidesk does ship lib/Controller/SettingsController.php,
so the alias is skipped and decidesk owes every method the canonical table
routes to settings#. It had index / create / load /
getPublicationConfig / setPublicationConfig — but no update().

The router matches the URL and the dispatcher reflects the method, so this is a
500, not a 404.

Pre-fix failure evidence (measured 2026-08-08, dev instance, NC 34.0.0.12)

curl -s -o /dev/null -w "%{http_code}" -X GET  -u admin:admin http://localhost:8080/apps/decidesk/api/settings
200      <- positive control: route + auth + controller all resolve

curl -s -o /dev/null -w "%{http_code}" -X PUT  -u admin:admin http://localhost:8080/apps/decidesk/api/settings
500

nextcloud.log, same reqId:

"method":"PUT","url":"/apps/decidesk/api/settings",
"message":"Method OCA\\Decidesk\\Controller\\SettingsController::update() does not exist",
"exception":{"Exception":"ReflectionException", ...
  ControllerMethodReflector.php:40 -> Dispatcher.php:68 -> App.php:137
  -> Router.php:324  {"_route":"decidesk.settings.update"}

What update() writes, and why that matches decidesk's settings surface

update() is the existing create() body, moved unchanged:

$data   = $this->request->getParams();
$config = $this->settingsService->updateSettings($data);
return new JSONResponse(['success' => true, 'config' => $config]);

SettingsService::updateSettings() writes the whitelisted CONFIG_KEYS
(register, ORI endpoint, email-voting flag, the three schema slugs, the
organisation name/logo/timezone/locale/currency/retention keys,
organisatie_modus, …) into the instance-wide IAppConfig, then returns
getSettings() — the refreshed map, with SECRET_KEYS omitted (they are
write-only and never echoed). So update() returns what was actually stored,
not the submission. That is exactly the canonical
GenericSettingsControllerBase::update() shape.

getPublicationConfig / setPublicationConfig were deliberately NOT folded
in. They live on a different URL (/api/settings/publication-config), are
backed by a different service (PublicationConfigService, per-governance-body
records), and their keys are not in SettingsService::CONFIG_KEYS — the
app's own settings payload genuinely does not include them. Folding them in
would change the response shape of both create() and update().

Auth posture — and why

update() carries #[AuthorizedAdminSetting(AdminSettings::class)], the same
posture as create(), because it is literally the same write path.

Reasoning:

• SettingsService::updateSettings() writes instance-wide IAppConfig, not
  per-user state. Any authenticated user reaching it could repoint the app's
  register, schema slugs and ORI endpoint for everybody.
• The sibling read methods on this controller (index(),
  getPublicationConfig()) carry #[NoAdminRequired] plus an in-body session
  check. That posture was not copied. Copying a read's #[NoAdminRequired]
  onto a write is exactly how docudesk nearly shipped a PII hole.
• create() keeps its own attribute even though its body is now
  return $this->update();. Nextcloud's middleware evaluates auth attributes
  on the dispatched method only, so delegation inherits nothing. create()
  had no in-body guard to move — the whole guard is the attribute — and the
  delegate path is therefore still guarded at both entry points.

A unit test (testBothWriteMethodsRequireAdmin) asserts each write method
carries AuthorizedAdminSetting and does not carry NoAdminRequired, so a
future posture regression on either entry point is caught.

Can-fail proof

The pre-fix tree was reconstructed by hand with the Edit tool (update()
removed, create()'s original inline body restored) — not git stash, not
git checkout --.

RED (pre-fix tree):

PHPUnit 10.5.63 ... .F.E.EE   7 / 7 (100%)

1) SettingsControllerWriteTest::testEmptySubmissionStillReachesTheService
Error: Call to undefined method OCA\Decidesk\Controller\SettingsController::update()

2) SettingsControllerWriteTest::testBothWriteMethodsRequireAdmin
ReflectionException: Method OCA\Decidesk\Controller\SettingsController::update() does not exist

3) SettingsControllerWriteTest::testUpdatePersistsTheRequestParametersAndReturnsTheStoredConfig
Error: Call to undefined method OCA\Decidesk\Controller\SettingsController::update()

1) CanonicalRouteMethodContractTest::testLeafOwnedControllersImplementEveryCanonicalMethodRoutedToThem
The canonical AppHost route table routes to these method(s), but decidesk ships the
controller itself so no generic is aliased in. Each of these is a 500, not a 404.
  - SettingsController::update()

ERRORS! Tests: 7, Assertions: 41, Errors: 3, Failures: 1.

The reflection test reproduces the same exception text the live 500
produced. Note the two route-table tests stayed green in the red run — that
is the point: the route is declared and live, and only the method is missing.

GREEN (this branch):

PHPUnit 10.5.63 ... .......   7 / 7 (100%)
OK (7 tests, 52 assertions)

Full unit suite on this branch: OK, but some tests were skipped! Tests: 805, Assertions: 2813, Skipped: 29 (798 before this PR; the 29 skips are
pre-existing).

Tests

tests/Unit/AppInfo/CanonicalRouteMethodContractTest.php

• Asserts the item, never the container: for every canonical controller
  decidesk ships on disk, each individual routed method must exist, be public,
  and be non-static. Merely class_exists() is explicitly not enough — the
  file-on-disk probe is what makes the AppHost skip the alias, and a booted
  container would satisfy class_exists() via the alias target itself.
• Asserts the openregister-absent fallback in appinfo/routes.php still
  declares every canonical route name, and pins
  'settings#update' / '/api/settings' / 'PUT' with a regex so a name
  appearing only in a comment cannot satisfy it.
• Positive controls: $inspected > 0 on the reflection loop and
  $checked > 0 on the route-name loop, so an empty finding list cannot be
  produced vacuously by a broken path probe.

tests/Unit/Controller/SettingsControllerWriteTest.php

• update() reaches SettingsService::updateSettings() with the request's
  own params and returns the refreshed config the service stored (not the
  submission).
• create() delegates and still performs the identical write — it is not an
  empty success.
• An empty submission still reaches the service (an early return would look
  identical to a successful no-op from the caller's side).
• Auth-posture assertions on both write methods, with a $checked === 2
  positive control.

Both new methods' statements are covered, so the coverage ratchet is not
waived.

Gates

Run from a clean extract of ConductionNL/.github at origin/main (the local
.github checkout is 77 commits behind, so it is a different program), scoped
to the diff against origin/development, reading stdout, not the exit byte:

[gate-1]  spdx-headers:            PASS
[gate-3]  stub-scan:               PASS
[gate-5]  route-auth:              PASS
[gate-9]  semantic-auth:           PASS
[gate-14] route-reachability:      PASS
[gate-16] spec-coverage:           PASS
[gate-46] spec-anchor-existence:   PASS
[gate-47] security-change-has-tests: PASS

54 GATE(S) GREEN — 54 of 57 applicable gates ran.

Honest residue: 3 applicable gates did not run and their subject matter is
unverified by this run — gate-19 e2e-coverage (the diff touches no spec
file), gate-62 store-plane and gate-63 settings-surface (the diff touches
no manifest or menu-layout). This is not "all 64 green".

phpcs: 0 errors on the changed file; 1 pre-existing warning (the REUSE SPDX
header line, which the ruleset deliberately tolerates).

Not verified

The fix itself was not re-probed live: the dev instance bind-mounts the main
checkout, not this worktree, and deploying to the shared instance is out of
scope for this PR. The 500 above is the pre-fix measurement; the fix is
evidenced by the unit-level can-fail proof.

[github-actions]

Quality Report — ConductionNL/decidesk @ 35f381f

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
composer			✅	✅ 100/100
npm			✅	✅ 549/549
PHPUnit					✅
Newman					❌
Playwright					❌
Hydra gates					❌

---

Quality workflow — 2026-08-08 16:35 UTC

Download the full PDF report from the workflow artifacts.