10000 [DoctrineBridge] Add decimal form type by fancyweb · Pull Request #24793 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DoctrineBridge] Add decimal form type #24793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

[DoctrineBridge] Add decimal form type #24793

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
63cb4b4
[DoctrineBridge] Add decimal form type
fancyweb Nov 2, 2017
7fb3068
Merge remote-tracking branch 'upstream/master' into number-type-as-st…
fancyweb Mar 13, 2018
5a4e427
add force_full_scale option to handle all cases
fancyweb Mar 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add force_full_scale option to handle all cases
  • Loading branch information
@fancyweb
fancyweb committed Mar 13, 2018
commit 5a4e42712782f7dc7520dff401773c0120a24c17
91 changes: 91 additions & 0 deletions src/Symfony/Bridge/Doctrine/Form/DataTransformer/NumberToStringTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\Form\DataTransformer;

use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;

class NumberToStringTransformer implements DataTransformerInterface
{
/**
* @var bool
*/
private $forceFullScale;

/**
* @var int|null
*/
private $scale;

/**
* @param bool $forceFullScale
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs documentation

* @param int|null $scale
*/
public function __construct($forceFullScale = false, $scale = null)
{
$this->forceFullScale = $forceFullScale;
$this->scale = $scale;
}

/**
* @param mixed $value
*
* @return string|null
*/
public function transform($value)
{
if (null === $value) {
return null;
}

if (!is_string($value)) {
throw new TransformationFailedException('Expected a string.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be reflected in the @param

}

return $value;
}

/**
* @param mixed $value
*
* @return string|null
*/
public function reverseTransform($value)
{
if (null === $value) {
return null;
}

if (is_string($value)) {
return $value;
}

$valueIsInt = is_int($value);
if (!$valueIsInt && !is_float($value)) {
throw new TransformationFailedException('Expected an int or a float.');
}

if ($this->forceFullScale && is_int($this->scale)) {
if ($valueIsInt) {
$value = floatval($value);
}

return number_format($value, $this->scale, '.', '');
}

try {
return (string) $value;
} catch (\Exception $e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be \Throwable IMO

throw new TransformationFailedException();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous exception should be passed on here

}
}
}
37 changes: 15 additions & 22 deletions src/Symfony/Bridge/Doctrine/Form/Type/DecimalType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@

namespace Symfony\Bridge\Doctrine\Form\Type;

use Symfony\Bridge\Doctrine\Form\DataTransformer\NumberToStringTransformer;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\CallbackTransformer;
use Symfony\Component\Form\Exception\TransformationFailedException;
use Symfony\Component\Form\Extension\Core\Type\NumberType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

class DecimalType extends AbstractType
{
Expand All @@ -24,27 +24,20 @@ class DecimalType extends AbstractType
*/
public function buildForm(FormBuilderInterface $builder, array $options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

force_full_scale option to be documented above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scale too

{
$builder->addModelTransformer(new CallbackTransformer(function ($value) {
if (null === $value) {
return null;
}

if (!is_string($value)) {
throw new TransformationFailedException('Expected a string.');
}

return $value;
}, function ($value) {
if (null === $value) {
return null;
}

if (!is_int($value) && !is_float($value)) {
throw new TransformationFailedException('Expected an int or a float.');
}
$builder->addModelTransformer(new NumberToStringTransformer($options['force_full_scale'], $options['scale']));
}

return (string) $value;
}));
/**
* {@inheritdoc}
*/
public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(array(
'force_full_scale' => false
));
$resolver->setAllowedTypes('force_full_scale', array(
'boolean'
));
}

/**
Expand Down
10 changes: 7 additions & 3 deletions src/Symfony/Bridge/Doctrine/Tests/Fixtures/Price.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ class Price
/** @Id @Column(type="integer") */
public $id;

/** @Column(type="decimal") */
public $value;
/** @Column(type="decimal", scale=2) */
public $doesNotPreserveFullScaleValue;

/** @Column(type="string") */
public $preserveFullScaleValueSimulation;

/**
* @param int $id
Expand All @@ -31,6 +34,7 @@ class Price
public function __construct(int $id, float $value)
{
$this->id = $id;
$this->value = $value;
$this->doesNotPreserveFullScaleValue = $value;
$this->preserveFullScaleValueSimulation = number_format($value, 2, '.', '');
}
}
103 changes: 92 additions & 11 deletions src/Symfony/Bridge/Doctrine/Tests/Form/Type/DecimalTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,33 +60,114 @@ protected function tearDown()
$this->em = null;
}

public function testSubmitWithSameStringValue()
// On some platforms, fetched decimal values are rounded (the full scale is not preserved)
// eg : on SQLite, inserted float value 4.50 will be fetched as string value "4.5"
public function testSubmitWithSameStringValueOnAPlatformThatDoesNotPreserveFullScaleValueWithoutForceFullScale()
{
$price = new Price(1, 1.23);
$this->em->persist($price);
$fullScalePrice = new Price(1, 1.23);
$nonFullScalePrice = new Price(2, 4.50);
$this->em->persist($fullScalePrice);
$this->em->persist($nonFullScalePrice);
$this->em->flush();

$this->em->refresh($price);
$this->em->refresh($fullScalePrice);
$this->em->refresh($nonFullScalePrice);

$this->assertInternalType('string', $price->value);
$stringValue = $price->value;
$this->assertInternalType('string', $fullScalePrice->doesNotPreserveFullScaleValue);
$fullScalePriceStringValue = $fullScalePrice->doesNotPreserveFullScaleValue;

$formBuilder = $this->factory->createBuilder(FormType::class, $price, array(
$formBuilder = $this->factory->createBuilder(FormType::class, $fullScalePrice, array(
'data_class' => Price::class
));
$formBuilder->add('value', static::TESTED_TYPE);
$formBuilder->add('doesNotPreserveFullScaleValue', static::TESTED_TYPE, array(
'force_full_scale' => false
));

$form = $formBuilder->getForm();
$form->submit(array(
'doesNotPreserveFullScaleValue' => $fullScalePriceStringValue
));

$this->assertSame($fullScalePriceStringValue, $fullScalePrice->doesNotPreserveFullScaleValue);

$this->assertInternalType('string', $nonFullScalePrice->doesNotPreserveFullScaleValue);
$nonFullScalePriceStringValue = $nonFullScalePrice->doesNotPreserveFullScaleValue;

$formBuilder = $this->factory->createBuilder(FormType::class, $nonFullScalePrice, array(
'data_class' => Price::class
));
$formBuilder->add('doesNotPreserveFullScaleValue', static::TESTED_TYPE, array(
'force_full_scale' => false
));

$form = $formBuilder->getForm();
$form->submit(array(
'doesNotPreserveFullScaleValue' => $nonFullScalePriceStringValue
));

$this->assertSame($nonFullScalePriceStringValue, $nonFullScalePrice->doesNotPreserveFullScaleValue);

$unitOfWork = $this->em->getUnitOfWork();
$unitOfWork->computeChangeSets();

$this->assertSame(array(), $unitOfWork->getEntityChangeSet($fullScalePrice));
$this->assertSame(array(), $unitOfWork->getEntityChangeSet($nonFullScalePrice));
}

// On some platforms, fetched decimal values are not rounded at all (the full scale is preserved)
// eg : on PostgreSQL, inserted float value 4.50 will be fetched as string value "4.50"
public function testSubmitWithSameStringValueOnAPlatformThatPreserveFullScaleValueWithForceFullScale()
{
$fullScalePrice = new Price(1, 1.23);
$nonFullScalePrice = new Price(2, 4.50);
$this->em->persist($fullScalePrice);
$this->em->persist($nonFullScalePrice);
$this->em->flush();

$this->em->refresh($fullScalePrice);
$this->em->refresh($nonFullScalePrice);

$this->assertInternalType('string', $fullScalePrice->preserveFullScaleValueSimulation);
$fullScalePriceStringValue = $fullScalePrice->preserveFullScaleValueSimulation;

$formBuilder = $this->factory->cre A6C7 ateBuilder(FormType::class, $fullScalePrice, array(
'data_class' => Price::class
));
$formBuilder->add('preserveFullScaleValueSimulation', static::TESTED_TYPE, array(
'force_full_scale' => true,
'scale' => 2
));

$form = $formBuilder->getForm();
$form->submit(array(
'preserveFullScaleValueSimulation' => $fullScalePriceStringValue
));

$this->assertSame($fullScalePriceStringValue, $fullScalePrice->preserveFullScaleValueSimulation);

$this->assertInternalType('string', $nonFullScalePrice->preserveFullScaleValueSimulation);
$nonFullScalePriceStringValue = $nonFullScalePrice->preserveFullScaleValueSimulation;

$formBuilder = $this->factory->createBuilder(FormType::class, $nonFullScalePrice, array(
'data_class' => Price::class
));
$formBuilder->add('preserveFullScaleValueSimulation', static::TESTED_TYPE, array(
'force_full_scale' => true,
'scale' => 2
));

$form = $formBuilder->getForm();
$form->submit(array(
'value' => $stringValue
'preserveFullScaleValueSimulation' => $nonFullScalePriceStringValue
));

$this->assertSame($stringValue, $price->value);
$this->assertSame($nonFullScalePriceStringValue, $nonFullScalePrice->preserveFullScaleValueSimulation);

$unitOfWork = $this->em->getUnitOfWork();
$unitOfWork->computeChangeSets();

$this->assertSame(array(), $unitOfWork->getEntityChangeSet($price));
$this->assertSame(array(), $unitOfWork->getEntityChangeSet($fullScalePrice));
$this->assertSame(array(), $unitOfWork->getEntityChangeSet($nonFullScalePrice));
}

public function testSubmitNull($expected = null, $norm = null, $view = null)
Expand Down
0