search

modmore / ClientConfig

ClientConfig is a MODX Revolution Extra to allow clients to maintain settings in a user friendly way.
https://docs.modmore.com/en/Open_Source/ClientConfig/index.html
MIT License
28 stars 27 forks source link

window.settings.js save error fix #127

Closed gsm-josh closed 7 years ago

gsm-josh commented 7 years ago

What does it do ?

Fixes a bug in PHP 7+ and MySQL 5.7+ where data types are not changed to the correct database default if entered as an invalid data type. Fix also avoids displaying the default value of "0" if the setting is an image field.

Why is it needed ?

The "source" field did not have a default value in the form and was passing and empty string. This was not being converted to the correct default of 0 before attempting to run the query. Adding the default value of 0 only if the setting has not been set as an image field resolves the issue and does not effect the field display when the setting has been set as an image.

Related issue(s)/PR(s)

Mark-H commented 7 years ago

I'm not entirely sure if I understand what this solves (or in other words: what to test for merging this)... you're setting the value to 0 in the JS, but you make mention of PHP 7 and MySQL 5.7 and database defaults... what am I missing?

gsm-josh commented 7 years ago

Sorry I guess I was being a little too concise.  This may affect earlier versions of php and/or mysql.  I don’t have enough varied systems to really track it down, but I noticed it when upgrading to the latest version of php and mysql (although I did skip some versions in between).  I’ve seen the problem on other sites when I do not submit the correct default value type – mysql throws an error that the value is the incorrect type and it fails to process the query.  For example… in this case, when the field is empty it submits and empty string.  In the schema, database, and map files, it was all defined as a default of 0 and data type of integer.  I was getting an error that the submitted value of “” was invalid and the submission was failing.  However, the displayed query in the error had the correct default of 0.  Setting the default in the form as 0 resolved the issue and allowed it process.

Perhaps a better fix would be to figure out why the correct default is not being applied before the error is thrown but adding the correct default format to the form submission has been a consistent fix.

Mark-H commented 7 years ago

I've merged #130, which accomplishes the same but is a bit of a simpler fix. Thanks tho @gsm-josh!