m-vo / contao-facebook-import

Facebook posts and events for Contao Open Source CMS
Other
11 stars 5 forks source link

Incompatibility of boolean value with strict db mode #36

Closed rabauss closed 1 year ago

rabauss commented 2 years ago

In Contao 4.13 the strict mode for the database is default. Unfortunately, the entity handling with ORM + the contao backend are not fully compatible for boolean values. That's why there is already a save_callback but this is also not compatible anymore.

At the moment you cannot create new facebook nodes, as mentioned in slack - see: https://contao.slack.com/archives/CLTUNH78X/p1653987828597869

If you replace the default dca config here: https://github.com/m-vo/contao-facebook-import/blob/0428b00314452734f7b5bca4e59d4629fae742a3/src/Resources/contao/dca/tl_mvo_facebook.php#L178 to 'default' => 0,, you are able to create node entries again.

I'm not sure if this would be the right solution or if it should be fixed in the core in DC_Table somewhere here: https://github.com/contao/contao/blob/0dd67460a79d81d3821ecfd0aeb42f395c85b194/core-bundle/src/Resources/contao/drivers/DC_Table.php#L632

Furthermore you cannot save node entries anymore because the save_callback is not compatible anymore with strict mode, too. You have to replace: https://github.com/m-vo/contao-facebook-import/blob/0428b00314452734f7b5bca4e59d4629fae742a3/src/Resources/contao/dca/tl_mvo_facebook.php#L183 to return (int) ('1' === $value);.

A temporary solution in my project is at the moment:

// contao/dca/tl_mvo_facebook.php
<?php
$GLOBALS['TL_DCA']['tl_mvo_facebook']['fields']['import_enabled']['default'] = 0;
$GLOBALS['TL_DCA']['tl_mvo_facebook']['fields']['import_enabled']['save_callback'] = [static fn ($v) => (int) ('1' === $v)];
m-vo commented 2 years ago

We added a SQL definition in https://github.com/m-vo/contao-survey/blob/main/src/Resources/contao/dca/tl_survey.php#L85-L95. Would this solve the issue here as well?

fritzmg commented 2 years ago

Very likely, yes.

m-vo commented 2 years ago

@rabauss Would you be able to test this and provide a PR? :see_no_evil:

rabauss commented 2 years ago

Unfortunately with 'sql' => ['type' => 'boolean', 'default' => false] I'm getting again the error while creating:

An exception occurred while executing a query: SQLSTATE[HY000]: General error: 1366 Incorrect integer value: '' for column 'import_enabled' at row 1

So this is not a valid solution! By the way it doesn't matter to use Contao 4.13.4 or 4.13.5 - but I think there were no changes in this direction 😉

fritzmg commented 2 years ago

Are you sure? Can you please post the stack trace and the final DCA.

rabauss commented 2 years ago

Yes, the error is still present with the added sql:

// contao/dca/tl_mvo_facebook.php
<?php
$GLOBALS['TL_DCA']['tl_mvo_facebook']['fields']['import_enabled']['sql'] = ['type' => 'boolean', 'default' => false];

image

and the sql is in the dca - see console debug:dca tl_mvo_facebook: image

rabauss commented 2 years ago

Is this a general problem within doctrine + MariaDB? Because in another project I have also own symfony entities and I cannot execute a sql query with:

$this->connection->executeQuery('UPDATE tl_my_table SET published = :published', ['published' => false]);

I always get the error:

Invalid datetime format: 1366 Incorrect integer value: '' for column `lamp`.`tl_my_table `.`published

With the following query it's working:

$this->connection->executeQuery('UPDATE tl_my_table SET published = :published', ['published' => 0]);

EDIT: But well there is no ORM between so this might be correct 😁

fritzmg commented 2 years ago

It's actually problem with PDO. PHP's PDO MySQL driver casts everything to string and (string) false will thus be an empty string - which is invalid for a tinyint. So in the code you either need to manually cast the boolean to an integer, or you need to provide the type. See also https://github.com/contao/contao/pull/4797