CS after merge https://github.com/joomla/joomla-cms/pull/7540 #11843
Conversation
| $query = $db->getQuery(true) | ||
| ->select('DISTINCT position') | ||
| ->from($db->quoteName('#__modules')) | ||
| ->where($db->quoteName('client_id') . ' = ' . $db->quote($clientId)) |
There was a problem hiding this comment.
why quote an integer?
There was a problem hiding this comment.
We shouldn't be quoting int's. In one of the db languages i remember that caused massive problems
There was a problem hiding this comment.
I've heard that there is a performance cost to quoting ints in that mysql needs to do a type conversion from string to the datatype of the column. Typically quoting is related to reducing SQL injection risks.
If I remember right Quoting * (an asterisk) in a select causes issues on MySQL but I'm unaware of quoting ints causing problems with the non-MySQL db languages.
reference http://www.sigmainfy.com/blog/sql-single-quotes-for-numeric-values-is-it-really-good-practice.html
"Do not use quote around numeric values in SQL query for performance. Rather, make sure in your say PHP code, make sure the numeric values are indeed valid numeric ones before making up the complete SQL query to protect your db from SQL injection."
|
Hmm travis fails on the last commit? But it don't let me go into the report. Can you double check that? |
|
it was/is the javascript part, doing my best to make it green |
|
can't make it green and it was failing at the moment it was merged not after |
Pull Request for Issue #7540 @rdeutz
Summary of Changes
CS after merge: #7540 (comment)
Testing Instructions
Edit the index.php of the protostar template and rename the module position in line 177 from
<jdoc:include type="modules" name="position-8" style="xhtml" />to
<jdoc:include type="modules" name="test123" style="xhtml" />Now go to the module and create a new custom module and place it in position test123 (it wont be on the list you have to manually enter it)
Go to global configuration and make sure that you have "Mouse-over Edit Icons for" set for modules
Go to the front end of the site and check the new module is displayed and then log in
Hover over the module and you get an icon to let you open the module for editing. Change the module title and press save. The module will have been saved in a different position.
Now apply the patch. Go to module manager and put the module back into test123 and repeat
You will now see that when you edit the module you will see at the bottom of the list of module positions a section "Active positions" and test123 is there. Dont change the position. Edit the title and sabe the module. The module will have been saved in the same position
Documentation Changes Required
None