Skip to content

Conversation

@HLeithner
Copy link
Member

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.

  • create a frontend module
  • create articles with the same metatags
  • You should see related items based on the metatags matching

Expected result

Nothing changed.

@ghost ghost changed the title Add prepared statements for mod_related_items [4.0] Add prepared statements for mod_related_items May 30, 2019
@alikon
Copy link
Contributor

alikon commented Jun 16, 2019

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.

@SharkyKZ
Copy link
Contributor

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.

@ghost
Copy link

ghost commented Jul 22, 2019

@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)')
Copy link
Contributor

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

Copy link
Member Author

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'])
?

Copy link
Contributor

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...

Copy link
Member Author

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.

Copy link
Contributor

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

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
@alikon
Copy link
Contributor

alikon commented Aug 27, 2019

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.

@ghost
Copy link

ghost commented Aug 27, 2019

@SharkyKZ can you please retest?

@Quy
Copy link
Contributor

Quy commented Aug 27, 2019

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.

@Quy
Copy link
Contributor

Quy commented Aug 27, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/25042.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 27, 2019
@wilsonge wilsonge merged commit b46b6c9 into joomla:4.0-dev Sep 2, 2019
@wilsonge
Copy link
Contributor

wilsonge commented Sep 2, 2019

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 2, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Sep 2, 2019
@HLeithner HLeithner deleted the prepared-mod-related-items branch March 29, 2020 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants