-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4.0] Add prepared statements for mod_related_items #25042
[4.0] Add prepared statements for mod_related_items #25042
Conversation
I have tested this item ✅ successfully on 29acfd9 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25042. |
… prepared-mod-related-items
I have tested this item ✅ successfully on 533ead7 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25042. |
@alikon can you please retest? |
} | ||
|
||
$query->where('(' . implode(' OR ', $wheres) . ')') | ||
->where('(a.publish_up = ' . $db->quote($nullDate) . ' OR a.publish_up <= ' . $db->quote($now) . ')') | ||
->where('(a.publish_down = ' . $db->quote($nullDate) . ' OR a.publish_down >= ' . $db->quote($now) . ')'); | ||
->where('(' . $db->quoteName('a.publish_up') . ' = :nullDate1 OR ' . $db->quoteName('a.publish_up') . ' <= :nowDate1)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use orWhere
but this can be matter for another pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't use orWhere instead we don't use the non alias function extendWhere correct?
should it be
->extendWhere('OR', [$db->quoteName('a.publish_up') . ' = :nullDate1', $db->quoteName('a.publish_up') . ' <= :nowDate1'])
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personal taste, i would prefer orWhere
cause is more readable imo, but...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
joomla-framework/database#174 (comment)
In this issue comment you would remove the orWhere function. now I'm confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never mentioned orWhere
there , but as it is a personal taste go with extendWhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
…ated-items # Conflicts: # modules/mod_related_items/Helper/RelatedItemsHelper.php
I have tested this item ✅ successfully on f79b149 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25042. |
@SharkyKZ can you please retest? |
I have tested this item ✅ successfully on f79b149 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25042. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25042. |
Thanks! |
Summary of Changes
Updated SQL queries to prepared statements and made some cleanups around the queries.
Testing Instructions
Use the module in all ways you can think of.
Expected result
Nothing changed.