[RTM] Fix different max length definition of DCA name fields in SQL and eval array

MetaModels / core

MetaModels Core Module

GNU Lesser General Public License v3.0

96 stars 42 forks source link

[RTM] Fix different max length definition of DCA name fields in SQL and eval array #1428

Closed cliffparnitzky closed 3 years ago

cliffparnitzky commented 3 years ago

Description

Some DCA fields have different max length definition in SQL and eval array. This leads for example to cutted text when saving to the database.

Checklist

[x] Read and understood the CONTRIBUTING guidelines
[ ] Created tests, if possible
[ ] All tests passing
[ ] Extended the README / documentation, if necessary
[x] Added myself to the @authors in touched PHP files
[ ] Checked the changes with phpcq and introduced no new issues

cliffparnitzky commented 3 years ago

Currently the name of tl_metamodel_attribute has no maxlength: https://github.com/MetaModels/core/blob/master/src/CoreBundle/Resources/contao/dca/tl_metamodel_attribute.php#L299 ... maybe it is useful to limit it, as all other name fields have a limitation.

cliffparnitzky commented 3 years ago

@discordier @zonky2 What about my comment https://github.com/MetaModels/core/pull/1428#issuecomment-708046762 ???

discordier commented 3 years ago

Currently the name of tl_metamodel_attribute has no maxlength: https://github.com/MetaModels/core/blob/master/src/CoreBundle/Resources/contao/dca/tl_metamodel_attribute.php#L299 ... maybe it is useful to limit it, as all other name fields have a limitation.

I am against doing so, as this would break existing installations. An attribute perfectly working before would suddenly become invalid, and further more, get truncated in the table (as the field is currently unlimited text type) when limiting it. We can do so in 3.0 earliest.

zonky2 commented 3 years ago

Currently the name of tl_metamodel_attribute has no maxlength: https://github.com/MetaModels/core/blob/master/src/CoreBundle/Resources/contao/dca/tl_metamodel_attribute.php#L299 ... maybe it is useful to limit it, as all other name fields have a limitation.

you can open an separate issue and I set this to MM 3.0 https://github.com/MetaModels/core/milestone/30

cliffparnitzky commented 3 years ago

I think it is not necessary. For me, it is okay.