From 716c8787ce3436b2d5f31ee2a8bd7af30dacce36 Mon Sep 17 00:00:00 2001 From: Ji Jia Jia Date: Mon, 21 Sep 2026 16:30:58 +0200 Subject: [PATCH] [Settings] Keep admin settings and config listings usable with a settings-store read target The symfony config write target is read-only once debug mode is off, so the only way to keep Appearance & Branding, perspectives and element tree widgets editable in production is to point both the read and the write target at the settings store. Doing that currently breaks them in two ways: * SettingRepository::loadConfig() destructured the result of loadConfigByKey() only in its caller, so the `!$data` fallback tested a two-element array and never fired. With an empty settings store the admin settings therefore resolved to null and every value configured in the symfony configuration was dropped, leaving an empty Appearance & Branding form. The fallback now works and returns the configured settings, still writeable because the data source stays unset. * listConfigurations() collected its keys with fetchAllKeys(), which merges the settings store ids with the symfony configuration keys, while getConfiguration() honours the read target. A single perspective or widget defined in the symfony configuration therefore made the whole listing fail with a 404. The keys are now taken from the read target when one is configured. --- .../ElementTreeWidgetConfigRepository.php | 19 ++++++++++++++++++- .../PerspectiveConfigRepository.php | 19 ++++++++++++++++++- .../Admin/Repository/SettingRepository.php | 13 +++++++++---- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/Perspective/Repository/ElementTreeWidgetConfigRepository.php b/src/Perspective/Repository/ElementTreeWidgetConfigRepository.php index 597cd7c3c..2ae792e90 100644 --- a/src/Perspective/Repository/ElementTreeWidgetConfigRepository.php +++ b/src/Perspective/Repository/ElementTreeWidgetConfigRepository.php @@ -147,7 +147,7 @@ function ($key, $data) { public function listConfigurations(): array { $configurations = []; - $keys = array_merge(ElementTreeWidgets::values(), $this->getRepository()->fetchAllKeys()); + $keys = array_merge(ElementTreeWidgets::values(), $this->getConfigurationKeys()); foreach ($keys as $key) { $configurations[] = $this->getConfiguration($key); } @@ -179,6 +179,23 @@ public function deleteConfiguration( } } + /** + * Only the keys the configured read target can actually load: getConfiguration() reads through + * the read target, so listing a key that lives in the other location makes it fail with a not + * found error and takes the whole listing down with it. Without a read target both locations + * are read, so both sets of keys belong in the listing. + * + * @throws Exception + */ + private function getConfigurationKeys(): array + { + $repository = $this->getRepository(); + + return $repository->getReadTargets() === [] + ? $repository->fetchAllKeys() + : $repository->fetchAllKeysByReadTargets(); + } + private function getRepository(): LocationAwareConfigRepository { if (!$this->repository) { diff --git a/src/Perspective/Repository/PerspectiveConfigRepository.php b/src/Perspective/Repository/PerspectiveConfigRepository.php index 4c56fddcf..3503382ef 100644 --- a/src/Perspective/Repository/PerspectiveConfigRepository.php +++ b/src/Perspective/Repository/PerspectiveConfigRepository.php @@ -88,7 +88,7 @@ function ($key, $data) { public function listConfigurations(): array { $configurations = []; - foreach ($this->getRepository()->fetchAllKeys() as $key) { + foreach ($this->getConfigurationKeys() as $key) { $configurations[] = $this->getConfiguration($key); } @@ -119,6 +119,23 @@ public function deleteConfiguration( } } + /** + * Only the keys the configured read target can actually load: getConfiguration() reads through + * the read target, so listing a key that lives in the other location makes it fail with a not + * found error and takes the whole listing down with it. Without a read target both locations + * are read, so both sets of keys belong in the listing. + * + * @throws Exception + */ + private function getConfigurationKeys(): array + { + $repository = $this->getRepository(); + + return $repository->getReadTargets() === [] + ? $repository->fetchAllKeys() + : $repository->fetchAllKeysByReadTargets(); + } + private function getRepository(): LocationAwareConfigRepository { if (!$this->repository) { diff --git a/src/Setting/Admin/Repository/SettingRepository.php b/src/Setting/Admin/Repository/SettingRepository.php index 3dc638653..2b0be99d4 100644 --- a/src/Setting/Admin/Repository/SettingRepository.php +++ b/src/Setting/Admin/Repository/SettingRepository.php @@ -77,15 +77,20 @@ private function getRepository(): LocationAwareConfigRepository */ private function loadConfig(): array { - $data = $this->getRepository()->loadConfigByKey(Configuration::ADMIN_SETTINGS_NODE); + [$data, $dataSource] = $this->getRepository()->loadConfigByKey(Configuration::ADMIN_SETTINGS_NODE); $loadType = $this->getRepository()->getReadTargets()[0] ?? null; + // The settings store only holds the admin settings once they have been saved through the UI. + // Until then the symfony configuration is their only source, so it has to serve as the + // fallback - otherwise configured branding silently disappears as soon as the read target is + // switched to the settings store, which is the only way to keep the settings writeable in a + // production environment. The data source stays unset: the settings are still written to the + // settings store, so they remain writeable. if (!$data && $loadType === LocationAwareConfigRepository::LOCATION_SETTINGS_STORE) { - $data = $this->adminConfig; - $data['writeable'] = $this->isRepositoryWritable(); + $data = $this->adminConfig[Configuration::ADMIN_SETTINGS_NODE] ?? []; } - return $data; + return [$data, $dataSource]; } /**