diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fe4b53e2db..1e09c1fc14d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Fixed a bug where saving a plugin’s settings stored values from its `config/craft/.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)) diff --git a/src/Plugin/Plugins.php b/src/Plugin/Plugins.php index 0e0ec25ba02..dd6a66edfa1 100644 --- a/src/Plugin/Plugins.php +++ b/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/.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}”", ); diff --git a/tests/Feature/Plugin/PluginConfigurationTest.php b/tests/Feature/Plugin/PluginConfigurationTest.php index eec90a3f676..91d7cb3bfd9 100644 --- a/tests/Feature/Plugin/PluginConfigurationTest.php +++ b/tests/Feature/Plugin/PluginConfigurationTest.php @@ -115,10 +115,8 @@ 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); @@ -126,7 +124,7 @@ function configurationTestPlugins(): Plugins 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 () { diff --git a/tests/Feature/Plugin/PluginsTest.php b/tests/Feature/Plugin/PluginsTest.php index 0b62b493a46..a54ab5aec57 100644 --- a/tests/Feature/Plugin/PluginsTest.php +++ b/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']); });