[#47742] - [5.4] Fix Tags With Match All & Include Children
- Pending
- Medium
- Build: 5.4-dev
- # 47742
- Diff
- CSGoat0:fix/tags-match-all-include-children
User tests: Successful: Unsuccessful:
Pull Request resolves #43627
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
Summary of Changes
Removed the condition && $includeChildren != 1 from the HAVING clause check in TagsHelper::getTagItemsQuery().
File changed: libraries/src/Helper/TagsHelper.php (line 418)
Before:
if ($ntagsr > 1 && $anyOrAll != 1 && $includeChildren != 1) {After:
if ($ntagsr > 1 && $anyOrAll != 1) {This change allows the "Match All" setting to work correctly when "Include Child Tags" is enabled. Previously, the HAVING clause was skipped entirely when child tags were included, causing the query to default to "Match Any" behavior.
Testing Instructions
Step 1: Create test tags
- Log in to Joomla Administrator
- Navigate to Components → Tags
- Click New to create the following tags:
- Tag A (e.g., "Fruits")
- Tag B (e.g., "Vegetables")
- For Tag A, create a child tag:
- Click New
- Title: e.g., "Apples"
- In the Parent field, select "Fruits"
- Click Save & Close
- For Tag B, create a child tag:
- Click New
- Title: e.g., "Carrots"
- In the Parent field, select "Vegetables"
- Click Save & Close
Step 2: Create test articles
-
Navigate to Content → Articles
-
Create the following three articles:
Article Title Tags to assign Article 1 Only Tag A (Fruits) Article 2 Tag A (Fruits) AND Tag B (Vegetables) Article 3 Only Tag B (Vegetables) -
For each article, click Save & Close
Step 3: Create the menu item
- Navigate to Menus → All Menu Items (or select your desired menu, e.g., Main Menu)
- Click New
- Menu Title: Enter "Tagged Items Test"
- Menu Item Type: Click Select → under Tags → Tagged Items
- In the Tags field, select both Tag A and Tag B
- Click the Item Selection Options tab
- Match Type: Select All
- Child Tags: Select Include
- Click Save & Close
Step 4: Test on the frontend
- Open your site frontend
- Navigate to the menu item you just created (e.g., "Tagged Items Test")
- Observe which articles are displayed
Expected result
Only Article 2 appears (the article that has both Tag A and Tag B)
Actual result BEFORE applying this Pull Request
When "Match Type: All" and "Child Tags: Include" are both selected with two or more tags:
- Articles appear as if "Match Type: Any" was selected
- Article 1, Article 2, and Article 3 all appear
- Items tagged with only one of the selected tags (or their children) are incorrectly included
Expected result AFTER applying this Pull Request
When "Match Type: All" and "Child Tags: Include" are both selected with two or more tags:
- Only items that have all selected tags (or tags from their child subtrees) appear
- Article 2 appears
- Article 1 and Article 3 do NOT appear
Additional Test Cases for Child Tag Combinations
To verify that the fix works correctly with child tags, please run the following additional tests using the same menu item (keep "Match Type: All" and "Child Tags: Include" selected):
-
Test 1: Replace Tag A (e.g., "Fruits") with one of its children, e.g., "Apples". Keep Tag B as "Vegetables".
Expected result: Same as before — only Article 2 appears (because Article 2 has both "Apples" from Tag A's family and "Vegetables" from Tag B's family). -
Test 2: Replace Tag B (e.g., "Vegetables") with one of its children, e.g., "Carrots". Keep Tag A as "Fruits".
Expected result: Same as before — only Article 2 appears (because Article 2 has both "Fruits" from Tag A's family and "Carrots" from Tag B's family). -
Test 3: Replace Tag A (e.g., "Fruits") with one of its children, e.g., "Apples", AND replace Tag B (e.g., "Vegetables") with one of its children, e.g., "Carrots".
Expected result: Same as before — only Article 2 appears (because Article 2 has both "Apples" from Tag A's family and "Carrots" from Tag B's family). -
Test 4: Replace only one tag (either Tag A or Tag B) with a child tag, and ensure the other tag is NOT present in any article.
Expected result: No articles appear (because no article has tags from both families).
These tests confirm that the "Match All" logic correctly follows tag hierarchies when child tags are included, regardless of whether the selected tags are parents or children.
Link to documentations
Please select:
- Documentation link for guide.joomla.org:
- No documentation changes for guide.joomla.org needed
- Pull Request link for manual.joomla.org:
- No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
PR-5.4-dev
|
||
Tested successfully on 5.4.5 with Patchtester
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
Tested successfully on 5.4.5 with Patchtester
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
@MacJoom Have you used the blue "Test this" button in the issue tracker? It seems your test was not counted. See https://issues.joomla.org/tracker/joomla-cms/47742 .
I have tested this item ✅ successfully on 3d9fc3b
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
I have tested this item ✅ successfully on 3d9fc3b
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
I have tested this item ✅ successfully on 3d9fc3b
This should be documented so that existing site operators who have worked around this issue can quickly find the information they need.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
I have tested this item ✅ successfully on 3d9fc3b
This should be documented so that existing site operators who have worked around this issue can quickly find the information they need.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
| Labels |
Added:
RTC
bug
|
||
@CSGoat0 Final test before merge seems to have failed?
- Created tag hierarchie and for each one article with the same name and this one tag
- Fruits
- Apples
- Granny-Smith
- Golden-Delicious
- Oranges
- Navel-Orange
- Blood-Orange
- Banana
- Apples
- Vegetables
- Carrots
- Imperator-Carrot
- Carrots
- Trees
- Fruits
- Created articles with respective 2/3 tags
- Fruits-Vegetables
- Apples-Oranges-Banana
- Granny-Smith-Blood-Orange
- Apples-Imperator-Carrot
- Granny-Smith-Blood-Orange-Banana
- Created menu items with one tag and respective settings (listed articles/with PR)
- 0-Trees-All-Include (1)
- 1-Fruits-All-Include (13)
- 2-Fruits-Any-Include (13)
- 3-Fruits-Any-Exclude (2)
- 4-Fruits-All-Exclude (2)
- two tags
- 5-Fruits-And-Vegetables-All-Include (16/3)
- 6-Fruits-And-Vegetables-Any-Include (16)
- 7-Fruits-And-Vegetables-Any-Exclude (3)
- 8-Fruits-And-Vegetables-All-Exclude (1)
- tree tags
- 9-Apples-Oranges-Banana-All-Include (11/2)
- 10-Apples-Oranges-Banana-Any-Include (11)
- 11-Apples-Oranges-Banana-Any-Exclude (6)
- 12-Apples-Oranges-Banana-All-Exclude (1)
✅ Applied PR with Patch Tester and the list of articles is reduced for 5-Fruits-And-Vegetables-All-Include and 9-Apples-Oranges-Banana-All-Include. This is correct, e.g. for 9-Apples-Oranges-Banana-All-Include before was the Banana article with only one tag in the list.
❌ For 5-Fruits-And-Vegetables-All-Include the list is reduced, but contains Granny-Smith-Blood-Orange which are two fruits and there is no vegetable tag? I would not expect this article listed.
As there is a lot of configuration it may be easier to import files and database as 47742.zip and 47742.sql – I will share the files for some days. If you use JBT you can simple use scripts/import 47742.sql 47742.zip. Superadmin is ci-admin with joomla-17082005.
I have tested this item 🔴 unsuccessfully on bb871ac
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
I have tested this item 🔴 unsuccessfully on bb871ac
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
| Status | Ready to Commit | ⇒ | Pending |
back to pending
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
back to pending
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
back to pending
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
Hi @muhme, Thanks for the testing.
Could you please provide the exact tags assigned to the article "Granny-Smith-Blood-Orange"?
back to pending
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.
Hi @muhme, Thanks for the testing.
Could you please provide the exact tags assigned to the article "Granny-Smith-Blood-Orange"?
Hi @muhme, Thanks for the testing.
Could you please provide the exact tags assigned to the article "Granny-Smith-Blood-Orange"?
- Article tags are 'Granny-Smith' and 'Blood-Orange'
- 'Granny-Smith' is a child of 'Apples' and a grandchild of 'Fruits'
- 'Blood-Orange' is a child of 'Oranges' and a grandchild of 'Fruits'
- Menu item '5-Fruits-And-Vegetables-All-Include' has tags 'Fruits' and 'Vegetables' and item selection options match type 'All' and child tags 'Include'
I have tested this PR and looks good to me.
I can replicate the error in your test, @muhme but seems that this is due to this:
https://github.com/joomla/joomla-cms/blob/5.4-dev/libraries/src/Helper/TagsHelper.php#L556
Then it is a conceptual error or intendedd behaviour.
I have tested this PR and looks good to me.
I can replicate the error in your test, @muhme but seems that this is due to this:
https://github.com/joomla/joomla-cms/blob/5.4-dev/libraries/src/Helper/TagsHelper.php#L556
Then it is a conceptual error or intendedd behaviour and not related to this PR
Hmm, from reading the code, I am not sure if this PR is complete.
At the top of the function which is modified by this PR, the $ntagsr variable is set to \count($tagIds), and then the $tagIds array is modified in case if $includeChildren is true, but the $ntagsr variable is not updated with the count of the modified array:
$ntagsr = \count($tagIds);
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
}
Later then, the code which used this variable is executed only if $includeChildren is not true without this PR, so that was ok.
Now this PR removes that condition from the check: https://github.com/joomla/joomla-cms/pull/47742/changes#diff-c7b5d9a47462499831f7ddac3c69e7ebd42794c250f04a3d3e9eebe5444f37e4R685-R688
Without this PR:
// Use HAVING if matching all tags and we are matching more than one tag.
if ($ntagsr > 1 && $anyOrAll != 1 && $includeChildren != 1) {
// The number of results should equal the number of tags requested.
$query->having('COUNT(' . $db->quoteName('m.tag_id') . ') = :ntagsr')
->bind(':ntagsr', $ntagsr, ParameterType::INTEGER);
}
That means $ntagsr is not used when $includeChildren != 1.
With this PR:
// Use HAVING if matching all tags and we are matching more than one tag.
if ($ntagsr > 1 && $anyOrAll != 1) {
// The number of results should equal the number of tags requested.
$query->having('COUNT(' . $db->quoteName('m.tag_id') . ') = :ntagsr')
->bind(':ntagsr', $ntagsr, ParameterType::INTEGER);
}
That means with this PR, the $ntagsr might not be right.
Maybe we should add after line 564 a line to update that value when i has been modified?
It would then be:
$ntagsr = \count($tagIds);
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
$ntagsr = \count($tagIds);
}
Or we move the $ntagsr = \count($tagIds); to below that if condition so we have it only once:
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
}
$ntagsr = \count($tagIds);
I am not sure if that really is an issue, I just found it when reading the code.
@CSGoat0 Could you check if my suggestions (chose the one you like more) work with your PR?
Hmm, from reading the code, I am not sure if this PR is complete.
At the top of the function which is modified by this PR, the $ntagsr variable is set to \count($tagIds), and then the $tagIds array is modified in case if $includeChildren is true, but the $ntagsr variable is not updated with the count of the modified array:
$ntagsr = \count($tagIds);
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
}
Later then, the code which used this variable is executed only if $includeChildren is not true without this PR, so that was ok.
Now this PR removes that condition from the check: https://github.com/joomla/joomla-cms/pull/47742/changes#diff-c7b5d9a47462499831f7ddac3c69e7ebd42794c250f04a3d3e9eebe5444f37e4R685-R688
Without this PR:
// Use HAVING if matching all tags and we are matching more than one tag.
if ($ntagsr > 1 && $anyOrAll != 1 && $includeChildren != 1) {
// The number of results should equal the number of tags requested.
$query->having('COUNT(' . $db->quoteName('m.tag_id') . ') = :ntagsr')
->bind(':ntagsr', $ntagsr, ParameterType::INTEGER);
}
That means $ntagsr is not used when $includeChildren is true (or true-ish).
With this PR:
// Use HAVING if matching all tags and we are matching more than one tag.
if ($ntagsr > 1 && $anyOrAll != 1) {
// The number of results should equal the number of tags requested.
$query->having('COUNT(' . $db->quoteName('m.tag_id') . ') = :ntagsr')
->bind(':ntagsr', $ntagsr, ParameterType::INTEGER);
}
That means with this PR, the $ntagsr is also used when $includeChildren is true (or true-ish), but the value might not be right in this case.
Maybe we should add after line 564 a line to update that value when i has been modified?
It would then be:
$ntagsr = \count($tagIds);
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
$ntagsr = \count($tagIds);
}
Or we move the $ntagsr = \count($tagIds); to below that if condition so we have it only once:
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
}
$ntagsr = \count($tagIds);
I am not sure if that really is an issue, I just found it when reading the code.
@CSGoat0 Could you check if my suggestions (chose the one you like more) work with your PR?
Hmm, from reading the code, I am not sure if this PR is complete.
At the top of the function which is modified by this PR, the $ntagsr variable is set to \count($tagIds), and then the $tagIds array is modified in case if $includeChildren is true, but the $ntagsr variable is not updated with the count of the modified array:
$ntagsr = \count($tagIds);
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
}
Later then, the code which used this variable is executed only if $includeChildren is not true without this PR, so that was ok.
Now this PR removes that condition from the check: https://github.com/joomla/joomla-cms/pull/47742/changes#diff-c7b5d9a47462499831f7ddac3c69e7ebd42794c250f04a3d3e9eebe5444f37e4R685-R688
Without this PR:
// Use HAVING if matching all tags and we are matching more than one tag.
if ($ntagsr > 1 && $anyOrAll != 1 && $includeChildren != 1) {
// The number of results should equal the number of tags requested.
$query->having('COUNT(' . $db->quoteName('m.tag_id') . ') = :ntagsr')
->bind(':ntagsr', $ntagsr, ParameterType::INTEGER);
}
That means $ntagsr is not used when $includeChildren is true (or true-ish).
With this PR:
// Use HAVING if matching all tags and we are matching more than one tag.
if ($ntagsr > 1 && $anyOrAll != 1) {
// The number of results should equal the number of tags requested.
$query->having('COUNT(' . $db->quoteName('m.tag_id') . ') = :ntagsr')
->bind(':ntagsr', $ntagsr, ParameterType::INTEGER);
}
That means with this PR, the $ntagsr is also used when $includeChildren is true (or true-ish), but the value might not be right in this case.
Maybe we should add after line 564 a line to update that value when it has been modified?
It would then be:
$ntagsr = \count($tagIds);
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
$ntagsr = \count($tagIds);
}
Or maybe even better, we move the $ntagsr = \count($tagIds); to below that if condition so we have it only once:
// If we want to include children we have to adjust the list of tags.
// We do not search child tags when the match all option is selected.
if ($includeChildren) {
$tagTreeArray = [];
foreach ($tagIds as $tag) {
$this->getTagTreeArray($tag, $tagTreeArray);
}
$tagIds = array_values(array_unique(array_merge($tagIds, $tagTreeArray)));
}
$ntagsr = \count($tagIds);
I am not sure if that really is an issue, I just found it when reading the code.
@CSGoat0 Could you check if my suggestions (chose the one you like more) work with your PR?
Tested successfully on 5.4.5 with Patchtester
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47742.