From 7d8d430156cb5553eb9a30102f211ea4433cbaa4 Mon Sep 17 00:00:00 2001
From: gellweiler <sg@gellweiler.net>
Date: Sat, 24 Dec 2022 14:51:00 +0100
Subject: [PATCH] Fix #4049

This commit fixes #4049: Nested tags are not showing under some circumstances.
---
 CRM/Core/BAO/Tag.php                   |  10 +-
 tests/phpunit/CRM/Core/BAO/TagTest.php | 149 +++++++++++++++++++++++++
 2 files changed, 157 insertions(+), 2 deletions(-)
 create mode 100644 tests/phpunit/CRM/Core/BAO/TagTest.php

diff --git a/CRM/Core/BAO/Tag.php b/CRM/Core/BAO/Tag.php
index d02fb85db4dc..e2c8422f19ae 100644
--- a/CRM/Core/BAO/Tag.php
+++ b/CRM/Core/BAO/Tag.php
@@ -89,12 +89,18 @@ public function buildTree($usedFor = NULL, $excludeHidden = FALSE) {
       $thisref['name'] = $dao->name;
       $thisref['description'] = $dao->description;
       $thisref['is_selectable'] = $dao->is_selectable;
-      $thisref['children'] = [];
-
+      if (!isset($thisref['children'])) {
+        $thisref['children'] = [];
+      }
       if (!$dao->parent_id) {
         $this->tree[$dao->id] = &$thisref;
       }
       else {
+        if (!isset($refs[$dao->parent_id])) {
+          $refs[$dao->parent_id] = array(
+            'children' => [],
+          );
+        }
         $refs[$dao->parent_id]['children'][$dao->id] = &$thisref;
       }
     }
diff --git a/tests/phpunit/CRM/Core/BAO/TagTest.php b/tests/phpunit/CRM/Core/BAO/TagTest.php
new file mode 100644
index 000000000000..c3672c0d45e8
--- /dev/null
+++ b/tests/phpunit/CRM/Core/BAO/TagTest.php
@@ -0,0 +1,149 @@
+<?php
+
+/**
+ * Class CRM_Core_BAO_TagTest
+ * @group headless
+ */
+class CRM_Core_BAO_TagTest extends CiviUnitTestCase {
+
+  /**
+   * Set up for class.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function setUp(): void {
+    parent::setUp();
+    $this->quickCleanup(['civicrm_tag']);
+
+    // Create an example hierarchy of tags.
+    // The family tree of Abraham is used as a well known example of a hierarchy (no statement intended).
+    // The order of ids is important because of: https://lab.civicrm.org/dev/core/-/issues/4049, that's why we create Isaac before Abraham.
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_tag (id,name,used_for,is_tagset) VALUES(1, 'Isaac', 'civicrm_contact', 0);");
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_tag (id,name,used_for,is_tagset) VALUES(2, 'Abraham', 'civicrm_contact', 0);");
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_tag (id,name,used_for,is_tagset) VALUES(3, 'Jacob', 'civicrm_contact', 0);");
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_tag (id,name,used_for,is_tagset) VALUES(4, 'Ishmael', 'civicrm_contact', 1);");
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_tag (id,name,used_for,is_tagset) VALUES(5, 'Kedar', 'civicrm_contact', 1);");
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_tag (id,name,used_for,is_tagset) VALUES(6, 'Working', 'civicrm_activity', 1);");
+    CRM_Core_DAO::executeQuery("INSERT INTO civicrm_tag (id,name,used_for,is_tagset) VALUES(7, 'Eating', 'civicrm_activity', 1);");
+
+    // Isaac is the son of abraham
+    CRM_Core_DAO::executeQuery("UPDATE civicrm_tag SET parent_id = 2 WHERE name = 'Isaac';");
+
+    // Jacob is the son of Isaac
+    CRM_Core_DAO::executeQuery("UPDATE civicrm_tag SET parent_id = 1 WHERE name = 'Jacob';");
+
+    // Ishmael is the son of abraham
+    CRM_Core_DAO::executeQuery("UPDATE civicrm_tag SET parent_id = 2 WHERE name = 'Ishmael';");
+
+    // Kedar is the son of Ishmael
+    CRM_Core_DAO::executeQuery("UPDATE civicrm_tag SET parent_id = 4 WHERE name = 'Kedar';");
+  }
+
+  /**
+   * Test that we can generate a correct tree of tags without suppliying additional filters.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testGetTreeWithoutFilters() {
+    $bao = new CRM_Core_BAO_Tag();
+
+    $tree = $bao->getTree();
+
+    $expected = [
+      2 => [
+        'parent_id' => NULL,
+        'name' => 'Abraham',
+        'description' => NULL,
+        'is_selectable' => '1',
+        'children' => [
+          1 => [
+            'parent_id' => '2',
+            'name' => 'Isaac',
+            'description' => NULL,
+            'is_selectable' => '1',
+            'children' => [
+              3 => [
+                'parent_id' => '1',
+                'name' => 'Jacob',
+                'description' => NULL,
+                'is_selectable' => '1',
+                'children' => [],
+              ],
+            ],
+          ],
+          4 => [
+            'parent_id' => '2',
+            'name' => 'Ishmael',
+            'is_selectable' => '1',
+            'description' => NULL,
+            'children' => [
+              5 => [
+                'parent_id' => '4',
+                'name' => 'Kedar',
+                'description' => NULL,
+                'is_selectable' => '1',
+                'children' => [],
+              ],
+            ],
+          ],
+        ],
+      ],
+      7 => [
+        'parent_id' => NULL,
+        'name' => 'Eating',
+        'description' => NULL,
+        'is_selectable' => '1',
+        'children' => [],
+      ],
+      6 => [
+        'parent_id' => NULL,
+        'name' => 'Working',
+        'description' => NULL,
+        'is_selectable' => '1',
+        'children' => [],
+      ],
+    ];
+
+    $this->assertEquals($expected, $tree);
+  }
+
+  /**
+   * Test that we can generate a correct tree of tags when using a usedFor filter and we're excluding hidden tags.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testGetTreeWithFilters() {
+    $bao = new CRM_Core_BAO_Tag();
+
+    $tree = $bao->getTree('civicrm_contact', TRUE);
+
+    $expected = [
+      2 => [
+        'parent_id' => NULL,
+        'name' => 'Abraham',
+        'description' => NULL,
+        'is_selectable' => '1',
+        'children' => [
+          1 => [
+            'parent_id' => '2',
+            'name' => 'Isaac',
+            'description' => NULL,
+            'is_selectable' => '1',
+            'children' => [
+              3 => [
+                'parent_id' => '1',
+                'name' => 'Jacob',
+                'description' => NULL,
+                'is_selectable' => '1',
+                'children' => [],
+              ],
+            ],
+          ],
+        ],
+      ],
+    ];
+
+    $this->assertEquals($expected, $tree);
+  }
+
+}