Data Transformers in list fields editable

[peter-gribanov]

Subject

Allow use Symfony Data Transformers in configureListFields() like:

class TestimonialAdmin extends BaseAdmin
{
    // ...

    protected function configureListFields(ListMapper $list_mapper): void
    {
        $list_mapper
            ->add('permission', 'choice', [
                'label' => 'Permission',
                'editable' => true,
                'choices' => Permission::change(),
                'template' => '@Frontend/Testimonial/admin/list_permission.html.twig',
                'catalogue' => 'messages',
                'data_transformer' => new PermissionDataTransformer(),
            ]);
    }
}

Closes #5693

Changelog

### Fixed
- Allow use Symfony Data Transformers in list fields editable

[peter-gribanov]

I'm not sure that we need a date transformer here. Using a Symfony BooleanToStringTransformer here may break BC. At least the tests use a non-string.

We can use CallbackTransformer here, so as not to create custom transformer.

$data_transformer = new CallbackTransformer(static function ($value): string {
    return (string) (int) $value;
}, static function ($value): ?bool {
    return filter_var($value, FILTER_VALIDATE_BOOLEAN);
});

[peter-gribanov]

Try rebuild

[OskarStark]

any docs changes needed?

[peter-gribanov]

@OskarStark this bug fix works more like a feature. It would be nice to document this feature.

[peter-gribanov]

@OskarStark i add example here a little bit later.

[peter-gribanov]

Try rebuild

[core23]

I would prefer a more generic way like a DataTransformerResolver. This way we can add new transformers more easily.

[peter-gribanov]

Do you suggest just putting this method in a separate class?

[peter-gribanov]

Like this?

final class DataTransformerResolver
{
    private $modelManager;

    public function __construct($modelManager)
    {
        $this->modelManager = $modelManager;
    }

    public function resolve(FieldDescriptionInterface $fieldDescription): ?DataTransformerInterface
    {
        $dataTransformer = $fieldDescription->getOption('data_transformer');

        // allow override transformers for 'date', 'boolean' and 'choice' field types
        if ($dataTransformer instanceof DataTransformerInterface) {
            return $dataTransformer;
        }

        // Handle date type has setter expect a DateTime object
        if ('date' === $fieldDescription->getType()) {
            return new DateTimeToStringTransformer();
        }

        // Handle boolean type transforming the value into a boolean
        if ('boolean' === $fieldDescription->getType()) {
            return new BooleanToStringTransformer('1');
        }

        // Handle entity choice association type, transforming the value into entity
        if ('choice' === $fieldDescription->getType()) {
            $className = $fieldDescription->getOption('class');

            if (null !== $className && $className === $fieldDescription->getTargetEntity()) {
                return new ModelToIdTransformer($this->modelManager, $className);
            }
        }

        return null;
    }
}

[peter-gribanov]

Or maybe you are interested in the option with individual services that are binded into a resolver?

[peter-gribanov]

Like this?

final class ResolverFieldDataTransformer implements FieldDataTransformer
{
    private $dataTransformers;

    public function __construct(array $dataTransformers)
    {
        $this->dataTransformers = $dataTransformers;
    }

    public function transform($value, FieldDescriptionInterface $fieldDescription)
    {
        $dataTransformer = $fieldDescription->getOption('data_transformer');

        // allow override predefined transformers
        if ($dataTransformer instanceof DataTransformerInterface) {
            return $dataTransformer->reverseTransform($value);
        }

        $type = $fieldDescription->getType();

        if (array_key_exists($type, $this->dataTransformers)) {
            return $this->dataTransformers[$type]->transform($value, $fieldDescription);
        }

        return $value;
    }
}

final class ChoiceFieldDataTransformer implements FieldDataTransformer
{
    private $modelManager;

    public function __construct($modelManager)
    {
        $this->modelManager = $modelManager;
    }

    public function transform($value, FieldDescriptionInterface $fieldDescription)
    {
        $className = $fieldDescription->getOption('class');

        if (null !== $className && $className === $fieldDescription->getTargetEntity()) {
            return (new ModelToIdTransformer($this->modelManager, $className))->reverseTransform($value);
        }

        return $value;
    }
}

$resolver = new ResolverFieldDataTransformer([
    'choice' => new ChoiceFieldDataTransformer($modelManager),
]);

[peter-gribanov]

@core23 i am very confused by the tight binding to the field type here. I understand why this is done, but it still looks like a dirty hack.

[peter-gribanov]

@core23 please review DataTransformerResolver service

[VincentLanglet]

@sonata-project/contributors have any news?

Can you fix the phpstan check ?

@sonata-project/contributors @core23 Please review.

[peter-gribanov]

Can you fix the phpstan check ?

@VincentLanglet problem is not on my side.
https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Guesser/TypeGuesserChain.php#L63

[VincentLanglet]

I fixed it ; you just have to rebase now

#6356

[SonataCI]

Could you please rebase your PR and fix merge conflicts?

[wbloszyk]

Awesome work.

[VincentLanglet]

Please review @core23. :)

(Or can you dismiss the review @greg0ire if we get no response after some days ?)

Dismissing it now, but you can wait some days if you want to :)

[wbloszyk]

You can use DateTimeInterface, anyway IMO we can accept this.

[wbloszyk]

Suggested change

	$this->assertInstanceOf(\DateTime::class, $resultDate);
	$this->assertInstanceOf(\DateTimeInterface::class, $resultDate);

[peter-gribanov]

Now the transformer always returns the \DateTime. If we expect the interface in tests, we may not notice how we break BC by replacing \DateTime with \DateTimeImmutable in the transformer.

[wbloszyk]

Suggested change

	$this->assertSame($expectedDate->format(\DateTime::ATOM), $resultDate->format(\DateTime::ATOM));
	$this->assertSame($expectedDate->format(\DateTimeInterface::ATOM), $resultDate->format(\DateTime::ATOM));

[OskarStark]

@core23 @phansys @jordisala1991 can you please give us a final review? Thanks

[VincentLanglet]

Thanks !