Skip to content

Commit

Permalink
Ensure empty plugin settings are saved correctly (#22885)
Browse files Browse the repository at this point in the history
* Send and process empty setting arrays as __empty__ value

* Add integration test for an empty settings array
  • Loading branch information
michalkleiner authored Dec 19, 2024
1 parent e3aece1 commit c51668c
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 15 deletions.
7 changes: 7 additions & 0 deletions plugins/CorePluginsAdmin/SettingsMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class SettingsMetadata
{
public const PASSWORD_PLACEHOLDER = '******';

public const EMPTY_ARRAY = '__empty__';

/**
* @param Settings[] $settingsInstances
* @param array $settingValues array('pluginName' => array('settingName' => 'settingValue'))
Expand All @@ -34,6 +36,11 @@ public function setPluginSettings($settingsInstances, $settingValues)

$fieldConfig = $setting->configureField();

// empty arrays are sent as __empty__ value, so we need to convert it here back to an array
if ($setting->getType() === FieldConfig::TYPE_ARRAY && $value === self::EMPTY_ARRAY) {
$value = [];
}

if (
isset($value) && (
$fieldConfig->uiControl !== FieldConfig::UI_CONTROL_PASSWORD ||
Expand Down
26 changes: 26 additions & 0 deletions plugins/CorePluginsAdmin/tests/Integration/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ class ApiTest extends IntegrationTestCase
],
];

private $testSystemSettingsEmptyValuePayload = [
'ExampleSettingsPlugin' => [
['name' => 'browsers', 'value' => '__empty__'],
],
];

protected static function beforeTableDataCached()
{
parent::beforeTableDataCached();
Expand Down Expand Up @@ -113,6 +119,26 @@ public function testGetSystemSettingsRedactsPasswordValues()
self::assertTrue(password_verify('newPassword', $pluginSettings->getSetting('password')->getValue()));
}

public function testSetSystemSettingsCorrectlySetsEmptyValue()
{
$pluginSettings = StaticContainer::get(\Piwik\Plugins\ExampleSettingsPlugin\SystemSettings::class);
$settings = $this->getPluginSettings('ExampleSettingsPlugin', 'browsers');
$defaultValue = ['firefox', 'chromium', 'safari'];

self::assertEquals('browsers', $settings['name']);
self::assertEquals($defaultValue, $settings['value']);
self::assertEquals($defaultValue, $pluginSettings->getSetting('browsers')->getValue());

$settingValues = $this->testSystemSettingsEmptyValuePayload;
\Piwik\Plugins\CorePluginsAdmin\API::getInstance()->setSystemSettings($settingValues, self::TEST_PASSWORD);

$newSettings = $this->getPluginSettings('ExampleSettingsPlugin', 'browsers');

self::assertEquals('browsers', $newSettings['name']);
self::assertEquals([], $newSettings['value']);
self::assertEquals([], $pluginSettings->getSetting('browsers')->getValue());
}

private function getPluginSettings(string $pluginName, string $settingName): array
{
$settings = \Piwik\Plugins\CorePluginsAdmin\API::getInstance()->getSystemSettings();
Expand Down
32 changes: 18 additions & 14 deletions plugins/CorePluginsAdmin/vue/dist/CorePluginsAdmin.umd.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit c51668c

Please sign in to comment.