Improve Callback URL generation (#1646)

demos/interactive/tabs.php

@@ -44,7 +44,7 @@
     $modelRegister->addField('name', ['caption' => 'Please enter your name (John)']);
 
     $form = \Atk4\Ui\Form::addTo($tab, ['segment' => true]);
-    $form->setModel($modelRegister);
+    $form->setModel($modelRegister->createEntity());
     $form->onSubmit(function (\Atk4\Ui\Form $form) {
         if ($form->model->get('name') !== 'John') {
             return $form->error('name', 'Your name is not John! It is "' . $form->model->get('name') . '". It should be John. Pleeease!');

demos/interactive/wizard.php

@@ -13,7 +13,7 @@
 /**
  * Demonstrates how to use a wizard.
  */
-$wizard = Wizard::addTo($app, ['stepCallback' => Callback::addTo($app, ['urlTrigger' => 'demo_wizard'])]);
+$wizard = Wizard::addTo($app, ['urlTrigger' => 'demo_wizard']);
 // First step will automatcally be active when you open page first. It
 // will contain the 'Next' button with a link.
 $wizard->addStep('Welcome', function (Wizard $wizard) {

phpstan.neon.dist

@@ -904,9 +904,6 @@ parameters:
         -
             path: 'src/View.php'
             message: '~^If condition is always true\.$~'
-        -
-            path: 'src/Wizard.php'
-            message: '~^Negated boolean expression is always false\.$~'
         -
             path: 'tests/DemosTest.php'
             message: '~^Else branch is unreachable because ternary operator condition is always true\.$~'
@@ -1201,12 +1198,6 @@ parameters:
         -
             path: 'src/AbstractView.php'
             message: '~^Method Atk4\\Ui\\AbstractView::add\(\) has parameter \$args with no typehint specified\.$~'
-        -
-            path: 'src/AbstractView.php'
-            message: '~^Instanceof between Atk4\\Ui\\View and Atk4\\Ui\\AbstractView will always evaluate to true\.$~'
-        -
-            path: 'src/AbstractView.php'
-            message: '~^Strict comparison using \!\=\= between Atk4\\Ui\\AbstractView and null will always evaluate to true\.$~'
         -
             path: 'src/Accordion.php'
             message: '~^Method Atk4\\Ui\\Accordion::activate\(\) has no return typehint specified\.$~'

src/AbstractView.php

@@ -52,7 +52,6 @@ abstract class AbstractView
      * @var bool
      */
     protected $_rendered = false;
-
     // }}}
 
     // {{{ Default init() method and add() logic
@@ -119,91 +118,4 @@ public function add($object, $args = null): self
 
         return $object;
     }
-
-    // }}}
-
-    // {{{ Sticky URLs
-
-    /** @var string[] stickyGet arguments */
-    public $stickyArgs = [];
-
-    /**
-     * Build an URL which this view can use for js call-backs. It should
-     * be guaranteed that requesting returned URL would at some point call
-     * $this->invokeInit().
-     *
-     * @param array $page
-     *
-     * @return string
-     */
-    public function jsUrl($page = [])
-    {
-        return $this->getApp()->jsUrl($page, false, $this->_getStickyArgs());
-    }
-
-    /**
-     * Build an URL which this view can use for call-backs. It should
-     * be guaranteed that requesting returned URL would at some point call
-     * $this->invokeInit().
-     *
-     * @param string|array $page URL as string or array with page name as first element and other GET arguments
-     *
-     * @return string
-     */
-    public function url($page = [])
-    {
-        return $this->getApp()->url($page, false, $this->_getStickyArgs());
-    }
-
-    /**
-     * Get sticky arguments defined by the view and parents (including API).
-     */
-    protected function _getStickyArgs(): array
-    {
-        if ($this->issetOwner() && $this->getOwner() instanceof self) {
-            $stickyArgs = array_merge($this->getOwner()->_getStickyArgs(), $this->stickyArgs);
-        } else {
-            $stickyArgs = $this->stickyArgs;
-        }
-
-        /** @var self $childView */
-        $childView = $this->mergeStickyArgsFromChildView();
-        if ($childView !== null && (!($childView instanceof Callback) || $childView->isTriggered())) {
-            $alreadyCalled = false;
-            foreach (debug_backtrace(\DEBUG_BACKTRACE_PROVIDE_OBJECT | \DEBUG_BACKTRACE_IGNORE_ARGS) as $frame) {
-                if ($childView === ($frame['object'] ?? null) && $frame['function'] === '_getStickyArgs') {
-                    $alreadyCalled = true;
-                }
-            }
-
-            if (!$alreadyCalled) {
-                $stickyArgs = array_merge($stickyArgs, $childView->_getStickyArgs());
-            }
-        }
-
-        return $stickyArgs;
-    }
-
-    protected function mergeStickyArgsFromChildView(): ?self
-    {
-        return null;
-    }
-
-    /**
-     * Mark GET argument as sticky. Calling url() on this view or any
-     * sub-views will embedd the value of this GET argument.
-     *
-     * If GET argument is empty or false, it won't make into URL.
-     *
-     * If GET argument is not presently set you can specify a 2nd argument
-     * to forge-set the GET argument for current view and it's sub-views.
-     */
-    public function stickyGet(string $name, string $newValue = null): ?string
-    {
-        $this->stickyArgs[$name] = $newValue ?? $_GET[$name] ?? null;
-
-        return $this->stickyArgs[$name];
-    }
-
-    // }}}
 }

src/App.php

@@ -124,6 +124,9 @@ class App
         'cache-control' => 'no-store', // disable caching by default
     ];
 
+    /** @var Modal[] Modal view that need to be rendered using json output. */
+    private $modals = [];
+
     /**
      * @var bool whether or not semantic-ui vue has been initialised
      */
@@ -222,6 +225,17 @@ static function (int $severity, string $msg, string $file, int $line): bool {
         $this->executorFactory = Factory::factory([ExecutorFactory::class]);
     }
 
+    /**
+     * Register a modal view.
+     * Fomantic-ui Modal are teleported in HTML template
+     * within specific location. This will keep track
+     * of modals when terminating app using json.
+     */
+    public function registerModal(Modal $modal): void
+    {
+        $this->modals[$modal->short_name] = $modal;
+    }
+
     public function setExecutorFactory(ExecutorFactory $factory)
     {
         $this->executorFactory = $factory;
@@ -553,8 +567,9 @@ public function run()
             $this->is_rendering = false;
             $this->hook(self::HOOK_BEFORE_OUTPUT);
 
-            if (isset($_GET['__atk_callback']) && $this->catch_runaway_callbacks) {
-                throw new Exception('Callback requested, but never reached. You may be missing some arguments in request URL.');
+            if (isset($_GET[Callback::URL_QUERY_TARGET]) && $this->catch_runaway_callbacks) {
+                throw (new Exception('Callback requested, but never reached. You may be missing some arguments in request URL.'))
+                    ->addMoreInfo('callback', $_GET[Callback::URL_QUERY_TARGET]);
             }
 
             $output = $this->html->template->renderToHtml();
@@ -1132,11 +1147,9 @@ public function getRenderedModals(): array
         unset($_GET['__atk_reload']);
 
         $modals = [];
-        foreach ($this->html !== null ? $this->html->elements : [] as $view) {
-            if ($view instanceof Modal) {
-                $modals[$view->name]['html'] = $view->getHtml();
-                $modals[$view->name]['js'] = $view->getJsRenderActions();
-            }
+        foreach ($this->modals as $view) {
+            $modals[$view->name]['html'] = $view->getHtml();
+            $modals[$view->name]['js'] = $view->getJsRenderActions();
         }
 
         return $modals;

src/Callback.php

@@ -9,7 +9,7 @@
  * executed directly will perform a PHP callback that you set().
  *
  * Callback function run when triggered, i.e. when it's urlTrigger param value is present in the $_GET request.
- * The current callback will be set within the $_GET['__atk_callback'] and will be set to urlTrigger as well.
+ * The current callback will be set within the $_GET[Callback::URL_QUERY_TARGET] and will be set to urlTrigger as well.
  *
  * $button = Button::addTo($layout);
  * $button->set('Click to do something')->link(
@@ -22,6 +22,12 @@
  */
 class Callback extends AbstractView
 {
+    /** @const string */
+    public const URL_QUERY_TRIGGER_PREFIX = '__atk_cb_';
+
+    /** @const string */
+    public const URL_QUERY_TARGET = '__atk_cbtarget';
+
     /** @var string Specify a custom GET trigger. */
     protected $urlTrigger;
 
@@ -43,6 +49,8 @@ protected function init(): void
     public function setUrlTrigger(string $trigger = null)
     {
         $this->urlTrigger = $trigger ?: $this->name;
+
+        $this->getOwner()->stickyGet(self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger);
     }
 
     public function getUrlTrigger(): string
@@ -61,13 +69,7 @@ public function getUrlTrigger(): string
     public function set($fx = null, $args = null)
     {
         if ($this->isTriggered() && $this->canTrigger()) {
-            $this->getApp()->catch_runaway_callbacks = false;
-            $t = $this->getApp()->run_called;
-            $this->getApp()->run_called = true;
-            $ret = $fx(...($args ?? []));
-            $this->getApp()->run_called = $t;
-
-            return $ret;
+            return $fx(...($args ?? []));
         }
     }
 
@@ -86,23 +88,23 @@ public function terminateJson(AbstractView $view): void
      */
     public function isTriggered()
     {
-        return isset($_GET[$this->urlTrigger]);
+        return isset($_GET[self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger]);
     }
 
     /**
      * Return callback triggered value.
      */
     public function getTriggeredValue(): string
     {
-        return $_GET[$this->urlTrigger] ?? '';
+        return $_GET[self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger] ?? '';
     }
 
     /**
      * Only current callback can terminate.
      */
     public function canTerminate(): bool
     {
-        return isset($_GET['__atk_callback']) && $_GET['__atk_callback'] === $this->urlTrigger;
+        return isset($_GET[self::URL_QUERY_TARGET]) && $_GET[self::URL_QUERY_TARGET] === $this->urlTrigger;
     }
 
     /**
@@ -115,12 +117,12 @@ public function canTrigger(): bool
 
     /**
      * Return URL that will trigger action on this call-back. If you intend to request
-     * the URL direcly in your browser (as iframe, new tab, or document location), you
+     * the URL directly in your browser (as iframe, new tab, or document location), you
      * should use getUrl instead.
      */
     public function getJsUrl(string $value = 'ajax'): string
     {
-        return $this->jsUrl($this->getUrlArguments($value));
+        return $this->getOwner()->jsUrl($this->getUrlArguments($value));
     }
 
     /**
@@ -129,22 +131,14 @@ public function getJsUrl(string $value = 'ajax'): string
      */
     public function getUrl(string $value = 'callback'): string
     {
-        return $this->url($this->getUrlArguments($value));
+        return $this->getOwner()->url($this->getUrlArguments($value));
     }
 
     /**
      * Return proper url argument for this callback.
      */
     private function getUrlArguments(string $value = null): array
     {
-        return ['__atk_callback' => $this->urlTrigger, $this->urlTrigger => $value ?? $this->getTriggeredValue()];
-    }
-
-    protected function _getStickyArgs(): array
-    {
-        // DEV NOTE:
-        // - getUrlArguments $value used only in https://github.com/atk4/ui/blob/08644a685a9ee07b4e94d1e35e3bd3c95b7a013d/src/VirtualPage.php#L134
-        // - $_GET['__atk_callback'] from getUrlArguments seems to control terminating behaviour!
-        return array_merge(parent::_getStickyArgs(), $this->getUrlArguments() /* TODO we do not want/need all Callback args probably*/);
+        return [self::URL_QUERY_TARGET => $this->urlTrigger, self::URL_QUERY_TRIGGER_PREFIX . $this->urlTrigger => $value ?? $this->getTriggeredValue()];
     }
 }

src/Loader.php

@@ -116,9 +116,4 @@ public function jsLoad($args = [], $apiConfig = [], $storeName = null)
             'storeName' => $storeName ? $storeName : null,
         ]);
     }
-
-    protected function mergeStickyArgsFromChildView(): AbstractView
-    {
-        return $this->cb;
-    }
 }

src/Modal.php

@@ -59,6 +59,12 @@ class Modal extends View
      */
     public $showActions = false;
 
+    protected function init(): void
+    {
+        parent::init();
+        $this->getApp()->registerModal($this);
+    }
+
     /**
      * Set callback function for this modal.
      * $fx is set as an array in order to comply with View::set().
@@ -330,12 +336,4 @@ protected function renderView(): void
 
         parent::renderView();
     }
-
-    /** @var AbstractView */
-    public $viewForUrl;
-
-    protected function mergeStickyArgsFromChildView(): ?AbstractView
-    {
-        return $this->viewForUrl ?? $this->cb;
-    }
 }

src/Panel/Content.php

@@ -57,9 +57,4 @@ public function getClearSelector(): array
     {
         return ['.atk-panel-content'];
     }
-
-    protected function mergeStickyArgsFromChildView(): ?\Atk4\Ui\AbstractView
-    {
-        return $this->cb;
-    }
 }

src/Panel/Right.php

@@ -202,9 +202,4 @@ protected function renderView(): void
 
         $this->js(true, $this->service()->addPanel($this->getPanelOptions()));
     }
-
-    protected function mergeStickyArgsFromChildView(): ?\Atk4\Ui\AbstractView
-    {
-        return $this->dynamicContent;
-    }
 }

src/Popup.php

@@ -295,9 +295,4 @@ protected function renderView(): void
 
         parent::renderView();
     }
-
-    protected function mergeStickyArgsFromChildView(): ?AbstractView
-    {
-        return $this->cb;
-    }
 }