Skip to content

Commit

Permalink
Fix: do not generates value if the form has no constraints
Browse files Browse the repository at this point in the history
  • Loading branch information
vincent4vx committed Feb 16, 2021
1 parent 821934b commit 7a99c76
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 3 deletions.
7 changes: 6 additions & 1 deletion src/Aggregate/Form.php
Expand Up @@ -360,7 +360,12 @@ private function submitToChildrenAndValidate($data, string $method): void
return;
}

$this->error = $this->validator->validate($this->value(), $this);
// Do not generate value if it's not necessary
$this->error = $this->validator->hasConstraints()
? $this->validator->validate($this->value(), $this)
: FormError::null()
;

$this->valid = $this->error->empty();
}

Expand Down
8 changes: 8 additions & 0 deletions src/Csrf/CsrfValueValidator.php
Expand Up @@ -74,4 +74,12 @@ public function constraints(): array
{
return []; // Does CsrfConstraint should be returns ?
}

/**
* {@inheritdoc}
*/
public function hasConstraints(): bool
{
return true;
}
}
2 changes: 1 addition & 1 deletion src/Phone/PhoneElement.php
Expand Up @@ -69,7 +69,7 @@ protected function toHttp($phpValue)
return null;
}

return $phpValue->getRawInput() ?: $this->formatter->format($phpValue, PhoneNumberFormat::E164);
return $phpValue->getRawInput() ?? $this->formatter->format($phpValue, PhoneNumberFormat::E164);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/Phone/Transformer/PhoneNumberToStringTransformer.php
Expand Up @@ -62,6 +62,10 @@ public function transformFromHttp($value, ElementInterface $input): ?string

$formatter = $this->formatter ?? ($input instanceof PhoneElement ? $input->getFormatter() : PhoneNumberUtil::getInstance());

if (!$formatter->isValidNumber($value)) {
return $value->getRawInput();
}

return $formatter->format($value, $this->format);
}
}
8 changes: 8 additions & 0 deletions src/Validator/ConstraintValueValidator.php
Expand Up @@ -102,6 +102,14 @@ public function constraints(): array
return $this->constraints;
}

/**
* {@inheritdoc}
*/
public function hasConstraints(): bool
{
return !empty($this->constraints);
}

/**
* Get the empty value validator instance
*
Expand Down
7 changes: 7 additions & 0 deletions src/Validator/ValueValidatorInterface.php
Expand Up @@ -43,4 +43,11 @@ public function onTransformerException(Exception $exception, $value, ElementInte
* @return Constraint[]
*/
public function constraints(): array;

/**
* Check if the validator has constraints, and the value should be validated, or not
*
* @return bool true if there is no constraints, and validate() always returns en ampty error
*/
public function hasConstraints(): bool;
}
26 changes: 26 additions & 0 deletions tests/Aggregate/FormTest.php
Expand Up @@ -67,6 +67,32 @@ public function test_submit_success()
$this->assertSame(4, $this->form['id']->element()->value());
}

/**
*
*/
public function test_submit_without_constraints_should_not_generate_the_value_on_submit()
{
$called = false;
$this->form = new Form(new ChildrenCollection([
$this->registry->childBuilder(StringElement::class, 'firstName')->setter(function ($value) use(&$called) { $called = true; return $value; })->length(['min' => 2])->buildChild(),
$this->registry->childBuilder(StringElement::class, 'lastName')->setter()->length(['min' => 2])->buildChild(),
$this->registry->childBuilder(IntegerElement::class, 'id')->setter()->buildChild(),
]));

$this->form->submit([
'firstName' => 'John',
'lastName' => 'Smith',
'id' => '4',
]);

$this->assertTrue($this->form->valid());
$this->assertTrue($this->form->error()->empty());
$this->assertFalse($called);

$this->form->value();
$this->assertTrue($called);
}

/**
*
*/
Expand Down
2 changes: 2 additions & 0 deletions tests/Phone/PhoneElementTest.php
Expand Up @@ -72,6 +72,7 @@ public function test_submit_null()

$this->assertTrue($element->submit(null)->valid());
$this->assertNull($element->value());
$this->assertNull($element->httpValue());
$this->assertTrue($element->error()->empty());
}

Expand All @@ -84,6 +85,7 @@ public function test_submit_empty_string()

$this->assertTrue($element->submit('')->valid());
$this->assertSame('', $element->value()->getRawInput());
$this->assertSame('', $element->httpValue());
$this->assertTrue($element->error()->empty());
}

Expand Down
16 changes: 16 additions & 0 deletions tests/Phone/Transformer/PhoneNumberToStringTransformerTest.php
Expand Up @@ -49,4 +49,20 @@ public function test_with_format_and_formatter()
$this->assertInstanceOf(PhoneNumber::class, $form['foo']->element()->value());
$this->assertEquals('236547841', $form['foo']->element()->value()->getNationalNumber());
}

/**
*
*/
public function test_with_invalid_number_should_return_unformated_value()
{
$builder = new FormBuilder();
$builder->phone('foo')->modelTransformer(new PhoneNumberToStringTransformer())->getter()->setter();

$form = $builder->buildElement();

$form->submit(['foo' => 'invalid']);
$this->assertSame(['foo' => 'invalid'], $form->value());

$this->assertInstanceOf(PhoneNumber::class, $form['foo']->element()->value());
}
}
3 changes: 2 additions & 1 deletion tests/Validator/ConstraintValueValidatorTest.php
Expand Up @@ -6,7 +6,6 @@
use Bdf\Form\Leaf\StringElement;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Validator\Constraints\NotBlank;
use Symfony\Component\Validator\Constraints\NotEqualTo;

/**
* Class ConstraintValueValidatorTest
Expand All @@ -22,6 +21,7 @@ public function test_validate_success()
$validator = new ConstraintValueValidator([new NotBlank()]);

$this->assertTrue($validator->validate('value', $element)->empty());
$this->assertTrue($validator->hasConstraints());
}

/**
Expand All @@ -33,6 +33,7 @@ public function test_validate_without_constraints()
$validator = new ConstraintValueValidator([]);

$this->assertTrue($validator->validate('value', $element)->empty());
$this->assertFalse($validator->hasConstraints());
}

/**
Expand Down

0 comments on commit 7a99c76

Please sign in to comment.