Work on Site components #12261
Work on Site components #12261
Conversation
…, otherwise concatenated. The issue here ,is of course what's the best way to deal with complicated conditions or otherwise dynamically created SQL.
| $quotedKeyword = $db->quote($keyword); | ||
| $prefixCondition = ($prefix === substr($keyword, 0, strlen($prefix)) ? '1' : '0'); | ||
|
|
||
| $condition1 = /** @lang SQL */ |
There was a problem hiding this comment.
in this part the fields names should use the API quoteName
Also, i think the API has the ´orWhere´ and ´andWhere´
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query.php#L1429
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query.php#L1448
There was a problem hiding this comment.
Yes, that's why I wrote the comment below those lines.
What I did in this part, was to just refactor concatenated strings (without the query builder) into HEREDOC strings.
As I noted in those comments, it will not make any sense, if the query builder is used instead and all queries will be migrated to that methodology.
There was a problem hiding this comment.
one relevant information
decide whether or not we are going to enforce the use of quoteName() and quote() in database queries. (Roland)
The PLT has decided that it will enforce the use of quoteName() and quote() methods on any new pull requests involving database queries. This to have consistent database queries that will work best across SQL platforms and with maximum security over time.
|
Hey I'm going to be a pain! I'm afraid this really is too much for me to go through in one go (i can just about manage the other ones). Can you open these on a component by component basis please so I have a chance at being able to review them! Thanks! |
|
No issues, will have a shot at that later. |
|
Thankyou! |
|
@wilsonge Is it OK to bundle a few smaller changes in 1-3 components or do you want PR's really by single component? |
|
It's ok :) |
… (#12292) * Cleanups, fixes and a bit of optimizations for site/components batch #3 - com_content Note: This is a single commit bundling all types of changes, since PR #12261 which had detailed commits, was rejected as a whole * Ch-Ch-Ch-Changes! Made some changes as pointed out by @andrepereiradasilva * A bit more... * Revert * Removed empty function, as there is a fallback. Change made according to comment from @wilsonge * Included @andrepereiradasilva's suggestions * Inserting whitespace before php closing tag
… (#12290) * Cleanups, fixes and a bit of optimizations for site/components batch #1 - com_ajax - com_banners - com_config Note: This is a single commit bundling all types of changes, since PR #12261 which had detailed commits, was rejected as a whole * Ch-Ch-Changes! * Removed the query changes * Fix for wrong merge resolve
… (#12293) * Cleanups, fixes and a bit of optimizations for site/components batch #4 - com_mailto - com_newsfeeds - com_search Note: This is a single commit bundling all types of changes, since PR #12261 which had detailed commits, was rejected as a whole * Missed to convert `and` and `or` * Some more fixes after conflict resolution * - Changed some things according to reviewer's suggestions - Some more changes in updated stuff * Reversed a change according to reviewer's correct comment.
… (#12294) * Cleanups, fixes and a bit of optimizations for site/components batch #5 - com_tags - com_users - com_wrapper Note: This is a single commit bundling all types of changes, since PR #12261 which had detailed commits, was rejected as a whole * Some more fixes after conflict resolution * - Changed some things according to reviewer's suggestions - Some more changes in updated stuff
Another big batch:
Cleanups, fixes and a bit of optimizations for site/components
The details are in the commit messages.
Please also check my // Todo: ... on the proposal for HEREDOC with SQL queries