remove duplicated queries by alikon · Pull Request #19682 · joomla/joomla-cms

alikon · 2018-02-14T20:06:22Z

Summary of Changes

the same query is executed once

Testing Instructions

in a multilanguage site
enable debug
remove duplicated queries #19682 (comment)
look at debug database duplicated queries

Expected result

no duplicated queries

Actual result

duplicate queries one of them is this one

Quy · 2018-02-14T20:09:35Z

administrator/components/com_categories/helpers/categories.php

-				->where('access IN (' . $groups . ')')
-				->where($db->qn('id') . ' = ' . (int) $assocId);
+				->where($db->qn('id') . ' IN (' . $catId . ')')
+				->where($db->qn('published') . ' =  1')


Remove extra space after =

Quy · 2018-02-14T20:10:20Z

administrator/components/com_categories/helpers/categories.php

+				$assocId  = $arrId[0];
+			}
+
+			$catId    = implode(',', $assocId);


Remove extra spaces before =

ethernidee · 2018-02-14T21:09:31Z

What do you think about renaming $filters to something like $cachedAssociations? Perhaps, it will be clearer, what is stored inside.

FILE: ...omla-cms/administrator/components/com_categories/helpers/categories.php
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
140 | ERROR | Space before closing parenthesis of function call prohibited

alikon · 2018-02-17T07:38:20Z

from my personal pov is explained in declaration Cached array of the category item id.
but if you feel that renaming is better .... the pr branch is public....

Quy · 2018-02-17T10:37:01Z

administrator/components/com_categories/helpers/categories.php

+			{
+				// Include only published categories with user access
+				$arrId    = explode(':', $langAssociation->id);
+				$assocId  = $arrId[0];


Isn’t $assocId being reassigned each time and not array as to be expected?

right, fixed

Quy · 2018-03-09T04:17:25Z

The return values are different.

Before PR:

array(2) {
  ["fr-FR"]=>
  string(18) "11:categorie-fr-fr"
  ["es-ES"]=>
  string(18) "12:categoria-es-es"
}

After PR:

array(2) {
  ["fr-FR"]=>
  string(2) "11"
  ["es-ES"]=>
  string(2) "12"
}

ladyjer · 2018-04-05T19:04:23Z

I have tested this item ✅ successfully on 7d724e0

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19682.}

Fix array's values to match previous syntax

ghost · 2018-04-27T05:05:28Z

@ladyjer can you please retest?

infograf768 · 2018-04-28T06:30:06Z

Notice: Undefined variable: assocId in /Applications/MAMP/htdocs/installmulti/trunkgitnew/administrator/components/com_categories/helpers/categories.php on line 135

Warning: implode(): Invalid arguments passed in /Applications/MAMP/htdocs/installmulti/trunkgitnew/administrator/components/com_categories/helpers/categories.php on line 135

1064 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 ') AND published = 1 AND access IN (1,1,5)' at line 3

infograf768 · 2018-04-28T08:01:31Z

administrator/components/com_categories/helpers/categories.php

+			$user             = JFactory::getUser();
+			$groups           = implode(',', $user->getAuthorisedViewLevels());
+
+			// No assocations


No associations

infograf768 · 2018-04-28T08:15:28Z

Test instructions should be updated.
This PR (after last correction) is checking duplicate queries of associated categories, NOT category menu items.
We can indeed associate category menu items without ever associating the categories concerned.

Therefore, to test, one should not associate the menu items, only the categories concerned.

Needs totally new tests.

alikon · 2018-04-28T08:24:27Z

updated

Needs totally new tests.

👍

Quy · 2018-05-06T20:38:33Z

I have tested this item ✅ successfully on 2d30362

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19682.}

csthomas · 2018-05-08T07:01:35Z

@alikon Are you sure we need static::$filters at all?

Before, the sql query was executed inside a loop, now it is executed outside. I have not yet tested it.

viocassel · 2019-02-03T21:48:33Z

I have tested this item ✅ successfully on 2d30362

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19682.}

Quy · 2019-02-03T22:24:28Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19682.}

alikon · 2019-05-02T16:45:42Z

@HLeithner please a final response

1 year old rtc pr

HLeithner · 2019-05-10T12:49:21Z

@alikon thx for you work but this is an optimization that should go into j4 and not in j3 because it doesn't fix a bug...

If this query still exists in j4 it would be great if you could rebase this pr to 4.0-dev

ymmv

[com_categories][Multilanguage] - remove duplicated queries

a370929

joomla-cms-bot added the PR-staging label Feb 14, 2018

Quy reviewed Feb 14, 2018

View reviewed changes

alikon mentioned this pull request Feb 14, 2018

Multilingual: Associated categories should display only when published #19551

Merged

cs

c9658d4

drone fix

0088627

Quy reviewed Feb 17, 2018

View reviewed changes

missed []

7d724e0

Quy and others added 2 commits April 21, 2018 13:28

Fix array's values to match previous syntax

2b9773f

Merge pull request #40 from Quy/patch-18

a74c9a3

Fix array's values to match previous syntax

no category assocations

6756b24

infograf768 reviewed Apr 28, 2018

View reviewed changes

typo

2d30362

infograf768 approved these changes May 7, 2018

View reviewed changes

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 3, 2019

ghost added the J3 Issue label Apr 5, 2019

ghost removed the J3 Issue label Apr 19, 2019

ghost changed the title ~~[com_categories][Multilanguage] - remove duplicated queries~~ remove duplicated queries Apr 19, 2019

HLeithner closed this May 10, 2019

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 10, 2019

Uh oh!

Conversation

alikon commented Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Testing Instructions

Expected result

Actual result

Uh oh!

Quy Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

alikon Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

Quy Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

alikon Feb 14, 2018

Choose a reason for hiding this comment

Uh oh!

ethernidee commented Feb 14, 2018

Uh oh!

alikon commented Feb 17, 2018

Uh oh!

Quy Feb 17, 2018

Choose a reason for hiding this comment

Uh oh!

alikon Feb 17, 2018

Choose a reason for hiding this comment

Uh oh!

Quy commented Mar 9, 2018

Uh oh!

ladyjer commented Apr 5, 2018

Uh oh!

ghost commented Apr 27, 2018

Uh oh!

infograf768 commented Apr 28, 2018

Uh oh!

infograf768 Apr 28, 2018

Choose a reason for hiding this comment

Uh oh!

infograf768 commented Apr 28, 2018

Uh oh!

alikon commented Apr 28, 2018

Uh oh!

Quy commented May 6, 2018

Uh oh!

csthomas commented May 8, 2018

Uh oh!

viocassel commented Feb 3, 2019

Uh oh!

Quy commented Feb 3, 2019

Uh oh!

alikon commented May 2, 2019

Uh oh!

HLeithner commented May 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

alikon commented Feb 14, 2018 •

edited

Loading