Move flash message context to the shaped array

Does not group the messages by context. Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
phpmyadmin · May 16, 2024 · c10d265 · c10d265
1 parent 64a8a9d
commit c10d265
Show file tree

Hide file tree

Showing 9 changed files with 92 additions and 118 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -7471,12 +7471,12 @@ parameters:
 			path: src/FlashMessenger.php
 
 		-
-			message: "#^Cannot access offset string on mixed\\.$#"
-			count: 2
+			message: "#^Method PhpMyAdmin\\\\FlashMessenger\\:\\:getCurrentMessages\\(\\) should return array\\<int, array\\{context\\: string, message\\: string, statement\\: string\\}\\> but returns mixed\\.$#"
+			count: 1
 			path: src/FlashMessenger.php
 
 		-
-			message: "#^Property PhpMyAdmin\\\\FlashMessenger\\:\\:\\$previousMessages \\(array\\<string, array\\<array\\<string\\>\\>\\>\\) does not accept mixed\\.$#"
+			message: "#^Property PhpMyAdmin\\\\FlashMessenger\\:\\:\\$previousMessages \\(array\\<int, array\\{context\\: string, message\\: string, statement\\: string\\}\\>\\) does not accept mixed\\.$#"
 			count: 1
 			path: src/FlashMessenger.php
 
@@ -15545,16 +15545,6 @@ parameters:
 			count: 1
 			path: tests/unit/Export/OptionsTest.php
 
-		-
-			message: "#^Parameter \\#2 \\$array of static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertArrayHasKey\\(\\) expects array\\|ArrayAccess, mixed given\\.$#"
-			count: 2
-			path: tests/unit/FlashMessengerTest.php
-
-		-
-			message: "#^Parameter \\#2 \\$array of static method PHPUnit\\\\Framework\\\\Assert\\:\\:assertArrayNotHasKey\\(\\) expects array\\|ArrayAccess, mixed given\\.$#"
-			count: 2
-			path: tests/unit/FlashMessengerTest.php
-
 		-
 			message: "#^Cannot access offset 'queries' on mixed\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -5944,12 +5944,17 @@
   </file>
   <file src="src/FlashMessenger.php">
     <MixedArrayAssignment>
-      <code><![CDATA[$this->storage[self::STORAGE_KEY][$key]]]></code>
-      <code><![CDATA[$this->storage[self::STORAGE_KEY][$key]]]></code>
+      <code><![CDATA[$this->storage[self::STORAGE_KEY][]]]></code>
     </MixedArrayAssignment>
     <MixedAssignment>
       <code><![CDATA[$this->previousMessages]]></code>
     </MixedAssignment>
+    <MixedInferredReturnType>
+      <code><![CDATA[list<array{context: string, message: string, statement: string}>]]></code>
+    </MixedInferredReturnType>
+    <MixedReturnStatement>
+      <code><![CDATA[$this->storage[self::STORAGE_KEY]]]></code>
+    </MixedReturnStatement>
   </file>
   <file src="src/Font.php">
     <MixedArgument>
@@ -13643,18 +13648,6 @@
     <InvalidScalarArgument>
       <code><![CDATA[$_SESSION]]></code>
     </InvalidScalarArgument>
-    <MixedArgument>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]]]></code>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]]]></code>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]]]></code>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]]]></code>
-    </MixedArgument>
-    <MixedArrayAccess>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]['error']]]></code>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]['error']]]></code>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]['success']]]></code>
-      <code><![CDATA[$_SESSION[self::STORAGE_KEY]['success']]]></code>
-    </MixedArrayAccess>
   </file>
   <file src="tests/unit/FooterTest.php">
     <DeprecatedMethod>

diff --git a/resources/templates/database/structure/index.twig b/resources/templates/database/structure/index.twig
@@ -1,13 +1,11 @@
-{% for flash_key, flash_messages in flash() %}
-  {% for flash_message in flash_messages %}
-    {% if flash_message.statement is empty %}
-      <div class="alert alert-{{ flash_key }}" role="alert">
-        {{ flash_message.message }}
-      </div>
-    {% else %}
-      {{ statement_message(flash_message.message, flash_message.statement, flash_key) }}
-    {% endif %}
-  {% endfor %}
+{% for flash_message in flash() %}
+  {% if flash_message.statement is empty %}
+    <div class="alert alert-{{ flash_message.context }}" role="alert">
+      {{ flash_message.message }}
+    </div>
+  {% else %}
+    {{ statement_message(flash_message.message, flash_message.statement, flash_message.context) }}
+  {% endif %}
 {% endfor %}
 
 {% if has_tables %}

diff --git a/resources/templates/export.twig b/resources/templates/export.twig
@@ -7,16 +7,14 @@
   {{ page_settings_error_html|raw }}
   {{ page_settings_html|raw }}
 
-  {% for flash_key, flash_messages in flash() %}
-    {% for flash_message in flash_messages %}
-      {% if flash_message.statement is empty %}
-        <div class="alert alert-{{ flash_key }}" role="alert">
-          {{ flash_message.message }}
-        </div>
-      {% else %}
-        {{ statement_message(flash_message.message, flash_message.statement, flash_key) }}
-      {% endif %}
-    {% endfor %}
+  {% for flash_message in flash() %}
+    {% if flash_message.statement is empty %}
+      <div class="alert alert-{{ flash_message.context }}" role="alert">
+        {{ flash_message.message }}
+      </div>
+    {% else %}
+      {{ statement_message(flash_message.message, flash_message.statement, flash_message.context) }}
+    {% endif %}
   {% endfor %}
 
   {% block message %}{% endblock %}

diff --git a/resources/templates/table/page_with_secondary_tabs.twig b/resources/templates/table/page_with_secondary_tabs.twig
@@ -14,16 +14,14 @@
   </ul>
 {% endif %}
 
-{% for flash_key, flash_messages in flash() %}
-  {% for flash_message in flash_messages %}
-    {% if flash_message.statement is empty %}
-      <div class="alert alert-{{ flash_key }}" role="alert">
-        {{ flash_message.message }}
-      </div>
-    {% else %}
-      {{ statement_message(flash_message.message, flash_message.statement, flash_key) }}
-    {% endif %}
-  {% endfor %}
+{% for flash_message in flash() %}
+  {% if flash_message.statement is empty %}
+    <div class="alert alert-{{ flash_message.context }}" role="alert">
+      {{ flash_message.message }}
+    </div>
+  {% else %}
+    {{ statement_message(flash_message.message, flash_message.statement, flash_message.context) }}
+  {% endif %}
 {% endfor %}
 
 <div id="structure_content">

diff --git a/src/FlashMessenger.php b/src/FlashMessenger.php
@@ -15,7 +15,7 @@ final class FlashMessenger
     /** @var mixed[] */
     private array $storage;
 
-    /** @var array<string, string[][]> */
+    /** @psalm-var list<array{context: string, message: string, statement: string}> */
     private array $previousMessages = [];
 
     public function __construct()
@@ -33,24 +33,20 @@ public function __construct()
         $this->storage[self::STORAGE_KEY] = [];
     }
 
-    public function addMessage(string $key, string $message, string $statement = ''): void
+    public function addMessage(string $context, string $message, string $statement = ''): void
     {
-        if (! isset($this->storage[self::STORAGE_KEY][$key])) {
-            $this->storage[self::STORAGE_KEY][$key] = [];
-        }
-
-        $this->storage[self::STORAGE_KEY][$key][] = ['message' => $message, 'statement' => $statement];
+        $this->storage[self::STORAGE_KEY][] = ['context' => $context, 'message' => $message, 'statement' => $statement];
     }
 
-    /** @return string[][]|null */
-    public function getMessage(string $key): array|null
+    /** @psalm-return list<array{context: string, message: string, statement: string}> */
+    public function getMessages(): array
     {
-        return $this->previousMessages[$key] ?? null;
+        return $this->previousMessages;
     }
 
-    /** @return array<string, string[][]> */
-    public function getMessages(): array
+    /** @psalm-return list<array{context: string, message: string, statement: string}> */
+    public function getCurrentMessages(): array
     {
-        return $this->previousMessages;
+        return $this->storage[self::STORAGE_KEY];
     }
 }
diff --git a/tests/unit/Controllers/Table/DropColumnControllerTest.php b/tests/unit/Controllers/Table/DropColumnControllerTest.php
@@ -42,20 +42,17 @@ public function testDropColumnController(): void
         $dummyDbi->addResult('ALTER TABLE `test_table` DROP `name`, DROP `datetimefield`;', true);
         $dbi = $this->createDatabaseInterface($dummyDbi);
 
-        self::assertArrayNotHasKey('FlashMessenger', $_SESSION);
-
+        $flashMessenger = new FlashMessenger();
         (new DropColumnController(
             new ResponseRenderer(),
             $dbi,
-            new FlashMessenger(),
+            $flashMessenger,
             new RelationCleanup($dbi, new Relation($dbi)),
         ))(self::createStub(ServerRequest::class));
 
-        self::assertArrayHasKey('FlashMessenger', $_SESSION);
-        /** @psalm-suppress InvalidArrayOffset */
         self::assertSame(
-            ['success' => [['message' => '2 columns have been dropped successfully.', 'statement' => '']]],
-            $_SESSION['FlashMessenger'],
+            [['context' => 'success', 'message' => '2 columns have been dropped successfully.', 'statement' => '']],
+            $flashMessenger->getCurrentMessages(),
         );
     }
 }
diff --git a/tests/unit/Export/ExportTest.php b/tests/unit/Export/ExportTest.php
@@ -243,7 +243,7 @@ public function testGetPageLocationAndSaveMessageForServerExportWithError(): voi
         $location = $export->getPageLocationAndSaveMessage('server', Message::error('Error message!'));
         self::assertSame('index.php?route=/server/export&server=2&lang=en', $location);
         self::assertSame(
-            ['danger' => [['message' => 'Error message!', 'statement' => '']]],
+            [['context' => 'danger', 'message' => 'Error message!', 'statement' => '']],
             (new FlashMessenger())->getMessages(),
         );
     }
@@ -258,7 +258,7 @@ public function testGetPageLocationAndSaveMessageForServerExportWithSuccess(): v
         $location = $export->getPageLocationAndSaveMessage('server', Message::success('Success message!'));
         self::assertSame('index.php?route=/server/export&server=2&lang=en', $location);
         self::assertSame(
-            ['success' => [['message' => 'Success message!', 'statement' => '']]],
+            [['context' => 'success', 'message' => 'Success message!', 'statement' => '']],
             (new FlashMessenger())->getMessages(),
         );
     }
@@ -274,7 +274,7 @@ public function testGetPageLocationAndSaveMessageForDatabaseExportWithError(): v
         $location = $export->getPageLocationAndSaveMessage('database', Message::error('Error message!'));
         self::assertSame('index.php?route=/database/export&db=test_db&server=2&lang=en', $location);
         self::assertSame(
-            ['danger' => [['message' => 'Error message!', 'statement' => '']]],
+            [['context' => 'danger', 'message' => 'Error message!', 'statement' => '']],
             (new FlashMessenger())->getMessages(),
         );
     }
@@ -290,7 +290,7 @@ public function testGetPageLocationAndSaveMessageForDatabaseExportWithSuccess():
         $location = $export->getPageLocationAndSaveMessage('database', Message::success('Success message!'));
         self::assertSame('index.php?route=/database/export&db=test_db&server=2&lang=en', $location);
         self::assertSame(
-            ['success' => [['message' => 'Success message!', 'statement' => '']]],
+            [['context' => 'success', 'message' => 'Success message!', 'statement' => '']],
             (new FlashMessenger())->getMessages(),
         );
     }
@@ -310,7 +310,7 @@ public function testGetPageLocationAndSaveMessageForTableExportWithError(): void
             $location,
         );
         self::assertSame(
-            ['danger' => [['message' => 'Error message!', 'statement' => '']]],
+            [['context' => 'danger', 'message' => 'Error message!', 'statement' => '']],
             (new FlashMessenger())->getMessages(),
         );
     }
@@ -330,7 +330,7 @@ public function testGetPageLocationAndSaveMessageForTableExportWithSuccess(): vo
             $location,
         );
         self::assertSame(
-            ['success' => [['message' => 'Success message!', 'statement' => '']]],
+            [['context' => 'success', 'message' => 'Success message!', 'statement' => '']],
             (new FlashMessenger())->getMessages(),
         );
     }

diff --git a/tests/unit/FlashMessengerTest.php b/tests/unit/FlashMessengerTest.php
@@ -32,57 +32,61 @@ public function testConstructorSessionNotFound(): void
     public function testAddMessage(): void
     {
         $flashMessenger = new FlashMessenger();
-        self::assertArrayNotHasKey('error', $_SESSION[self::STORAGE_KEY]);
+        self::assertSame([], $_SESSION[self::STORAGE_KEY]);
         $flashMessenger->addMessage('error', 'Error');
-        self::assertArrayHasKey('error', $_SESSION[self::STORAGE_KEY]);
-        self::assertIsArray($_SESSION[self::STORAGE_KEY]['error']);
-        self::assertSame([['message' => 'Error', 'statement' => '']], $_SESSION[self::STORAGE_KEY]['error']);
+        /** @psalm-suppress DocblockTypeContradiction addMessage() mutates $_SESSION[self::STORAGE_KEY] */
+        self::assertSame(
+            [['context' => 'error', 'message' => 'Error', 'statement' => '']],
+            $_SESSION[self::STORAGE_KEY],
+        );
     }
 
     public function testAddMessageWithStatement(): void
     {
         $flashMessenger = new FlashMessenger();
-        self::assertArrayNotHasKey('success', $_SESSION[self::STORAGE_KEY]);
-        $flashMessenger->addMessage('success', 'Your SQL query has been executed successfully.', 'SELECT 1;');
-        self::assertArrayHasKey('success', $_SESSION[self::STORAGE_KEY]);
-        self::assertIsArray($_SESSION[self::STORAGE_KEY]['success']);
+        self::assertSame([], $_SESSION[self::STORAGE_KEY]);
+        $flashMessenger->addMessage('success', 'Success!', 'SELECT 1;');
+        /** @psalm-suppress DocblockTypeContradiction addMessage() mutates $_SESSION[self::STORAGE_KEY] */
         self::assertSame(
-            [['message' => 'Your SQL query has been executed successfully.', 'statement' => 'SELECT 1;']],
-            $_SESSION[self::STORAGE_KEY]['success'],
+            [['context' => 'success', 'message' => 'Success!', 'statement' => 'SELECT 1;']],
+            $_SESSION[self::STORAGE_KEY],
         );
     }
 
-    public function testGetMessage(): void
-    {
-        $_SESSION[self::STORAGE_KEY] = ['warning' => [['message' => 'Warning', 'statement' => '']]];
-        $flashMessenger = new FlashMessenger();
-        $message = $flashMessenger->getMessage('error');
-        self::assertNull($message);
-        $message = $flashMessenger->getMessage('warning');
-        self::assertSame([['message' => 'Warning', 'statement' => '']], $message);
-    }
-
     public function testGetMessages(): void
     {
-        $_SESSION[self::STORAGE_KEY] = [
-            'error' => [
-                ['message' => 'Error1', 'statement' => ''],
-                ['message' => 'Error2', 'statement' => ''],
+        $_SESSION[self::STORAGE_KEY] = [];
+        $flashMessengerOne = new FlashMessenger();
+        self::assertSame([], $flashMessengerOne->getCurrentMessages());
+        $flashMessengerOne->addMessage('error', 'Error1');
+        $flashMessengerOne->addMessage('error', 'Error2', 'SOME SQL;');
+        $flashMessengerOne->addMessage('warning', 'Warning');
+        self::assertSame(
+            [
+                ['context' => 'error', 'message' => 'Error1', 'statement' => ''],
+                ['context' => 'error', 'message' => 'Error2', 'statement' => 'SOME SQL;'],
+                ['context' => 'warning', 'message' => 'Warning', 'statement' => ''],
             ],
-            'warning' => [['message' => 'Warning', 'statement' => '']],
-        ];
-        $flashMessenger = new FlashMessenger();
-        $flashMessenger->addMessage('notice', 'Notice');
-        $messages = $flashMessenger->getMessages();
+            $flashMessengerOne->getCurrentMessages(),
+        );
+        self::assertSame([], $flashMessengerOne->getMessages());
+        $flashMessengerTwo = new FlashMessenger();
+        $flashMessengerTwo->addMessage('notice', 'Notice');
+        self::assertSame(
+            [['context' => 'notice', 'message' => 'Notice', 'statement' => '']],
+            $flashMessengerTwo->getCurrentMessages(),
+        );
         self::assertSame(
             [
-                'error' => [
-                    ['message' => 'Error1', 'statement' => ''],
-                    ['message' => 'Error2', 'statement' => ''],
-                ],
-                'warning' => [['message' => 'Warning', 'statement' => '']],
+                ['context' => 'error', 'message' => 'Error1', 'statement' => ''],
+                ['context' => 'error', 'message' => 'Error2', 'statement' => 'SOME SQL;'],
+                ['context' => 'warning', 'message' => 'Warning', 'statement' => ''],
             ],
-            $messages,
+            $flashMessengerTwo->getMessages(),
+        );
+        self::assertSame(
+            [['context' => 'notice', 'message' => 'Notice', 'statement' => '']],
+            (new FlashMessenger())->getMessages(),
         );
     }
 }