review: check if rolesDiagram is not empty before injecting flowchart scripts

damienfern · damienfern · commit 7863d4eb364d · 2025-10-27T11:52:19.000+01:00
diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
@@ -3,10 +3,9 @@ CHANGELOG
 
 7.4
 ---
-
  * Add role hierarchy graph to the profiler security panel
  * Add `debug:security:role-hierarchy` command to dump role hierarchy graphs in the Mermaid.js flowchart format
- * Add `Security::getAccessDecision()` and `getAccessDecisionForUser()` helpers
+* Add `Security::getAccessDecision()` and `getAccessDecisionForUser()` helpers
  * Add options to configure a cache pool and storage service for login throttling rate limiters
  * Register alias for argument for password hasher when its key is not a class name:
 
diff --git a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php
@@ -94,7 +94,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
                 $impersonatorUser = $originalToken->getUserIdentifier();
             }
 
-            if (null !== $this->roleHierarchy) {
+            if ($this->roleHierarchy) {
                 foreach ($this->roleHierarchy->getReachableRoleNames($assignedRoles) as $role) {
                     if (!\in_array($role, $assignedRoles, true)) {
                         $inheritedRoles[] = $role;
@@ -223,7 +223,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
                 $response->headers->clearCookie($authCookieName);
             }
         }
-        if (null !== $this->roleHierarchy) {
+        if ($this->roleHierarchy) {
             $this->data['roles_diagram'] = $this->mermaidDumper->dump($this->roleHierarchy);
         }
     }
diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig
@@ -246,7 +246,7 @@
                                     </td>
                                 </tr>
 
-                                {% if collector.supportsRoleHierarchy %}
+                                {% if collector.supportsRoleHierarchy and collector.rolesDiagram is not empty %}
                                 <script>
                                     {{ source('@WebProfiler/Script/Mermaid/mermaid-flowchart-v2.min.js') }}
                                     const isDarkMode = document.querySelector('body').classList.contains('theme-dark');
diff --git a/src/Symfony/Component/Security/Core/Dumper/MermaidDumper.php b/src/Symfony/Component/Security/Core/Dumper/MermaidDumper.php
@@ -32,7 +32,7 @@ public function dump(RoleHierarchyInterface $roleHierarchy, MermaidDirection $di
         $hierarchy = $this->extractHierarchy($roleHierarchy);
 
         if (!$hierarchy) {
-            return "graph {$direction->value}\n    classDef default fill:#e1f5fe;";
+            return "";
         }
 
         $output = ["graph {$direction->value}"];
diff --git a/src/Symfony/Component/Security/Core/Tests/Dumper/MermaidDumperTest.php b/src/Symfony/Component/Security/Core/Tests/Dumper/MermaidDumperTest.php
@@ -57,8 +57,7 @@ public function testDumpEmptyHierarchy()
         $dumper = new MermaidDumper();
         $output = $dumper->dump($roleHierarchy);
 
-        $this->assertStringContainsString('graph TB', $output);
-        $this->assertStringContainsString('classDef default fill:#e1f5fe;', $output);
+        $this->assertEmpty($output);
     }
 
     public function testDumpComplexHierarchy()

Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep`
`94`	`94`	`$impersonatorUser = $originalToken->getUserIdentifier();`
`95`	`95`	`}`
`96`	`96`
`97`		`- if (null !== $this->roleHierarchy) {`
	`97`	`+ if ($this->roleHierarchy) {`
`98`	`98`	`foreach ($this->roleHierarchy->getReachableRoleNames($assignedRoles) as $role) {`
`99`	`99`	`if (!\in_array($role, $assignedRoles, true)) {`
`100`	`100`	`$inheritedRoles[] = $role;`
`@@ -223,7 +223,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep`
`223`	`223`	`$response->headers->clearCookie($authCookieName);`
`224`	`224`	`}`
`225`	`225`	`}`
`226`		`- if (null !== $this->roleHierarchy) {`
	`226`	`+ if ($this->roleHierarchy) {`
`227`	`227`	`$this->data['roles_diagram'] = $this->mermaidDumper->dump($this->roleHierarchy);`
`228`	`228`	`}`
`229`	`229`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ public function dump(RoleHierarchyInterface $roleHierarchy, MermaidDirection $di`
`32`	`32`	`$hierarchy = $this->extractHierarchy($roleHierarchy);`
`33`	`33`
`34`	`34`	`if (!$hierarchy) {`
`35`		`- return "graph {$direction->value}\n classDef default fill:#e1f5fe;";`
	`35`	`+ return "";`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`$output = ["graph {$direction->value}"];`
Original file line number	Diff line number	Diff line change
`@@ -57,8 +57,7 @@ public function testDumpEmptyHierarchy()`
`57`	`57`	`$dumper = new MermaidDumper();`
`58`	`58`	`$output = $dumper->dump($roleHierarchy);`
`59`	`59`
`60`		`- $this->assertStringContainsString('graph TB', $output);`
`61`		`- $this->assertStringContainsString('classDef default fill:#e1f5fe;', $output);`
	`60`	`+ $this->assertEmpty($output);`
`62`	`61`	`}`
`63`	`62`
`64`	`63`	`public function testDumpComplexHierarchy()`