codefog / contao-haste

Haste is a collection of tools and classes to ease working with Contao
http://codefog.pl/extension/haste.html
MIT License
43 stars 24 forks source link

Boolean saveCallbacks in DcaAjaxOperationsListener with strict database #197

Closed rabauss closed 1 year ago

rabauss commented 1 year ago

This issue relates to #196 but I guess we need another solution.

I tried to support haste v5 in m-vo/facebook-import, which uses the haste_ajax_operation for toggling - e.g. see: https://github.com/m-vo/contao-facebook-import/blob/main/src/Resources/contao/dca/tl_mvo_facebook.php#L69

But there is also a save_callback (https://github.com/m-vo/contao-facebook-import/blob/main/src/Resources/contao/dca/tl_mvo_facebook.php#L181) which returns a boolean value

The boolean value cannot be set in the database here: https://github.com/codefog/contao-haste/blob/e32f31cfc4efc0653fa765963ffffedd3cb483c6/src/EventListener/DcaAjaxOperationsListener.php#L75-L78

You will get a database error

Incorrect integer value: '' for column

How can we solve this problem? Can we cast the value after executing the save_callback or do we need to refactor the save_callback of the extension?

EDIT: This problem already exists in v4 😭

see also: https://github.com/m-vo/contao-facebook-import/issues/36

fritzmg commented 1 year ago

It's already a boolean. Why does it try to save an empty string?

rabauss commented 1 year ago

I need to debug that again today - it was to late this night ;-)

qzminski commented 1 year ago

@aschempp @Toflar should we type cast the value based on its SQL definition before it is even evaluated and stored in the database? The single checkbox value in Contao can return a lot of stuff:

Or shall we have an internal mapper that converts the values to boolean for evaluation, and sets the correct type right before updating the database?

Toflar commented 1 year ago

Casting the value to a string for an empty check ('' === (string) $value) together with Widget::getEmptyValueByFieldType() should be your friend. See https://github.com/contao/contao/blob/5.x/core-bundle/contao/drivers/DC_Table.php#L3215-L3231.

rabauss commented 1 year ago

Thanks @Toflar 🧡 that's what I also figured out while analysis. I startet a PR. But I need to test it with my project in the next few days!

rabauss commented 1 year ago

Unfortunately the getEmptyValueByFieldType is not enough or better say in my case it is already false because of the callback but the database update https://github.com/codefog/contao-haste/blob/e32f31cfc4efc0653fa765963ffffedd3cb483c6/src/EventListener/DcaAjaxOperationsListener.php#L78 rejects false with Incorrect integer value: '' for column.

Do we need some special cast for doctrine call?

Toflar commented 1 year ago

Why is your value an empty string if it expects a boolean?

fritzmg commented 1 year ago

Do we need some special cast for doctrine call?

You need to specify the field type otherwise Doctrine will cast everything to string (which is then an empty string for false).