User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Category manager creates 1 query per category to get the assigned item counts by state
These are used to displays the columns
total published, total unpublished, total archived, total trashed
Both 1st and 2nd tabs should show the same counted items per state
Category managers to test
Tags manager:
1 query to get the total assigned items for the categories shown by current page
20, 50, 100 queries to get the assigned items for the categories
None
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_content |
Labels |
Added:
?
|
Category | Administration com_content | ⇒ | Administration com_banners com_contact com_content com_fields com_newsfeeds Libraries |
Title |
|
countTagItems()
you can accuse me for that
That PR has a completely different purpose than PR #21667,
PR #21667 tries to make a reusable method in a trait
Furthermore this PR not only replaces multiple queries for category assignments with 1 query (this is the purpose of this PR)
but this PR also fixes same issue with Tag counting which PR #21667 does not even touch
A note
the code here is agnostic in regards to which state / condition exist
thus it can be rather easily ported to J4
please, can I ask you if can we concentrate our effort on 3.x series for a while !!
if we can improve performance in the current staging....it will'be a real gain
4.x ..... it will come later......
please, can I ask you if can we concentrate our effort on 3.x series for a while !!
if we can improve performance in the current staging....it will'be a real gain
4.x ..... it will come later......
sure, just i mention that the configuration object of the method countRelations() is structured having in mind to make easy
sorry for my idiosyncrasy... i just want to plant my foots on earth and not on mars
20, 50, 100 queries to get the assigned items for the categories
This function is used only in the back end category list view. As far as I can see it, it is executed only once. But please prove that I'm wrong.
This function is used only in the back end category list view. As far as I can see it, it is executed only once. But please prove that I'm wrong.
Sure here is the proof
please notice the existing code inside
helper classes
e.g. for categories of articles ($items contains category objects)
foreach ($items as $item)
{
$item->count_trashed = 0;
$item->count_archived = 0;
$item->count_unpublished = 0;
$item->count_published = 0;
$query = $db->getQuery(true);
$query->select('state, count(*) AS count')
->from($db->qn('#__content'))
->where('catid = ' . (int) $item->id)
->group('state');
$db->setQuery($query);
$articles = $db->loadObjectList();
...
}
and then also i tested to confirm that they are executed, and then spent considerable time for this PR also revising code for tags assignments too
You can confirm the multiple queries by looking at the Debug queries slider,
and then also inside the items loop after:
$articles = $db->loadObjectList();
you can add
echo 'Executed : ' . $query . '<br><br>';
I speak of the multiple sql queries (20, 50, 100) that a single call to the methods will do
i never said that the method is called multiple times
Now I got you.
What's the idea behind the config variable?
ggppdk
Furthermore this PR not only replaces multiple queries for category assignments with 1 query (this is the purpose of this PR)
but this PR also fixes same issue with Tag counting which PR #21667 does not even touch
I didn't noticed that you changed the complete PR to more then changing the DB query sorry I missed this I talked about this part.
What's the idea behind the config variable?
basically it makes possible the method to be reusable
we now have 1 method in the parent class and 5 countItems() and 3 counTagItems() methods in descendants helpers classes call it
also copying this from my previous answer
sure, just i mention that the configuration object of the method countRelations() is structured having in mind to make easy
- to be used by J3 components with custom states / custom states subsets
- to be used (after ported) with a custom subset of "conditions" for J4 too
e.g. banner has different table name and different column name than content
also the counting for fields in fieldgroups does not use 'catid' column, the column name is 'group_id'
I have tested this item
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
@ggppdk Can you explain why you have:
public static function countItems(&$items, $config = null)
{
$config = $config ?: (object) array(
'related_tbl' => 'banners',
'state_col' => 'state',
'group_col' => 'catid',
'relation_type' => 'category_or_group',
);
return parent::countRelations($items, $config);
}
IMO this would be better:
public static function countItems(&$items)
{
$config = (object) array(
'related_tbl' => 'banners',
'state_col' => 'state',
'group_col' => 'catid',
'relation_type' => 'category_or_group',
);
return parent::countRelations($items, $config);
}
If someone would like to change some parameter then should use BannersHelper::countRelations($items, $config)
directly.
indeed if someone would want to count relations with a custom table / custom case,
then someone could call somenameHelper::countRelations($items, $config)
which is the method inherited by the parent class
So indeed it seems to me that it can be as you say,
we do not need $config = null
variable in methods
public static function countItems(&$items, $config = null)
public static function countTagItems(&$items, $extension, $config = null)
they can be (have same signature like before this)
public static function countItems(&$items)
public static function countTagItems(&$items)
Status | Ready to Commit | ⇒ | Pending |
Status back on Pending as stated above.
I have tested this item
There is a problem with mark this test as success on github.
I got error "Bad credentials" at
https://issues.joomla.org/tracker/joomla-cms/22117
I have tested this item successfully but I got error "Bad credentials" at https://issues.joomla.org/tracker/joomla-cms/22117
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-11-01 02:34:25 |
Closed_By | ⇒ | mbabker | |
Labels |
Added:
?
|
yes that was my thought too
that is why those local variables in the first commit for the columns names, that seemed redundant ...
I intended to replace countItems() method everywhere, but i see that countTagItems() has the same problem of 20, 50, 100 queries instead of 1
so i have added a countRelations() method to parent helper class that replace both methods,
actually the methods are not replaced instead they call the parent class method like this
parent::countRelations($items, ...,$config);