Merge pull request #972 from matomo-org/PG-3900-improve-variable-refe…

…rences-check Updating reference check to include variables referencing variables
matomo-org · Jan 13, 2025 · b71e23d · b71e23d
2 parents 002ddae + 62a7d20
commit b71e23d
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 7 deletions.
diff --git a/Model/Variable.php b/Model/Variable.php
@@ -167,11 +167,10 @@ public function getContainerVariableReferences($idSite, $idContainerVersion, $id
         }
 
         foreach ($variables as $var) {
-            foreach ($var['typeMetadata']['parameters'] as $parameter) {
-                if ($this->isUsingParameterTheVariable($parameter, $varName)) {
-                    $variableRef = new VariableReference($var['idvariable'], $var['name']);
-                    $references[] = $variableRef->toArray();
-                }
+            $tempReferences = $this->listVariableNamesInParameters($var);
+            if (in_array($varName, $tempReferences)) {
+                $variableRef = new VariableReference($var['idvariable'], $var['name']);
+                $references[] = $variableRef->toArray();
             }
         }
 
@@ -355,7 +354,7 @@ public function getContainerVariables($idSite, $idContainerVersion)
     public function deleteContainerVariable($idSite, $idContainerVersion, $idVariable)
     {
         if ($this->getContainerVariableReferences($idSite, $idContainerVersion, $idVariable)) {
-            throw new \Exception('This variable cannot be deleted as it is used in other places. To remove this variable, first remove all places where this variable is used');
+            throw new \Exception(Piwik::translate('TagManager_ErrorDeleteReferencedVariable'));
         }
         $this->dao->deleteContainerVariable($idSite, $idContainerVersion, $idVariable, $this->getCurrentDateTime());
     }

diff --git a/lang/en.json b/lang/en.json
@@ -1166,6 +1166,7 @@
         "MatomoConfigurationMatomoSetCountPreRenderedTitle": "Enable count sites in pre-rendered state",
         "MatomoConfigurationMatomoSetCountPreRenderedDescription": "If enabled, it will count sites in pre-rendered state.",
         "MatomoConfigurationMatomoSetRequestQueueIntervalTitle": "Set Request Queue Interval",
-        "MatomoConfigurationMatomoSetRequestQueueIntervalDescription": "Defines after how many ms a queued requests will be executed after the request was queued initially. The higher the value the more tracking requests can be sent together at once. interval has to be at least 1000 (1000ms = 1s) and defaults to 2.5 seconds."
+        "MatomoConfigurationMatomoSetRequestQueueIntervalDescription": "Defines after how many ms a queued requests will be executed after the request was queued initially. The higher the value the more tracking requests can be sent together at once. interval has to be at least 1000 (1000ms = 1s) and defaults to 2.5 seconds.",
+        "ErrorDeleteReferencedVariable": "This variable is in use elsewhere and cannot be deleted. Please remove all references to it and try again."
     }
 }
diff --git a/tests/Integration/Model/VariableTest.php b/tests/Integration/Model/VariableTest.php
@@ -10,6 +10,7 @@
 namespace Piwik\Plugins\TagManager\tests\Integration\Model;
 
 use Piwik\Container\StaticContainer;
+use Piwik\Piwik;
 use Piwik\Plugins\TagManager\Context\WebContext;
 use Piwik\Plugins\TagManager\Dao\VariablesDao;
 use Piwik\Plugins\TagManager\Input\Name;
@@ -631,6 +632,42 @@ public function testDeleteContainerVariable()
         $this->assertSame(1, $count);
     }
 
+    public function testDeleteContainerVariableReferencedByVariable()
+    {
+        $variableParams = ['jsFunction' => 'function () { return 12345; }'];
+        $idVariable = $this->addContainerVariable($this->idSite, $this->containerVersion1, CustomJsFunctionVariable::ID, 'TestVariable', $variableParams);
+        $this->assertSame(2, $idVariable);
+
+        $idReferencingVariable = $this->addContainerVariable($this->idSite, $this->containerVersion1, CustomJsFunctionVariable::ID, 'ReferencingTestVariable', $parameters = ['jsFunction' => 'function () { return {{TestVariable}}; }']);
+        $this->assertSame(3, $idReferencingVariable);
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage(Piwik::translate('TagManager_ErrorDeleteReferencedVariable'));
+        $this->model->deleteContainerVariable($this->idSite, $this->containerVersion1, $idVariable);
+    }
+
+    public function testGetVariableReferencesFromVariable()
+    {
+        $variableParams = ['jsFunction' => 'function () { return 12345; }'];
+        $idVariable = $this->addContainerVariable($this->idSite, $this->containerVersion1, CustomJsFunctionVariable::ID, 'TestVariable', $variableParams);
+        $this->assertSame(2, $idVariable);
+
+        $idReferencingVariable = $this->addContainerVariable($this->idSite, $this->containerVersion1, CustomJsFunctionVariable::ID, 'ReferencingTestVariable', $parameters = ['jsFunction' => 'function () { return {{TestVariable}}; }']);
+        $this->assertSame(3, $idReferencingVariable);
+
+        $result = $this->model->getContainerVariableReferences($this->idSite, $this->containerVersion1, $idVariable);
+        $this->assertIsArray($result);
+        $this->assertCount(1, $result);
+        $this->assertSame([
+            [
+                'referenceId' => $idReferencingVariable,
+                'referenceType' => 'variable',
+                'referenceTypeName' => 'Variable',
+                'referenceName' => 'ReferencingTestVariable',
+            ]
+        ], $result);
+    }
+
     public function testGetVariableReferencesWhenNoReferences()
     {
         // we test the references apart from this via API in system tests