Skip to content
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.

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

Symfony 6.4 Update issue #23

Open
On5-Repos opened this issue Apr 22, 2024 · 12 comments
Open

Symfony 6.4 Update issue #23

On5-Repos opened this issue Apr 22, 2024 · 12 comments

Comments

@On5-Repos
Copy link
Contributor

On5-Repos commented Apr 22, 2024

With symfony 6.4 the dynamic fields start to break because Doctrine Annotation is not supported anymore.

After update to 6.4, all forms break with error related to Response Entity.

Uncaught PHP Exception Twig\Error\RuntimeError: "An exception has been thrown during the rendering of a template ("Can't get a way to read the property "Question01" in class "App\Entity\FormResponse".")." at _form.html.twig line 201

If I downgrade to 6.3 then it starts to work. I suspect its related to Doctrine Annotations, as in 6.4 annotations are been completed deprecated without replacement.

Edit: Some more investigation show that TWIG is unable to query the Form --> Questions

@craigh
Copy link
Member

craigh commented Apr 22, 2024

Hi @On5-Repos

sure, annotations are deprecated in Symfony 6 in favor of annotations. A couple ideas you could try:

  1. You could enable annotations in framework.validation.enable_annotations config
  2. You could create your own versions of this library's entities with annotations instead.

I've not worked on this lib much because I didn't know many people are using it. I'd love to learn how you are using it.

@On5-Repos
Copy link
Contributor Author

On5-Repos commented Apr 22, 2024

I like the idea of creating own entities that way future migration to 7 will be easy. used this to allow app users to create custom forms .. I was surprised how much they started to use dynamic forms. Initially, i thought it was not going to be used much... we can make it a more generic bundle for symfony and submit to the community to be approved for use. What are your thoughts?

@On5-Repos
Copy link
Contributor Author

Option 2: created own entities
but its still does not like it, what worked was returning TRUE for __asset magic method.

return isset($this->data[$name]); --> Throws exception that response Data Array does not the the KEY (question)

Remove __isset: If I remove it completely then it complains because of other magic methods are been called in Class.

return true: everything works as expected.

@craigh
Copy link
Member

craigh commented Apr 23, 2024

try changing to

    public function __isset(string $name): bool
    {
        return array_key_exists($name, $this->data);
    }

@craigh
Copy link
Member

craigh commented Apr 23, 2024

Also - (after just now looking at the actual code) the Entities in this bundle are not defined with annotation or attribute but rather by XML definition. So you if you having trouble with annotations, they are almost certainly a result of your own entity definitions within your code and not from this bundle.

@On5-Repos
Copy link
Contributor Author

try changing to

    public function __isset(string $name): bool
    {
        return array_key_exists($name, $this->data);
    }

Yea I tried that as well .. as long as __isset returns false... it breaks .. and starts to complain that Key does not exist in data array.. Which holds answers

@On5-Repos
Copy link
Contributor Author

Also - (after just now looking at the actual code) the entities in this bundle are not defined with annotation or attribute but rather by XML definition. So you if you are having trouble with annotations, they are almost certainly a result of your own entity definitions within your code and not from this bundle.

That's true ... my initial observation was incorrect it's not related to annotations at all. One thing during debug I notice is that field type is set to array in XML. Doctrine has made ARRAY type deprecated and recommends using JSON. I tested by updating xml field definitions to json. it works for all fields after doctrine migration, where i unserialize payload and then Json decoded .. save it back in the database. but for formOptions, it doesn't. As of now it holds Regex Entity in the serialised array for constraints but converts it into json that does come out well.. meaning REGEX ENTITY DEFINITIONS GOES MISSING What are your thoughts? I also try to create entities, but it complaints about duplicate definitions ..

@On5-Repos
Copy link
Contributor Author

On5-Repos commented Apr 27, 2024

Hi @craigh

This fixed the problem, in Symfony 6.4 and I also tested it on 6.3. You can see below, if you are ok i can create a PR for this.

public function __isset(string $name): bool { if (isEmpty($this->data)) { return true; } return array_key_exists($name, $this->data); }

I have updated the unit tests as well, as follow

public function testMagicUnset(): void { $this->responseData->setData(['foo' => 1]); unset($this->responseData->foo); $this->assertFalse(array_key_exists('foo', $this->responseData->getData())); $this->assertNull($this->responseData->foo); // uses __get @phpstan-ignore-line }

Unit Test Results:

`PHPUnit 9.6.12 by Sebastian Bergmann and contributors.

Runtime: PHP 8.2.17
Configuration: /Users/aahmed/Documents/Repos/nParent/vendor/zikula/dynamic-form-bundle/phpunit.xml.dist

............................................................... 63 / 132 ( 47%)
............................................................... 126 / 132 ( 95%)
...... 132 / 132 (100%)

Time: 00:00.348, Memory: 36.00 MB

OK (132 tests, 476 assertions)
`

@craigh
Copy link
Member

craigh commented Apr 27, 2024

this doesn't make any sense:
if (isEmpty($this->data)) { return true; }

  1. there is no php function isEmpty()
  2. if data is empty, then the key you are looking for does not exist, so returning true makes no sense.

@On5-Repos
Copy link
Contributor Author

On5-Repos commented Apr 27, 2024

@craigh correct,
class DynamicFieldsType -- has inherit_data set to true, in this case response data array must have keys set. If i make inherit_data to false. it starts to work. hmm any suggestion ? what you think might be causing it..

Onside note, when I run unit test without on bundle as it is ":1) Zikula\Bundle\DynamicFormBundle\Tests\Form\Type\DynamicFieldsTypeTest::testSubmitValidData
Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException: Can't get a way to read the property "name" in class "Zikula\Bundle\DynamicFormBundle\Entity\AbstractResponseData@anonymous"

its downloading symfony 6.4 components e.g (symfony/form (v6.4.6): Extracting archive)... which also breaks unit test.

Also I found this symfony/symfony#40515

@On5-Repos
Copy link
Contributor Author

@craigh [PropertyAccess] Fix checking for missing properties #54194

did you get anywhere with __isset method?

@On5-Repos
Copy link
Contributor Author

@craigh I have updated __asset to check if key does not exist then set the key and it seems to work ok.

I will also update tests ...

public function __isset(string $name) { if (!array_key_exists($name, $this->data)) { $this->data[$name] = null; } return array_key_exists($name, $this->data); }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants