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 tags popular #25043

Merged
merged 14 commits into from Jul 23, 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 tags
  • check the tags in the module if its the same without the PR

Expected result

Nothing changed.

@ghost ghost changed the title Add prepared statements for mod tags popular [4.0] Add prepared statements for mod tags popular May 30, 2019
@@ -116,22 +123,24 @@ public static function getList(&$params)
$query->order('count DESC');
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply $db->quoteName?

@alikon
Copy link
Contributor

alikon commented Jun 16, 2019

I have tested this item ✅ successfully on 86ee169


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

@Quy
Copy link
Contributor

Quy commented Jun 20, 2019

Error: Unknown column 'MAX(tag_id) AS tag_id' in 'field list': Unknown column 'MAX(tag_id) AS tag_id' in 'field list'

@Quy
Copy link
Contributor

Quy commented Jun 20, 2019

Warning: Illegal offset type in \libraries\vendor\joomla\database\src\Mysqli\MysqliQuery.php on line 123

Warning: mysqli_stmt::bind_param(): Number of variables doesn't match number of parameters in prepared statement in \libraries\vendor\joomla\database\src\Mysqli\MysqliStatement.php on line 425

@alikon
Copy link
Contributor

alikon commented Jun 21, 2019

could be related to this line https://github.com/joomla/joomla-cms/pull/25043/files#diff-b76ce82322bfdac12f53dcffdcc8a16cR104 the actual bind() doesn't allow what has been made here
probably a composer update will solve

@HLeithner
Copy link
Member Author

I closed my own PR joomla-framework/database#162 accidentally, this one is needed for this PR to work.

Applying the database Pr makes this one work again...

@alikon
Copy link
Contributor

alikon commented Jun 21, 2019 via email

# Conflicts:
#	modules/mod_tags_popular/Helper/TagsPopularHelper.php
Co-Authored-By: SharkyKZ <sharkykz@gmail.com>
@SharkyKZ
Copy link
Contributor

I have tested this item ✅ successfully on 19d980f


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

@ghost
Copy link

ghost commented Jul 22, 2019

@alikon can you please retest?

@Quy
Copy link
Contributor

Quy commented Jul 22, 2019

I have tested this item ✅ successfully on 19d980f


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

@Quy
Copy link
Contributor

Quy commented Jul 22, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 22, 2019
@wilsonge wilsonge merged commit ba51dc5 into joomla:4.0-dev Jul 23, 2019
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 23, 2019
@wilsonge wilsonge added this to the Joomla 4.0 milestone Jul 23, 2019
@HLeithner HLeithner deleted the prepared-mod-tags-popular branch March 29, 2020 19:15
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

6 participants