neos / form

Flow Form Framework
https://flow-form-framework.readthedocs.org/en/latest/
MIT License
15 stars 36 forks source link

BUGFIX: Allow DateTime for unserialization of form state #127

Closed dlubitz closed 3 years ago

dlubitz commented 3 years ago

Fixes #126

dlubitz commented 3 years ago

Ready for merge and release? Or do you still have any objections @bwaidelich ?

bwaidelich commented 3 years ago

A test would be nice of course, but no general objections.

bwaidelich commented 3 years ago

I wonder if using serialize()/unserialize() to pass the form state is a good idea at all, but

Fair point, but what would you suggest instead?

kdambekalns commented 3 years ago

I wonder if using serialize()/unserialize() to pass the form state is a good idea at all, but

Fair point, but what would you suggest instead?

For the CR node properties we switched to JSON ages ago. But… 🤷‍♂️

bwaidelich commented 3 years ago

For the CR node properties we switched to JSON ages ago

Yes, true that.. Would be a breaking change though because currently you can basically put any object into the form state

bwaidelich commented 3 years ago

you can basically put any object into the form state

..OTOH you actually can't as we see with this fix o.O

kdambekalns commented 3 years ago

Yes. When I added that restriction I (casually) checked FormState and assumed it only contains "harmless" things. Date objects might still count as harmless and that was just an oversight from my side.

But I did not realize one might actually have objects in a form that need to be stored in the FormState in a way that needs serialization…

Edit: FormState accepts anything as a value for a property, but how far does it need to go? What is actually possible in the form framework? I don't know, TBH.