Skip to content
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

Merged
merged 13 commits into from
Sep 2, 2019

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
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants