Fix App::jsUrl() to always append "__atk_json=1" (#2248)

atk4 · Feb 1, 2025 · e008029 · e008029
1 parent 48213c1
commit e008029
Show file tree

Hide file tree

Showing 11 changed files with 39 additions and 32 deletions.
diff --git a/js/src/Service/accordionService.js b/js/src/Service/accordionService.js
@@ -12,7 +12,7 @@ class AccordionService {
 
     onOpening() {
         if ($(this).data('url')) {
-            $(this).atkReloadView({ url: $(this).data('url'), urlOptions: { __atk_json: 1 } });
+            $(this).atkReloadView({ url: $(this).data('url') });
         }
     }
 }

diff --git a/js/src/Service/modalService.js b/js/src/Service/modalService.js
@@ -86,11 +86,6 @@ class ModalService {
             args = data.args;
         }
 
-        // check for data type, usually JSON or HTML
-        if (data.type === 'json') {
-            args = $.extend(true, args, { __atk_json: 1 });
-        }
-
         // does modal content need to be loaded dynamically
         if (data.url) {
             $modal.data('closeOnLoadingError', true);

diff --git a/public/js/atkjs-ui.js b/public/js/atkjs-ui.js
@@ -1987,10 +1987,7 @@ class AccordionService {
   onOpening() {
     if (external_jquery__WEBPACK_IMPORTED_MODULE_0___default()(this).data('url')) {
       external_jquery__WEBPACK_IMPORTED_MODULE_0___default()(this).atkReloadView({
-        url: external_jquery__WEBPACK_IMPORTED_MODULE_0___default()(this).data('url'),
-        urlOptions: {
-          __atk_json: 1
-        }
+        url: external_jquery__WEBPACK_IMPORTED_MODULE_0___default()(this).data('url')
       });
     }
   }
@@ -2689,13 +2686,6 @@ class ModalService {
       args = data.args;
     }
 
-    // check for data type, usually JSON or HTML
-    if (data.type === 'json') {
-      args = external_jquery__WEBPACK_IMPORTED_MODULE_5___default().extend(true, args, {
-        __atk_json: 1
-      });
-    }
-
     // does modal content need to be loaded dynamically
     if (data.url) {
       $modal.data('closeOnLoadingError', true);

diff --git a/public/js/atkjs-ui.js.map b/public/js/atkjs-ui.js.map
diff --git a/public/js/atkjs-ui.min.js b/public/js/atkjs-ui.min.js
diff --git a/public/js/atkjs-ui.min.js.map b/public/js/atkjs-ui.min.js.map
diff --git a/src/App.php b/src/App.php
@@ -778,6 +778,22 @@ public function stickyForget(string $name): void
         unset($this->stickyGetArguments[$name]);
     }
 
+    /**
+     * @return array<0|string, string>
+     */
+    private function explodeUrlPage(string $page): array
+    {
+        $pageExploded = explode('?', $page, 2);
+
+        $pagePath = $pageExploded[0] !== ''
+            ? $pageExploded[0]
+            : null;
+
+        parse_str($pageExploded[1] ?? '', $pageArgs);
+
+        return [$pagePath] + $pageArgs;
+    }
+
     /**
      * Build a URL that application can use for loading HTML data.
      *
@@ -787,14 +803,12 @@ public function stickyForget(string $name): void
     public function url($page = [], array $extraRequestUrlArgs = []): string
     {
         if (is_string($page)) {
-            $pageExploded = explode('?', $page, 2);
-            parse_str($pageExploded[1] ?? '', $page);
-            $pagePath = $pageExploded[0] !== '' ? $pageExploded[0] : null;
-        } else {
-            $pagePath = $page[0] ?? null;
-            unset($page[0]);
+            $page = $this->explodeUrlPage($page);
         }
 
+        $pagePath = $page[0] ?? null;
+        unset($page[0]);
+
         $request = $this->getRequest();
 
         if ($pagePath === null) {
@@ -842,8 +856,13 @@ public function url($page = [], array $extraRequestUrlArgs = []): string
      */
     public function jsUrl($page = [], array $extraRequestUrlArgs = []): string
     {
-        // append to the end but allow override
-        $extraRequestUrlArgs = array_merge($extraRequestUrlArgs, ['__atk_json' => 1], $extraRequestUrlArgs);
+        if (is_string($page)) {
+            $page = $this->explodeUrlPage($page);
+        }
+
+        if (($page['__atk_json'] ?? null) !== false) {
+            $page['__atk_json'] = 1;
+        }
 
         return $this->url($page, $extraRequestUrlArgs);
     }

diff --git a/src/JsSearch.php b/src/JsSearch.php
@@ -86,7 +86,9 @@ protected function renderView(): void
         $this->template->set('ButtonRemoveIcon', $this->buttonRemoveIcon);
 
         $this->js(true)->atkJsSearch([
-            'url' => $this->reload->jsUrl(),
+            'url' => $this->useAjax
+                ? $this->reload->jsUrl()
+                : $this->reload->url(),
             'urlOptions' => array_merge(['__atk_reload' => $this->reload->name], $this->args),
             'urlQueryKey' => $this->name . '_q',
             'autoQuery' => $this->autoQuery,

diff --git a/src/Tabs.php b/src/Tabs.php
@@ -29,7 +29,7 @@ public function addTab($name, ?\Closure $callback = null, array $settings = [])
         // if there is callback action, then use VirtualPage
         if ($callback) {
             $vp = VirtualPage::addTo($sub, ['ui' => '']);
-            $item->setUrl($vp->getJsUrl('cut'));
+            $item->setUrl($vp->getUrl('cut'));
 
             $vp->set($callback);
         }

diff --git a/src/VirtualPage.php b/src/VirtualPage.php
@@ -107,7 +107,7 @@ public function getHtml(): string
             }
 
             // render and terminate
-            if ($this->getApp()->hasRequestQueryParam('__atk_json')) {
+            if ($this->getApp()->isJsUrlRequest()) {
                 $this->getApp()->terminateJson($this);
             }
 

diff --git a/tests/AppTest.php b/tests/AppTest.php
@@ -170,6 +170,7 @@ public static function provideUrlCases(): iterable
         yield ['/?v=V', ['v' => false], ['x'], [], 'x.php'];
         yield ['/', ['v' => false], ['x', 'v' => 'page'], [], 'x.php?v=page'];
         yield ['/', ['v' => false], ['x'], ['v' => 'extra'], 'x.php'];
+        yield ['/', ['__atk_json' => false], ['x'], [], 'x.php'];
 
         // /wo page path
         yield ['/x', [], [], [], '/x.php'];
@@ -202,7 +203,7 @@ public function testUrl(string $requestUrl, array $appStickyGetArguments, array
         self::assertSame($expectedUrl, $app->url($page, $extraRequestUrlArgs));
         $pageAssocOnly = array_diff_key($page, [true]);
         self::assertSame($expectedUrl, $app->url(($page[0] ?? '') . (count($pageAssocOnly) > 0 ? '?' . implode('&', array_map(static fn ($k) => $k . '=' . $pageAssocOnly[$k], array_keys($pageAssocOnly))) : ''), $extraRequestUrlArgs));
-        self::assertSame($expectedUrl, $app->jsUrl($page, array_merge(['__atk_json' => null], $extraRequestUrlArgs)));
+        self::assertSame($expectedUrl . (str_contains($expectedUrl, '?') ? '&' : '?') . '__atk_json=1', $app->jsUrl($page, $extraRequestUrlArgs));
 
         $makeExpectedUrlFx = static function (string $indexPage, string $ext) use ($page, $expectedUrl) {
             return preg_replace_callback('~^[^?]*?\K([^/?]*)(\.php)(?=\?|$)~', static function ($matches) use ($page, $indexPage, $ext) {