? Pending

User tests: Successful: Unsuccessful:

avatar alikon
alikon
20 Aug 2019

Pull Request for Issue # #25929 (comment).

Summary of Changes

Fix wrong group by in preset menu joomla.xml

avatar alikon alikon - open - 20 Aug 2019
avatar alikon alikon - change - 20 Aug 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 20 Aug 2019
Category Administration com_menus
avatar alikon alikon - change - 20 Aug 2019
Labels Added: ?
avatar infograf768
infograf768 - comment - 20 Aug 2019

@alikon
This PR is OK for multilingual icons. I see no difference with sqli. Does it solve Postgres?

I suggest though, as I also modified the other 2 presets containing that code, to modify also modern.xml and menus.xml

avatar infograf768
infograf768 - comment - 20 Aug 2019

LOL. Does it solve Postgres?

avatar alikon
alikon - comment - 20 Aug 2019

no unfortunately.... this is only a macroscopic one

avatar infograf768
infograf768 - comment - 20 Aug 2019

My modification was to not use l.image but l.lang_code in the sql_select.
So, shall it also be modified in the sql_group ?

avatar alikon
alikon - comment - 20 Aug 2019

yes this is wrong from 3.x https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_menus/presets/joomla.xml#L233

where we use sql_group="a.id, m.language, l.image"

but the

a.id is not in the select list of fields

avatar infograf768
infograf768 - comment - 20 Aug 2019

I mean, shall we not have also

sql_group="a.id, a.title, a.menutype, m.language, l.lang_code"

instead of

sql_group="a.id, a.title, a.menutype, m.language, l.image"

as we do not use l.image anymore.

avatar alikon
alikon - comment - 20 Aug 2019

yes, my bad, you are right

avatar infograf768
infograf768 - comment - 20 Aug 2019

At looking at @Hackwar error here:
#25929 (comment)
and if I understand German OK, It looks like this could be the solution to the Posgres issue...

avatar infograf768
infograf768 - comment - 20 Aug 2019

Please do it for the other presets too and test in Posgres

avatar infograf768
infograf768 - comment - 20 Aug 2019

Now, can you check Postgres?

avatar alikon
alikon - comment - 20 Aug 2019
select a.id,a.title, a.menutype, CASE COALESCE(SUM(m.home), 0) WHEN 0 THEN '' WHEN 1 THEN CASE m.language WHEN '*' THEN 'class:icon-home' ELSE CONCAT('image:', l.lang_code) END ELSE 'image:mod_languages/icon-16-language.png' END AS icon
from my4_menu_types AS a
left join my4_menu AS m ON m.menutype = a.menutype AND m.home = 1 LEFT JOIN my4_languages AS l ON l.lang_code = m.language
where a.client_id = 0
group by a.id,a.title, a.menutype, m.language, l.lang_code
order by a.id DESC

currently i'm only able to test on postgresql console , and the resulting query is working now....
still not able to save an admin menu, but this is just another issue

avatar infograf768
infograf768 - comment - 20 Aug 2019

let's wait to see if drone gets happy ;)

As for admin menu, see my comments on what is not solved in #25929 Summary of Changes

avatar infograf768 infograf768 - test_item - 20 Aug 2019 - Tested successfully
avatar infograf768
infograf768 - comment - 20 Aug 2019

I have tested this item successfully on 01d1e58

Great!


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

avatar Hackwar Hackwar - test_item - 20 Aug 2019 - Tested successfully
avatar Hackwar
Hackwar - comment - 20 Aug 2019

I have tested this item successfully on 01d1e58


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

avatar infograf768 infograf768 - change - 20 Aug 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-08-20 08:21:11
Closed_By infograf768
avatar infograf768 infograf768 - close - 20 Aug 2019
avatar infograf768 infograf768 - merge - 20 Aug 2019
avatar infograf768
infograf768 - comment - 20 Aug 2019

Thanks!

Add a Comment

Login with GitHub to post a comment