Change Console\UpdateConfigController to return a Response object

Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
phpmyadmin · Mar 29, 2024 · 774ec82 · 774ec82
1 parent 2c32a6c
commit 774ec82
Show file tree

Hide file tree

Showing 6 changed files with 74 additions and 32 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -14488,6 +14488,7 @@
       <code><![CDATA[Config::getInstance()]]></code>
     </DeprecatedMethod>
     <MixedAssignment>
+      <code><![CDATA[$json['error']]]></code>
       <code><![CDATA[$value]]></code>
     </MixedAssignment>
     <PossiblyUnusedMethod>

diff --git a/resources/js/src/modules/ajax.ts b/resources/js/src/modules/ajax.ts
@@ -939,7 +939,7 @@ const AJAX = {
                 console.log('AJAX error: status=' + request.status + ', text=' + request.statusText);
             }
 
-            if (settings.url.includes('/git-revision')) {
+            if (settings.url.includes('/git-revision') || settings.url.includes('/console/update-config')) {
                 return;
             }
 

diff --git a/resources/js/src/modules/console/config.ts b/resources/js/src/modules/console/config.ts
@@ -1,6 +1,7 @@
 import $ from 'jquery';
 import { ajaxShowMessage } from '../ajax-message.ts';
 import { CommonParams } from '../common.ts';
+import { escapeHtml } from '../functions/escape.ts';
 
 /**
  * @link https://docs.phpmyadmin.net/en/latest/config.html#console-settings
@@ -123,23 +124,16 @@ export default class Config {
  * @param {boolean|string|number} value
  */
 function setConfigValue (key: string, value: boolean|number|string): void {
-    $.ajax({
-        url: 'index.php?route=/console/update-config',
-        type: 'POST',
-        dataType: 'json',
-        data: {
+    $.post(
+        'index.php?route=/console/update-config',
+        {
             'ajax_request': true,
             server: CommonParams.get('server'),
             key: key,
             value: value,
         },
-        success: function (data) {
-            if (data.success !== true) {
-                // Try to find a message to display
-                if (data.error || data.message) {
-                    ajaxShowMessage(data.error || data.message);
-                }
-            }
-        }
+    ).fail(function (data) {
+        const message = '<div class="alert alert-danger" role="alert">' + escapeHtml(data.responseJSON.error) + '</div>';
+        ajaxShowMessage(message, false);
     });
 }
diff --git a/src/Controllers/Console/UpdateConfigController.php b/src/Controllers/Console/UpdateConfigController.php
@@ -4,11 +4,12 @@
 
 namespace PhpMyAdmin\Controllers\Console;
 
+use Fig\Http\Message\StatusCodeInterface;
 use InvalidArgumentException;
 use PhpMyAdmin\Config;
 use PhpMyAdmin\Controllers\AbstractController;
+use PhpMyAdmin\Http\Response;
 use PhpMyAdmin\Http\ServerRequest;
-use PhpMyAdmin\Message;
 use PhpMyAdmin\ResponseRenderer;
 use PhpMyAdmin\Template;
 
@@ -23,25 +24,31 @@ public function __construct(ResponseRenderer $response, Template $template, priv
         parent::__construct($response, $template);
     }
 
-    public function __invoke(ServerRequest $request): void
+    public function __invoke(ServerRequest $request): Response
     {
         try {
             $key = $this->parseKeyParam($request->getParsedBodyParam('key'));
             $value = $this->parseValueParam($key, $request->getParsedBodyParam('value'));
         } catch (InvalidArgumentException $exception) {
+            $this->response->setStatusCode(StatusCodeInterface::STATUS_BAD_REQUEST);
             $this->response->setRequestStatus(false);
-            $this->response->addJSON(['message' => Message::error($exception->getMessage())]);
+            $this->response->addJSON(['message' => $exception->getMessage()]);
 
-            return;
+            return $this->response->response();
         }
 
         $result = $this->config->setUserValue(null, 'Console/' . $key, $value);
-        if ($result === true) {
-            return;
+        if ($result !== true) {
+            $this->response->setStatusCode(StatusCodeInterface::STATUS_INTERNAL_SERVER_ERROR);
+            $this->response->setRequestStatus(false);
+            $this->response->addJSON(['message' => $result->getMessage()]);
+
+            return $this->response->response();
         }
 
-        $this->response->setRequestStatus(false);
-        $this->response->addJSON(['message' => $result]);
+        $this->response->addJSON('message', __('Console settings has been updated successfully.'));
+
+        return $this->response->response();
     }
 
     /** @psalm-return 'StartHistory'|'AlwaysExpand'|'CurrentQuery'|'EnterExecutes'|'DarkTheme'|'Mode'|'Height'|'GroupQueries'|'OrderBy'|'Order' */

diff --git a/tests/unit/Controllers/Console/UpdateConfigControllerTest.php b/tests/unit/Controllers/Console/UpdateConfigControllerTest.php
@@ -6,6 +6,7 @@
 
 use PhpMyAdmin\Config;
 use PhpMyAdmin\Controllers\Console\UpdateConfigController;
+use PhpMyAdmin\DatabaseInterface;
 use PhpMyAdmin\Http\Factory\ServerRequestFactory;
 use PhpMyAdmin\Message;
 use PhpMyAdmin\Template;
@@ -14,6 +15,8 @@
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
 
+use function json_decode;
+
 #[CoversClass(UpdateConfigController::class)]
 final class UpdateConfigControllerTest extends AbstractTestCase
 {
@@ -23,14 +26,20 @@ public function testValidParams(string $key, string $value, bool|int|string $exp
         $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/')
             ->withParsedBody(['key' => $key, 'value' => $value]);
 
+        DatabaseInterface::$instance = $this->createDatabaseInterface();
         $config = new Config();
         $responseRenderer = new ResponseRenderer();
+        $responseRenderer->setAjax(true);
         $controller = new UpdateConfigController($responseRenderer, new Template($config), $config);
-        $controller($request);
+        $response = $controller($request);
 
+        $responseBody = (string) $response->getBody();
+        self::assertJson($responseBody);
+        self::assertSame(
+            ['message' => 'Console settings has been updated successfully.', 'success' => true],
+            json_decode($responseBody, true),
+        );
         self::assertSame($expected, $config->settings['Console'][$key]);
-        self::assertSame([], $responseRenderer->getJSONResult());
-        self::assertTrue($responseRenderer->hasSuccessState(), 'Should be a successful response.');
     }
 
     /** @return iterable<array{string, string, bool|int|string}> */
@@ -70,16 +79,19 @@ public function testInvalidParams(array|string $key, array|string $value): void
         $request = ServerRequestFactory::create()->createServerRequest('POST', 'http://example.com/')
             ->withParsedBody(['key' => $key, 'value' => $value]);
 
+        DatabaseInterface::$instance = $this->createDatabaseInterface();
         $config = new Config();
         $responseRenderer = new ResponseRenderer();
+        $responseRenderer->setAjax(true);
         $controller = new UpdateConfigController($responseRenderer, new Template($config), $config);
-        $controller($request);
+        $response = $controller($request);
 
+        $responseBody = (string) $response->getBody();
+        self::assertJson($responseBody);
         self::assertSame(
-            ['message' => Message::error('Unexpected parameter value.')->getDisplay()],
-            $responseRenderer->getJSONResult(),
+            ['success' => false, 'error' => 'Unexpected parameter value.'],
+            json_decode($responseBody, true),
         );
-        self::assertFalse($responseRenderer->hasSuccessState(), 'Should be a failed response.');
     }
 
     /** @return iterable<array{string|string[], string|string[]}> */
@@ -130,13 +142,18 @@ public function testFailedConfigSaving(): void
         $config = self::createStub(Config::class);
         $config->method('setUserValue')->willReturn(Message::error('Could not save configuration'));
         $responseRenderer = new ResponseRenderer();
+        $responseRenderer->setAjax(true);
         $controller = new UpdateConfigController($responseRenderer, new Template($config), $config);
-        $controller($request);
+        $response = $controller($request);
 
+        $responseBody = (string) $response->getBody();
+        self::assertJson($responseBody);
         self::assertSame(
-            ['message' => Message::error('Could not save configuration')->getDisplay()],
-            $responseRenderer->getJSONResult(),
+            ['success' => false, 'error' => 'Could not save configuration'],
+            json_decode($responseBody, true),
         );
+
+        self::assertSame(['message' => 'Could not save configuration'], $responseRenderer->getJSONResult());
         self::assertFalse($responseRenderer->hasSuccessState(), 'Should be a failed response.');
     }
 }
diff --git a/tests/unit/Stubs/ResponseRenderer.php b/tests/unit/Stubs/ResponseRenderer.php
@@ -25,6 +25,7 @@
 use PhpMyAdmin\Template;
 
 use function is_array;
+use function json_encode;
 
 class ResponseRenderer extends \PhpMyAdmin\ResponseRenderer
 {
@@ -186,4 +187,26 @@ public function getResponse(): Response
     {
         return $this->response;
     }
+
+    public function response(): Response
+    {
+        if ($this->isAjax()) {
+            $json = $this->getJSONResult();
+            if ($this->isSuccess) {
+                $json['success'] = true;
+            } else {
+                $json['success'] = false;
+                $json['error'] = $json['message'];
+                unset($json['message']);
+            }
+
+            $output = (string) json_encode($json);
+        } else {
+            $output = $this->getHTMLResult();
+        }
+
+        $this->response->getBody()->write($output);
+
+        return $this->response;
+    }
 }