Skip to content

Commit

Permalink
Mostly revert "Fixes for nested multipliers (#59)"
Browse files Browse the repository at this point in the history 
This partially reverts commit 39725d3.

The commit introduced a regression in `testGroupManualRenderWithButtons`,
changed the internal architecture in a way that made it harder to reason about
(the Multiplier inconsistently took over some responsibilities of ComponentResolver).
Additionally, it applied the following changes that are unrelated
to the purported fix and not really necessary:

- Latte 2 macros are trivial to migrate and unclearly named, no need to keep them for BC.
- The onSuccess handlers were already fixed in ed96ba0.
- `testGroupManualRenderWithButtons` look like remnants of debugging effort, they do not fix the test.

We are only keeping the following:

- Test for nested support (to be implemented later).
- Fix for `assertMatchesRegularExpression` deprecation in PHPUnit.

For reference, the reverted change is the following commit that was squashed
into 39725d3, except for the change to `testSendNested`:
2c33de2
  • Loading branch information
@jtojnar
jtojnar committed May 14, 2024
1 parent bfe52eb commit 71cc347
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 78 deletions.
9 changes: 7 additions & 2 deletions src/ComponentResolver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,22 @@ final class ComponentResolver
/** @var mixed[] */
private ?array $purgedHttpData = null;

/** @var mixed[] */
private array $defaults = [];

private int $minCopies;

private bool $reached = false;

/**
* @param mixed[] $httpData
* @param mixed[] $defaults
*/
public function __construct(array $httpData, ?int $maxCopies, int $minCopies)
public function __construct(array $httpData, array $defaults, ?int $maxCopies, int $minCopies)
{
$this->httpData = $httpData;
$this->maxCopies = $maxCopies;
$this->defaults = $defaults;
$this->minCopies = $minCopies;

foreach ($httpData as $index => $_) {
Expand Down Expand Up @@ -66,7 +71,7 @@ public function getCreateNum(): int
*/
public function getDefaults(): array
{
return array_slice([], 0, $this->maxCopies, true);
return array_slice($this->defaults, 0, $this->maxCopies, true);
}

/**
Expand Down
2 changes: 0 additions & 2 deletions src/Latte/Extension/MultiplierExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ public function getTags(): array
'n:multiplier' => [MultiplierNode::class, 'create'],
'multiplier:remove' => [MultiplierRemoveNode::class, 'create'],
'multiplier:add' => [MultiplierAddNode::class, 'create'],
'btnRemove' => [MultiplierRemoveNode::class, 'create'],
'btnCreate' => [MultiplierAddNode::class, 'create'],
];
}

Expand Down
58 changes: 24 additions & 34 deletions src/Multiplier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ class Multiplier extends Container

protected ?RemoveButton $removeButton = null;

/** @var mixed[] */
protected array $httpData = [];

protected ?int $maxCopies = null;

protected int $totalCopies = 0;
Expand All @@ -63,8 +66,6 @@ class Multiplier extends Container
/** @var Container[] */
protected array $noValidate = [];

protected ComponentResolver $resolver;

private ?Form $form = null;

private bool $attachedCalled = false;
Expand Down Expand Up @@ -211,33 +212,28 @@ public function addCopy(?int $number = null, array|object $defaults = []): Conta
return $container;
}

public function createCopies(bool $forceValues = false): void
public function createCopies(): void
{
if ($this->created === true) {
return;
}

$this->created = true;

if (!isset($this->resolver)) {
$this->resolver = new ComponentResolver($this->values, $this->maxCopies, $this->minCopies);
}
$resolver = new ComponentResolver($this->httpData, $this->values, $this->maxCopies, $this->minCopies);

$this->attachCreateButtons();
$this->createComponents($forceValues);
$this->createComponents($resolver);
$this->detachCreateButtons();

if ($this->maxCopies === null || $this->totalCopies < $this->maxCopies) {
$this->attachCreateButtons();
}

if (
$this->form !== null &&
$this->resolver->isRemoveAction() &&
$this->totalCopies >= $this->minCopies &&
!$this->resolver->reachedMinLimit()
) {
$this->form->setSubmittedBy($this->removeButton->create($this));
if ($this->form !== null && $resolver->isRemoveAction() && $this->totalCopies >= $this->minCopies && !$resolver->reachedMinLimit()) {
/** @var RemoveButton $removeButton */
$removeButton = $this->removeButton;
$this->form->setSubmittedBy($removeButton->create($this));

$this->resetFormEvents();

Expand Down Expand Up @@ -337,12 +333,10 @@ public function setValues(array|object $values, bool $erase = false, bool $onlyD
if ($this->created) {
foreach ($this->getContainers() as $container) {
$this->removeComponent($container);
$this->totalCopies--;
}

$this->created = false;
$this->detachCreateButtons();
$this->resolver = new ComponentResolver($this->values, $this->maxCopies, $this->minCopies);
$this->createCopies();
}

Expand Down Expand Up @@ -389,11 +383,10 @@ protected function isFormSubmitted(): bool

protected function loadHttpData(): void
{
if ($this->isFormSubmitted()) {
if ($this->form !== null && $this->isFormSubmitted()) {
/** @var array<mixed> $httpData The other types from the union can only be returned when the htmlName argument is passed. https://github.com/nette/forms/pull/333 */
$httpData = $this->getForm()->getHttpData();
$httpData = Arrays::get($httpData, $this->getHtmlName(), []);
$this->resolver = new ComponentResolver($httpData ?? [], $this->maxCopies, $this->minCopies);
$httpData = $this->form->getHttpData();
$this->httpData = (array) Arrays::get($httpData, $this->getHtmlName(), []);
}
}

Expand Down Expand Up @@ -457,34 +450,31 @@ protected function removeComponentProperly(IComponent $component): void
$this->removeComponent($component);
}

private function createComponents(bool $forceValues = false): void
private function createComponents(ComponentResolver $resolver): void
{
$containers = [];
$containerDefaults = $this->createContainer()->getValues(self::Array);

// Components from httpData
if ($this->isFormSubmitted() && !$forceValues) {
foreach ($this->resolver->getValues() as $number => $_) {
if ($this->isFormSubmitted()) {
foreach ($resolver->getValues() as $number => $_) {
$containers[] = $container = $this->addCopy($number);

/** @var BaseControl $control */
foreach ($container->getComponents(false, Control::class) as $control) {
foreach ($container->getControls() as $control) {
$control->loadHttpData();
}
}
} else { // Components from default values
foreach ($this->resolver->getValues() as $number => $values) {
$containers[] = $container = $this->addCopy($number, $values);
$container->setValues($values);
foreach ($resolver->getDefaults() as $number => $values) {
$containers[] = $this->addCopy($number, $values);
}
}

// Default number of copies
if (!$this->values) {
if (!$this->isFormSubmitted() && !$this->values) {
$copyNumber = $this->copyNumber;
while ($copyNumber > 0 && $this->isValidMaxCopies() && $this->totalCopies < $this->minCopies) {
while ($copyNumber > 0 && $this->isValidMaxCopies()) {
$containers[] = $container = $this->addCopy();
$container->setValues($containerDefaults);
$copyNumber--;
}
}
Expand All @@ -495,11 +485,11 @@ private function createComponents(bool $forceValues = false): void
}

// New containers, if create button hitted
if ($this->form !== null && $this->resolver->isCreateAction() && $this->form->isValid()) {
$count = $this->resolver->getCreateNum();
if ($this->form !== null && $resolver->isCreateAction() && $this->form->isValid()) {
$count = $resolver->getCreateNum();
while ($count > 0 && $this->isValidMaxCopies()) {
$this->noValidate[] = $containers[] = $container = $this->addCopy();
$container->setValues($containerDefaults);
$container->setValues($this->createContainer()->getValues(self::Array));
$count--;
}
}
Expand Down
6 changes: 1 addition & 5 deletions tests/Unit/CreateButtonTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,7 @@ public function testCallback()
$submitter->setHtmlAttribute('class', 'add-btn');
});

$response = $this->services->form->createRequest($factory
->formModifier(function (\Nette\Application\UI\Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})->createForm())->setPost([
$response = $this->services->form->createRequest($factory->createForm())->setPost([
'm' => [
['bar' => ''],
['bar' => ''],
Expand Down
25 changes: 2 additions & 23 deletions tests/Unit/MultiplierTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ public function testSendBase()
$this->parameters['onCreate'][] = $container;
};
})
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
)
->setPost($params = [
Expand Down Expand Up @@ -102,10 +98,6 @@ public function testSendCopy2()
$this->parameters['onCreate'][] = $container;
};
})
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
)
->setPost($params = [
Expand Down Expand Up @@ -156,10 +148,6 @@ public function testSendMaxCopy()
$this->parameters['onCreate'][] = $container;
};
})
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
)
->setPost([
Expand Down Expand Up @@ -226,10 +214,6 @@ public function testSendNested()
}));
$container['m2']->addCreateButton('create');
})
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
);
$request->setPost([
Expand Down Expand Up @@ -396,15 +380,14 @@ public function testGroupManualRenderWithButtons()
->multiplierModifier(function (Multiplier $multiplier) {
$multiplier->onCreate[] = function (Container $container) {
$this->parameters['onCreate'][] = $container;
$container->setParent(null, 'X');
//var_dump($container);
};
$multiplier->addCreateButton();
$multiplier->addRemoveButton();
//$multiplier->setMinCopies(1);
$multiplier->setMinCopies(1);
})
->createForm());
$dom = $request->render(__DIR__ . '/templates/group.latte')->toDomQuery();

$this->assertDomHas($dom, 'input[name="m[0][multiplier_remover]"]');
$this->assertDomHas($dom, 'input[name="m[1][multiplier_remover]"]');
}
Expand Down Expand Up @@ -473,10 +456,6 @@ public function testPromptSelect()
->setPrompt('Select');
})
->addCreateButton()
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
)
->setPost($params = [
Expand Down
12 changes: 0 additions & 12 deletions tests/Unit/RemoveButtonTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,6 @@ public function testAddClass()
$submitter->setHtmlAttribute('class', 'btn btn-remove');
})
->addCreateButton()
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
)->setPost([
'm' => [
Expand All @@ -219,10 +215,6 @@ public function testDeleteLastElementToZero()
->setMinCopies(0)
->addRemoveButton()
->addCreateButton()
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
)->modifyForm(function (Form $form) {
$form['m']->setValues([
Expand Down Expand Up @@ -252,10 +244,6 @@ public function testOnRemoveEvent()
$called = true;
};
})
->formModifier(function (Form $form) {
$form->onSuccess[] = $form->onError[] = $form->onSubmit[] = function () {
};
})
->createForm()
)->setPost([
'm' => [
Expand Down

0 comments on commit 71cc347

Please sign in to comment.