Skip to content

Module Popular Tags is not working properly #9351#9376

Merged
rdeutz merged 3 commits intojoomla:stagingfrom
alikon:patch-59
Apr 13, 2016
Merged

Module Popular Tags is not working properly #9351#9376
rdeutz merged 3 commits intojoomla:stagingfrom
alikon:patch-59

Conversation

@alikon
Copy link
Copy Markdown
Contributor

@alikon alikon commented Mar 11, 2016

Pull Request for Issue #9351 .

Summary of Changes

queryA = current query
if order = title
select a.tag_id, a.count, a.title, a.access, a.alias from (queryA) A order by a.title DESC

Testing Instructions

settings: order = title

#9351 (comment)

example:

tag | count
fauna | 5
flora | 4
nature | 2
oceans | 30

In module we want to see 3 most popular item, sorting by title.

Expecting:
fauna, flora, oceans ("slice" 3 most popular, and then sort by the title)

Actual Result:

fauna, flora, nature (sort by title, and then slice 3). MOST POPULAR TAG ON THE SITE "OCEANS" NOT SHOWING!

@brianteeman brianteeman mentioned this pull request Mar 11, 2016
Closed
@vitaly80
Copy link
Copy Markdown

vitaly80 commented Mar 11, 2016

I have tested this item 🔴 unsuccessfully on 28eeb62

get error, if enabled "Random" sort option in module. in other cases - works perfect! thanks! Error log:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'LIMIT 0, 5' at line 7 SQL=SELECT MAX(`tag_id`) AS tag_id, COUNT(*) AS count,MAX(t.title) AS title,MAX(`t`.`access`) AS access,MAX(`t`.`alias`) AS alias FROM `zwdj7_contentitem_tag_map` AS `m` INNER JOIN `zwdj7_tags` AS `t` ON `tag_id` = t.id INNER JOIN `zwdj7_ucm_content` AS `c` ON `m`.`core_content_id` = `c`.`core_content_id` WHERE `t`.`access` IN (1,1,5) AND `t`.`published` = 1 AND `m`.`type_alias` = `c`.`core_type_alias` AND `c`.`core_state` = 1 AND (`c`.`core_publish_up` = '0000-00-00 00:00:00' OR `c`.`core_publish_up` <= '2016-03-11 23:29:56') AND (`c`.`core_publish_down` = '0000-00-00 00:00:00' OR `c`.`core_publish_down` >= '2016-03-11 23:29:56') GROUP BY `tag_id`,`title`,`access`,`alias` ORDER BY LIMIT 0, 5
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9376.

@alikon
Copy link
Copy Markdown
Contributor Author

alikon commented Mar 12, 2016

something went wrong on your side applying the pr i guess ?
when order = rand() alias Random is selected
then this code
https://github.com/alikon/joomla-cms/blob/patch-59/modules/mod_tags_popular/helper.php#L88
should render something like this
ORDER BY RAND() LIMIT 0, 5
and not like
ORDER BY <something is missing> LIMIT 0, 5

wich version of joomla are you using?

@vitaly80
Copy link
Copy Markdown

vitaly80 commented Mar 12, 2016

I use now Joomla! 3.4.8

@vitaly80
Copy link
Copy Markdown

vitaly80 commented Mar 12, 2016 

$query->order('rand()');

works perfect!

@alikon
Copy link
Copy Markdown
Contributor Author

alikon commented Mar 12, 2016

yes under 3.4.8 cause of this since 3.5
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query/mysqli.php#L134

can you test under current staging?

@vitaly80
Copy link
Copy Markdown

vitaly80 commented Mar 12, 2016

i.e. on 3.5?

@alikon
Copy link
Copy Markdown
Contributor Author

alikon commented Mar 12, 2016

i think was merged on 16 Jul 2015 on 3.5 branch
d6f35c3#diff-515fdfdc216c99a6f7795cd28c915887

@vitaly80
Copy link
Copy Markdown

vitaly80 commented Mar 12, 2016

I try it for 3.5?

@alikon
Copy link
Copy Markdown
Contributor Author

alikon commented Mar 12, 2016

yes https://github.com/joomla/joomla-cms/releases/tag/3.5.0-rc 2 days ago

@vitaly80
Copy link
Copy Markdown

vitaly80 commented Mar 12, 2016

on 3.5 works perfect.

@vitaly80
Copy link
Copy Markdown

vitaly80 commented Mar 12, 2016

I have tested this item ✅ successfully on 28eeb62

Works fine on Joomla! 3.5. Thanks!

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

@zero-24
Some CS Fixes for #9376
d3cc11c 
@alikon this here are some things to make the code more readable.
@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Mar 13, 2016

I have tested this item ✅ successfully on 28eeb62

Works good here. Please see for a CS improvment here: alikon#18

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

@joomla-cms-bot
Copy link
Copy Markdown

joomla-cms-bot commented Mar 13, 2016

This PR has received new commits.

CC: @vitaly80, @zero-24

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

@brianteeman
Copy link
Copy Markdown
Contributor

brianteeman commented Mar 13, 2016

As the last changes were just code style I am setting this RTC as we have two good tests

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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 13, 2016
@rdeutz rdeutz added this to the Joomla 3.5.2 milestone Apr 13, 2016
@rdeutz rdeutz merged commit f841943 into joomla:staging Apr 13, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 13, 2016
@rdeutz rdeutz modified the milestones: Joomla 3.5.2, Joomla! 3.6.0 May 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Joomla 3.6.0

Development

Successfully merging this pull request may close these issues.

6 participants

@alikon @vitaly80 @zero-24 @joomla-cms-bot @brianteeman @rdeutz