Skip to content

Commit f55d4e4

Browse files
TheSyscallTheSyscall
committed
WIP
1 parent ff70741 commit f55d4e4

4 files changed

Lines changed: 65 additions & 58 deletions

File tree

‎library/Icinga/Web/Form/ConfigForm.php‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,14 @@
1010
use Icinga\Web\Widget\ShowConfiguration;
1111
use ipl\Html\FormElement\PasswordElement;
1212
use ipl\Stdlib\Str;
13-
use ipl\Web\Common\CsrfCounterMeasure;
1413
use ipl\Web\Compat\CompatForm;
1514

1615
/**
1716
* Form base-class providing standard functionality for configuration forms
1817
*/
1918
class ConfigForm extends CompatForm
2019
{
21-
use CsrfCounterMeasure;
22-
23-
/**
24-
* Name of the submit button element
25-
*
26-
* @var string
27-
*/
20+
/** @var string Name of the submit button element */
2821
protected const SUBMIT_BUTTON_NAME = 'store';
2922

3023
/**
@@ -80,7 +73,8 @@ protected function populateFromConfig(): void
8073
{
8174
$populate = [];
8275
foreach ($this->getElements() as $element) {
83-
if (($parts = $this->getIniKeyFromName($element->getName())) === null) {
76+
$parts = $this->getIniKeyFromName($element->getName());
77+
if (count($parts) !== 2) {
8478
continue;
8579
}
8680

@@ -101,13 +95,12 @@ protected function populateFromConfig(): void
10195
*
10296
* @param string $name The element name
10397
*
104-
* @return ?array<string, string>
98+
* @return string[] An array containing the section and key, or an empty array
99+
* if the name is not a valid configuration key
105100
*/
106-
protected function getIniKeyFromName(string $name): ?array
101+
protected function getIniKeyFromName(string $name): array
107102
{
108-
$parts = explode($this->sectionKeyDelimiter, $name, 2);
109-
110-
return count($parts) === 2 ? $parts : null;
103+
return explode($this->sectionKeyDelimiter, $name, 2);
111104
}
112105

113106
/**
@@ -120,12 +113,12 @@ protected function getIniKeyFromName(string $name): ?array
120113
*/
121114
protected function getConfigValue(string $name, mixed $default = null): mixed
122115
{
123-
if (($parts = $this->getIniKeyFromName($name)) === null) {
116+
$parts = $this->getIniKeyFromName($name);
117+
if (count($parts) !== 2) {
124118
return $default;
125119
}
126-
[$section, $key] = $parts;
127120

128-
return $this->config->get($section, $key, $default);
121+
return $this->config->get($parts[0], $parts[1], $default);
129122
}
130123

131124
/**
@@ -154,7 +147,8 @@ protected function shouldRevertIfEmpty(string $elementName): bool
154147
protected function save(): void
155148
{
156149
foreach ($this->getValues() as $element => $value) {
157-
if (($parts = $this->getIniKeyFromName($element)) === null) {
150+
$parts = $this->getIniKeyFromName($element);
151+
if (count($parts) !== 2) {
158152
continue;
159153
}
160154

@@ -180,6 +174,15 @@ protected function save(): void
180174
$this->config->saveIni();
181175
}
182176

177+
/**
178+
* Handle the form submission
179+
*
180+
* If the form submission is successful, the configuration is saved to disk.
181+
* If an error occurs, the form is re-rendered with the error message and the
182+
* raw INI configuration. The original exception is re-thrown.
183+
*
184+
* @return void
185+
*/
183186
protected function onSuccess(): void
184187
{
185188
try {
@@ -188,6 +191,7 @@ protected function onSuccess(): void
188191
$content = $this->getContent();
189192
array_unshift($content, new ShowConfiguration($e, $this->config));
190193
$this->setContent($content);
194+
191195
throw $e;
192196
}
193197
}

‎library/Icinga/Web/Form/ConfigSectionForm.php‎

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -37,25 +37,13 @@ class ConfigSectionForm extends ConfigForm
3737
/** @var string Event emitted when the form has successfully renamed a configuration section */
3838
public const ON_RENAME = 'rename';
3939

40-
/**
41-
* Whether the form is used for creating a new configuration section
42-
*
43-
* @var bool
44-
*/
40+
/** @var bool Whether the form is used for creating a new configuration section */
4541
protected bool $isCreateForm = false;
4642

47-
/**
48-
* Whether the form allows deletion of the configuration section
49-
*
50-
* @var bool
51-
*/
43+
/** @var bool Whether the form allows deletion of the configuration section */
5244
protected bool $allowDeletion = true;
5345

54-
/**
55-
* Whether the form allows renaming of the configuration section
56-
*
57-
* @var bool
58-
*/
46+
/** @var bool Whether the form allows renaming of the configuration section */
5947
protected bool $allowRename = true;
6048

6149
/**
@@ -119,6 +107,11 @@ public function isValid(): bool
119107
return parent::isValid();
120108
}
121109

110+
/**
111+
* Whether the form is used for creating a new configuration section
112+
*
113+
* @return bool
114+
*/
122115
public function isCreateForm(): bool
123116
{
124117
return $this->isCreateForm;
@@ -131,16 +124,17 @@ public function isCreateForm(): bool
131124
*
132125
* @return static
133126
*
134-
* @throws LogicException If the form has already been assembled or if the form is a creation form
127+
* @throws LogicException If the form has already been assembled or if the form
128+
* is a creation form
135129
*/
136130
public function setAllowDeletion(bool $allowDeletion = true): static
137131
{
138132
if ($this->isCreateForm()) {
139-
throw new LogicException('Can never delete a new configuration section.');
133+
throw new LogicException('Can never delete a new configuration section');
140134
}
141135

142136
if ($this->hasBeenAssembled) {
143-
throw new LogicException('Form has already been assembled.');
137+
throw new LogicException('Form has already been assembled');
144138
}
145139

146140
$this->allowDeletion = $allowDeletion;
@@ -167,16 +161,17 @@ public function allowDeletion(): bool
167161
*
168162
* @return $this
169163
*
170-
* @throws LogicException If the form has already been assembled or if the form is a creation form
164+
* @throws LogicException If the form has already been assembled or if the form
165+
* is a creation form
171166
*/
172167
public function setAllowRename(bool $allowRename = true): static
173168
{
174169
if ($this->isCreateForm()) {
175-
throw new LogicException('Can never rename a new configuration section.');
170+
throw new LogicException('Can never rename a new configuration section');
176171
}
177172

178173
if ($this->hasBeenAssembled) {
179-
throw new LogicException('Form has already been assembled.');
174+
throw new LogicException('Form has already been assembled');
180175
}
181176

182177
$this->allowRename = $allowRename;
@@ -241,11 +236,7 @@ protected function handleRename(): void
241236
$this->config->removeSection($oldName);
242237
$this->section = $newName;
243238
parent::onSuccess();
244-
$this->emit(static::ON_RENAME, [
245-
$this,
246-
$oldName,
247-
$this->section,
248-
]);
239+
$this->emit(static::ON_RENAME, [$this, $oldName, $this->section]);
249240
}
250241

251242
/**
@@ -267,6 +258,11 @@ public function shouldDelete(): bool
267258
return $deleteButton->hasBeenPressed();
268259
}
269260

261+
/**
262+
* Check if the form has a delete button
263+
*
264+
* @return bool
265+
*/
270266
public function hasDeleteButton(): bool
271267
{
272268
return $this->hasElement(static::DELETE_BUTTON_NAME);
@@ -297,12 +293,17 @@ protected function addSectionNameElement(array $params = []): void
297293
$params['ignore'] = true;
298294
$params['label'] ??= $this->translate('Name');
299295
$params['validators'][] = new CallbackValidator(function ($value, CallbackValidator $validator) {
300-
if (Str::isEmpty($value) || $value === $this->section) {
296+
if ($value === $this->section) {
301297
return true;
302298
}
303299

300+
if (Str::isEmpty($value)) {
301+
$validator->addMessage($this->translate('Please enter a name'));
302+
return false;
303+
}
304+
304305
if ($this->config->hasSection($value)) {
305-
$validator->addMessage($this->translate('An entry with this name already exists.'));
306+
$validator->addMessage($this->translate('An entry with this name already exists'));
306307
return false;
307308
}
308309

@@ -312,19 +313,19 @@ protected function addSectionNameElement(array $params = []): void
312313
$this->addElement('text', static::NAME_ELEMENT_NAME, $params);
313314
}
314315

315-
protected function getIniKeyFromName(string $name): ?array
316+
protected function getIniKeyFromName(string $name): array
316317
{
317-
return $this->section === null ? null : [$this->section, $name];
318+
if ($this->section === null) {
319+
return [];
320+
}
321+
322+
return [$this->section, $name];
318323
}
319324

320325
protected function onSuccess(): void
321326
{
322327
if ($this->isCreateForm()) {
323328
$this->section = $this->getValue(static::NAME_ELEMENT_NAME);
324-
325-
if (Str::isEmpty($this->section)) {
326-
throw new LogicException('Section must be set before saving a new configuration section.');
327-
}
328329
parent::onSuccess();
329330
} elseif ($this->shouldDelete()) {
330331
$this->handleDelete();
@@ -413,12 +414,14 @@ protected function save(): void
413414
if (Str::isEmpty($value) && $this->shouldRevertIfEmpty($element)) {
414415
$value = $this->config->get($this->section, $element);
415416
}
417+
416418
if (Str::isEmpty($value)) {
417419
unset($configSection[$element]);
418420
} else {
419421
$configSection->$element = $value;
420422
}
421423
}
424+
422425
$this->config->setSection($this->section, $configSection);
423426
$this->config->saveIni();
424427
}

‎test/php/library/Icinga/Web/Form/ConfigFormTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,10 @@ public function testGetIniKeyFromNameSplitsOnDoubleUnderscore(): void
4242
$this->assertSame(['section', 'key'], $form->exposeGetIniKeyFromName('section__key'));
4343
}
4444

45-
public function testGetIniKeyFromNameReturnsNullWithoutDelimiter(): void
45+
public function testGetIniKeyFromNameReturnsEmptyArrayWithoutDelimiter(): void
4646
{
4747
$form = $this->makeForm();
48-
$this->assertNull($form->exposeGetIniKeyFromName('nodelimiter'));
48+
$this->assertSame([], $form->exposeGetIniKeyFromName('nodelimiter'));
4949
}
5050

5151
public function testGetIniKeyFromNameSplitsOnFirstDelimiterOnly(): void
@@ -57,7 +57,7 @@ public function testGetIniKeyFromNameSplitsOnFirstDelimiterOnly(): void
5757
public function testGetIniKeyFromNameDefaultDelimiterDoesNotMatchSingleUnderscore(): void
5858
{
5959
$form = $this->makeForm();
60-
$this->assertNull($form->exposeGetIniKeyFromName('section_key'));
60+
$this->assertSame([], $form->exposeGetIniKeyFromName('section_key'));
6161
}
6262

6363
public function testGetIniKeyFromNameUsesCustomDelimiter(): void
@@ -71,7 +71,7 @@ public function testCustomDelimiterNoLongerMatchesDefaultDoubleUnderscore(): voi
7171
{
7272
$form = $this->makeForm();
7373
$form->exposeSetSectionKeyDelimiter('|');
74-
$this->assertNull($form->exposeGetIniKeyFromName('section__key'));
74+
$this->assertSame([], $form->exposeGetIniKeyFromName('section__key'));
7575
}
7676

7777
public function testGetConfigValueReturnsValueFromConfig(): void

‎test/php/library/Icinga/Web/Form/ConfigSectionFormTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,10 @@ public function testGetIniKeyFromNameIgnoresDoubleUnderscoreConvention(): void
137137
);
138138
}
139139

140-
public function testGetIniKeyFromNameReturnsNullForCreateForm(): void
140+
public function testGetIniKeyFromNameReturnsEmptyArrayForCreateForm(): void
141141
{
142142
$form = $this->makeCreateForm();
143-
$this->assertNull($form->exposeGetIniKeyFromName('anykey'));
143+
$this->assertSame([], $form->exposeGetIniKeyFromName('anykey'));
144144
}
145145

146146
public function testShouldDeleteReturnsFalseWhenNoDeleteButtonExists(): void

0 commit comments

Comments
 (0)