[6.x] Keep config file overrides out of saved plugin settings

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+- Fixed a bug where saving a plugin’s settings stored values from its `config/craft/<handle>.php` file in the project config, replacing the values previously saved from the control panel. ([#20624](https://github.com/craftcms/cms/pull/20624))
 - Fixed a bug where Yii HTTP exceptions, such as `yii\web\NotFoundHttpException`, thrown by legacy site controller actions resulted in a 500 error rather than their own status code. ([#20614](https://github.com/craftcms/cms/issues/20614), [#20617](https://github.com/craftcms/cms/pull/20617))
 - Fixed a bug where fields could be saved with the same handle as another field. ([#20600](https://github.com/craftcms/cms/issues/20600), [#20607](https://github.com/craftcms/cms/pull/20607))
 - Fixed an error that occurred when changing a password from the **My Account** → **Password** screen. ([#20585](https://github.com/craftcms/cms/pull/20585))

src/Plugin/Plugins.php

@@ -743,11 +743,18 @@ public function savePluginSettings(PluginInterface $plugin, array $settings): bo
             return false;
         }
 
-        // Update the plugin’s settings in the project config
-        $pluginSettings = ProjectConfigHelper::packAssociativeArrays($pluginSettings->validationData());
-        app(ProjectConfig::class)->set(
-            path: ProjectConfig::PATH_PLUGINS.'.'.$plugin->handle.'.settings',
-            value: $pluginSettings,
+        // Update the plugin’s settings in the project config, keeping the stored values of
+        // settings that are overridden by config/craft/<handle>.php
+        $projectConfig = app(ProjectConfig::class);
+        $path = ProjectConfig::PATH_PLUGINS.'.'.$plugin->handle.'.settings';
+        $overrides = Config::get("craft.$plugin->handle", []);
+
+        $projectConfig->set(
+            path: $path,
+            value: [
+                ...array_diff_key(ProjectConfigHelper::packAssociativeArrays($pluginSettings->validationData()), $overrides),
+                ...array_intersect_key($projectConfig->get($path) ?? [], $overrides),
+            ],
             message: "Change settings for plugin “{$plugin->handle}”",
         );
 

tests/Feature/Plugin/PluginConfigurationTest.php

@@ -115,18 +115,16 @@ function configurationTestPlugins(): Plugins
     $this->assertDatabaseMissing(Table::PLUGINS, ['handle' => 'uninstalled-plugin']);
 });
 
-it('flushes submitted overrides and omitted configuration values to the database on the test connection', function () {
-    $input = FluentTestPlugin::config()->foo('File foo')->bar('File bar');
-    Config::set('craft.fluent-test-plugin', $input);
-    new ConfigServiceProvider(app())->register();
+it('flushes submitted settings but not configuration overrides to the database on the test connection', function () {
+    Config::set('craft.fluent-test-plugin', ['bar' => 'File bar']);
     $plugin = $this->plugins->getPlugin('fluent-test-plugin');
     $settings = $plugin->getSettings();
     $projectConfig = app(ProjectConfig::class);
 
     expect($this->plugins->savePluginSettings($plugin, ['foo' => 'Submitted']))->toBeTrue();
     expect($plugin->getSettings())->toBe($settings)
         ->and($settings->validationData())->toBe(['foo' => 'Submitted', 'bar' => 'File bar'])
-        ->and($projectConfig->get('plugins.fluent-test-plugin.settings'))->toBe(['bar' => 'File bar', 'foo' => 'Submitted']);
+        ->and($projectConfig->get('plugins.fluent-test-plugin.settings'))->toBe(['bar' => 'Stored bar', 'foo' => 'Submitted']);
     expect(DB::table(Table::PROJECTCONFIG)
         ->whereIn('path', ['plugins.fluent-test-plugin.settings.foo', 'plugins.fluent-test-plugin.settings.bar'])
         ->orderBy('path')->pluck('value', 'path')->all())->toBe([
@@ -139,18 +137,16 @@ function configurationTestPlugins(): Plugins
     expect(DB::table(Table::PROJECTCONFIG)
         ->whereIn('path', ['plugins.fluent-test-plugin.settings.foo', 'plugins.fluent-test-plugin.settings.bar'])
         ->orderBy('path')->pluck('value', 'path')->all())->toBe([
-            'plugins.fluent-test-plugin.settings.bar' => '"File bar"',
+            'plugins.fluent-test-plugin.settings.bar' => '"Stored bar"',
             'plugins.fluent-test-plugin.settings.foo' => '"Submitted"',
         ]);
 
     $projectConfig->reset();
 
-    expect($projectConfig->get('plugins.fluent-test-plugin.settings'))->toBe(['bar' => 'File bar', 'foo' => 'Submitted'])
-        ->and($this->plugins->getStoredPluginInfo('fluent-test-plugin')['settings'])->toBe(['bar' => 'Stored bar', 'foo' => 'Stored foo'])
+    expect($projectConfig->get('plugins.fluent-test-plugin.settings'))->toBe(['bar' => 'Stored bar', 'foo' => 'Submitted'])
         ->and($plugin->getSettings())->toBe($settings)
         ->and($settings->foo)->toBe('Submitted')
-        ->and(Config::get('craft.fluent-test-plugin'))->toBe(['foo' => 'File foo', 'bar' => 'File bar'])
-        ->and($input->configData())->toBe(['foo' => 'File foo', 'bar' => 'File bar']);
+        ->and($settings->bar)->toBe('File bar');
 });
 
 it('does not write invalid settings to the database when project config is flushed', function () {

tests/Feature/Plugin/PluginsTest.php

@@ -546,7 +546,7 @@ protected static function createSettings(): SnapshotPluginSettings
     expect($this->plugins->createPlugin('test-plugin')->getSettings()->foo)->toBeNull();
 });
 
-it('saves submitted overrides and omitted file values without mutating configuration input', function () {
+it('saves submitted settings without overridden file values or mutating configuration input', function () {
     app()->offsetUnset(TestPlugin::class);
     $input = new TestPluginSettings()->foo('File foo')->bar('File bar');
     Config::set('craft.test-plugin', $input);
@@ -558,12 +558,26 @@ protected static function createSettings(): SnapshotPluginSettings
 
     expect($plugin->getSettings())->toBe($settings)
         ->and($settings->validationData())->toBe(['foo' => 'Submitted', 'bar' => 'File bar'])
-        ->and(app(ProjectConfig::class)->get('plugins.test-plugin.settings'))->toBe(['bar' => 'File bar', 'foo' => 'Submitted'])
+        ->and(app(ProjectConfig::class)->get('plugins.test-plugin.settings'))->toBe([])
         ->and(Config::get('craft.test-plugin'))->toBe(['foo' => 'File foo', 'bar' => 'File bar'])
         ->and($input->validationData())->toBe(['foo' => 'File foo', 'bar' => 'File bar']);
 });
 
-it('saves full validation data even when configuration data is customized', function () {
+it('keeps stored values of settings overridden by configuration when saving', function (array $stored, array $expected) {
+    app(ProjectConfig::class)->set('plugins.test-plugin.settings', $stored);
+    Config::set('craft.test-plugin', ['bar' => 'File bar']);
+    $plugin = $this->plugins->createPlugin('test-plugin', ['settings' => $stored]);
+
+    expect($this->plugins->savePluginSettings($plugin, ['foo' => 'Submitted', 'bar' => 'File bar']))->toBeTrue();
+
+    expect($plugin->getSettings()->validationData())->toBe(['foo' => 'Submitted', 'bar' => 'File bar'])
+        ->and(app(ProjectConfig::class)->get('plugins.test-plugin.settings'))->toBe($expected);
+})->with([
+    'stored' => [['foo' => 'CP foo', 'bar' => 'CP bar'], ['bar' => 'CP bar', 'foo' => 'Submitted']],
+    'not stored' => [['foo' => 'CP foo'], ['foo' => 'Submitted']],
+]);
+
+it('saves validation data outside the configured settings when configuration data is customized', function () {
     app()->offsetUnset(TestPlugin::class);
     $plugin = new class(app()) extends TestPlugin
     {
@@ -591,7 +605,7 @@ public function configData(): array
 
     expect($plugin->getSettings()->validationData())->toBe(['foo' => 'Submitted', 'bar' => 'Saved bar'])
         ->and($plugin->getSettings()->configData())->toBe(['foo' => 'Config foo'])
-        ->and(app(ProjectConfig::class)->get('plugins.test-plugin.settings'))->toBe(['bar' => 'Saved bar', 'foo' => 'Submitted'])
+        ->and(app(ProjectConfig::class)->get('plugins.test-plugin.settings'))->toBe(['bar' => 'Saved bar'])
         ->and(Config::get('craft.test-plugin'))->toBe(['foo' => 'Config foo'])
         ->and($input->validationData())->toBe(['foo' => 'Input foo', 'bar' => 'Input bar']);
 });