symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
820 stars 297 forks source link

[LiveComponent] addCollectionItem don't work when using DTO as source #1384

Open jpvdw86 opened 8 months ago

jpvdw86 commented 8 months ago

When using the LiveCollectionType with a DTO as a data object, you now encounter the following error during the addCollectionItem live action.

Scherm­afbeelding 2024-01-08 om 23 42 48

This occurs because the new item is still null and, according to the $propMetadata, an object is expected.

Example code:

Component:

#[AsLiveComponent]
class VehicleFinancialFormComponent extends AbstractController
{
    use DefaultActionTrait;
    use LiveCollectionTrait;

    #[LiveProp]
    public ?FinancialsModel $financials = null;

    protected function instantiateForm(): FormInterface
    {
        return $this->createForm(FinancialsFormType::class, $this->financials);
    }
}

FinancialsDTO:

class FinancialsModel
{
    #[Assert\Valid]
    private array $invoiceRecords;

    #[Assert\Valid]
    private array $estimatedRecords;
}

FormType

class FinancialsFormType extends AbstractType
{
    /**
     * {@inheritDoc}
     */
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {

        $builder->add('invoiceRecords', LiveCollectionType::class, [
            'entry_type' => FinancialRecordForm::class,
            'entry_options' => [
                'label' => false,
                'isEstimation' => false,
            ],
            'label' => false,
            'required' => false,
            'allow_add' => true,
            'allow_delete' => true,
            'by_reference' => false,
        ]);
        $builder->add('estimatedRecords', LiveCollectionType::class, [
            'entry_type' => FinancialRecordForm::class,
            'entry_options' => [
                'label' => false,
                'isEstimation' => true,
            ],
            'label' => false,
            'required' => false,
            'allow_add' => true,
            'allow_delete' => true,
            'by_reference' => false,
        ]);
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'data_class' => FinancialsModel::class,
        ]);
    }
}

RecordFormType

class FinancialRecordForm extends AbstractType
{
        public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->add('description', ChoiceType::class, [
                'label' => false,
                'required' => false,
                'choices' => FinancialRecordType::getChoices(),
                'empty_data' => '',
            ])
            ->add('amountExVat', MoneyType::class, [
                'label' => false,
                'required' => false,
                'empty_data' => '',
            ])
            ->add('vatAmount', MoneyType::class, [
                'label' => false,
                'required' => false,
                'empty_data' => '',
            ])
            ->add('vatable', HiddenType::class, [
                'label' => false,
                'required' => false,
            ]);
        }
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'data_class' => FinancialRecordModel::class,
            'isEstimation' => false,
        ]);
        $resolver->setAllowedTypes('isEstimation', 'bool');
    }
}

RecordDto:

class FinancialRecordModel
{
    #[Assert\NotBlank(message: 'Categorie is verplicht')]
    private ?string $description = null;

    #[Assert\NotBlank(message: 'Bedrag dient minimaal 0 of hoger te zijn')]
    #[Assert\PositiveOrZero(message: 'Bedrag mag niet negatief zijn')]
    private ?float $amountExVat = null;

    #[Assert\NotBlank(message: 'BTW dient minimaal 0 of hoger te zijn')]
    #[Assert\PositiveOrZero(message: 'Bedrag mag niet negatief zijn')]
    private ?float $vatAmount = null;

    private ?bool $vatable = null;

    public function getDescription(): ?string
    {
        return $this->description;
    }

    public function setDescription(?string $description): void
    {
        $this->description = $description;
    }

    public function getAmountExVat(): ?float
    {
        return $this->amountExVat;
    }

    public function setAmountExVat(?float $amountExVat): void
    {
        $this->amountExVat = $amountExVat;
    }

    public function getVatAmount(): ?float
    {
        return $this->vatAmount;
    }

    public function setVatAmount(?float $vatAmount): void
    {
        $this->vatAmount = $vatAmount;
    }

    public function getVatable(): ?bool
    {
        return $this->vatable;
    }

    public function setVatable(?bool $vatable): void
    {
        $this->vatable = $vatable;
    }
}
jpvdw86 commented 8 months ago

Edit: Example is now more dynamic.

For anyone facing the same issue, I've resolved it by creating a specific CustomLiveCollectionTrait for this usecase. This initializes the correct object.

Example code:

#[AsLiveComponent]
class VehicleFinancialFormComponent extends AbstractController
{
    use DefaultActionTrait;
    use CustomLiveCollectionTrait;

    #[LiveProp]
    public ?FinancialsModel $financials = null;

    protected function instantiateForm(): FormInterface
    {
      .....
}
services:
   Symfony\UX\LiveComponent\Metadata\LiveComponentMetadataFactory: '@ux.live_component.metadata_factory'
trait CustomLiveCollectionTrait
{
    use LiveCollectionTrait;

    public function __construct(
        private readonly LiveComponentMetadataFactory $liveComponentMetadataFactory,
        private readonly PropertyAccessorInterface $propertyAccessor
    ) {
    }

    #[LiveAction]
    public function addCollectionItem(#[LiveArg] string $name): void
    {
        $propertyPath = $this->fieldNameToPropertyPath($name, $this->formName);
        $value = $this->initializeValue($propertyPath);

        $data = $this->propertyAccessor->getValue($this->formValues, $propertyPath);
        if (!\is_array($data)) {
            $this->propertyAccessor->setValue($this->formValues, $propertyPath, []);
            $data = [];
        }

        $index = [] !== $data ? max(array_keys($data)) + 1 : 0;
        $this->propertyAccessor->setValue($this->formValues, $propertyPath."[$index]", $value);
    }

    private function deHydrateClass(string $className): array
    {
        $propertyData = [];

        $class = new $className();
        foreach ((new PropertyInfoExtractor([new ReflectionExtractor()]))->getProperties($class::class) as $property) {
            $propValue = $this->propertyAccessor->getValue($class, $property);
            $propertyData[$property] = $this->deHydrateValue($propValue);
        }

        return $propertyData;
    }

    private function deHydrateValue(mixed $value): mixed
    {
        if ($value instanceof \DateTimeInterface) {
            return $value->format(\DateTimeInterface::RFC3339);
        }

        if ($value instanceof \BackedEnum) {
            return $value->value;
        }

        if (\is_bool($value) || null === $value || is_numeric($value) || \is_string($value)) {
            return $value;
        }

        throw new \LogicException(sprintf('Unable to dehydrate value of type "%s".', get_debug_type($value)));
    }

    private function initializeValue(string $propertyPath): array
    {
        $dataClass = $this->getForm()->getConfig()->getDataClass();

        $property = str_replace(['[', ']'], '', $propertyPath);
        $propMetadata = $this->liveComponentMetadataFactory->createLivePropMetadata(
            $dataClass,
            $property,
            new \ReflectionProperty($dataClass, $property),
            new LiveProp()
        );

        $collectionClass = $propMetadata->collectionValueType()?->getClassName();
        if ($collectionClass) {
            return $this->deHydrateClass($collectionClass);
        }

        return [];
    }
}

if this is the right solution, then i will make a PR to change the existing LiveCollectionTrait. With usage of the already existing deHydrateValue function in this package.

smnandre commented 8 months ago

Did it work "before" ? Do you know what changed ?

jpvdw86 commented 8 months ago

Did it work "before" ? Do you know what changed ?

Until 2.7 its works, but after migrating to the latest version. I got now 2 actual issues.

This all is about this change; "[BC BREAK]: LiveProp values are no longer automatically (de)hydrated through Symfony's serializer. Use LiveProp(useSerializerForHydration: true) to activate this. Also, a serializationContext option was added to LiveProp."

And useSerializerForHydratio:true don't fix every problem and result in other issues like circular references.

smnandre commented 8 months ago

Ok, can we see this via another angle ?

This all is about this change; "[BC BREAK]: LiveProp values are no longer automatically (de)hydrated through Symfony's serializer. Use LiveProp(useSerializerForHydration: true) to activate this. Also, a serializationContext option was added to LiveProp."

So "before" 2.8, your values were dehydrated through the serializer and all worked as expected, and "after" 2.8 that did not work.

Can we see if there is something there to check / fix ?