search

getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

[3.4.0-rc.2] Error on Panel builder file select #2689

Closed modufolio closed 4 years ago

modufolio commented 4 years ago

Describe the bug
I'm getting an error when i want to select an image in structured field with the builder plugin.

Error: Access to non-existing property pages/modules on array

To Reproduce
site/blueprints/pages/modules.yml

title: Modules

fields:
  modules:
    label: Modules
    type: builder
    columns: 1
    fieldsets:
      cards:
        label: Cards
        type: fields
        tabs:
          content:
            label: Content
            icon: edit
            fields:
              cardimage:
                label: Card Image
                type: files
                multiple: false

Console output 

app.js:1 GET http://kirby3-dev.test/api/kirby-builder/pages/trouwalbums/fields/modules+cards+cards+cardimage?page=0 400 (Bad Request)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 {status: "error", message: "Access to non-existing property pages/trouwalbums on array", code: 400, exception: "Kirby\Exception\BadMethodCallException", key: "error.invalidMethod", …}
rv.config.onError @ app.js:1
(anonymous) @ app.js:1
Promise.catch (async)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 POST http://kirby3-dev.test/api/site/children 400 (Bad Request)
request @ app.js:1
post @ app.js:1
submit @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
oe @ vendor.js:23
Tn.t.$emit @ vendor.js:23
submit @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 {status: "error", message: "A page draft with the URL appendix "modules" already exists", code: 400, exception: "Kirby\Exception\DuplicateException", key: "error.page.draft.duplicate", …}
rv.config.onError @ app.js:1
(anonymous) @ app.js:1
Promise.catch (async)
request @ app.js:1
post @ app.js:1
submit @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
oe @ vendor.js:23
Tn.t.$emit @ vendor.js:23
submit @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 GET http://kirby3-dev.test/api/kirby-builder/pages/modules/fields/modules+cards+cards+cardimage?page=0 400 (Bad Request)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 {status: "error", message: "Access to non-existing property pages/modules on array", code: 400, exception: "Kirby\Exception\BadMethodCallException", key: "error.invalidMethod", …}
rv.config.onError @ app.js:1
(anonymous) @ app.js:1
Promise.catch (async)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 GET http://kirby3-dev.test/api/kirby-builder/pages/modules/fields/modules+cards+cards+cardimage?page=0 400 (Bad Request)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 {status: "error", message: "Access to non-existing property pages/modules on array", code: 400, exception: "Kirby\Exception\BadMethodCallException", key: "error.invalidMethod", …}
rv.config.onError @ app.js:1
(anonymous) @ app.js:1
Promise.catch (async)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
/panel/site#settings:1 Autofocus processing was blocked because a document already has a focused element.
app.js:1 GET http://kirby3-dev.test/api/kirby-builder/pages/modules/fields/modules+cards+cards+cardimage?page=0 400 (Bad Request)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 {status: "error", message: "Access to non-existing property pages/modules on array", code: 400, exception: "Kirby\Exception\BadMethodCallException", key: "error.invalidMethod", …}
rv.config.onError @ app.js:1
(anonymous) @ app.js:1
Promise.catch (async)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 GET http://kirby3-dev.test/api/kirby-builder/pages/modules/fields/modules+cards+cards+cardimage?page=0 400 (Bad Request)
request @ app.js:1
get @ app.js:1
fetch @ app.js:1
open @ app.js:1
open @ app.js:1
oe @ vendor.js:23
n @ vendor.js:23
Ni.i._wrapper @ vendor.js:23
app.js:1 {status: "error", message: "Access to non-existing property pages/modules on array", code: 400, exception: "Kirby\Exception\BadMethodCallException", key: "error.invalidMethod", …}
lukasbestle commented 4 years ago

I was able to reproduce the issue with your blueprint. It only happens when the files field is nested inside the builder field, not with files fields that are directly used in the blueprint. I also couldn't reproduce the issue inside a structure field outside of a builder instance.

Here's the backtrace for @modufolio's blueprint (which I have reduced above to be as minimal as possible to still reproduce the issue):

#0  Kirby\Toolkit\Query::accessError() called at [/kirby/src/Toolkit/Query.php:107]
#1  Kirby\Toolkit\Query->resolve() called at [/kirby/src/Toolkit/Query.php:69]
#2  Kirby\Toolkit\Query->result() called at [/kirby/src/Toolkit/Str.php:531]
#3  Kirby\Toolkit\Str::query() called at [/kirby/src/Cms/ModelWithContent.php:456]
#4  Kirby\Cms\ModelWithContent->query() called at [/kirby/config/fields/files.php:50]
#5  Kirby\Form\Field->{closure}()
#6  Closure->call() called at [/kirby/src/Toolkit/Component.php:212]
#7  Kirby\Toolkit\Component->applyComputed() called at [/kirby/src/Toolkit/Component.php:127]
#8  Kirby\Toolkit\Component->__construct() called at [/kirby/src/Form/Field.php:61]
#9  Kirby\Form\Field->__construct() called at [/site/plugins/kirby-builder/index.php:57]
#10 fieldFromPath() called at [/site/plugins/kirby-builder/index.php:52]
#11 fieldFromPath() called at [/site/plugins/kirby-builder/index.php:20]
#12 callFieldAPI() called at [/site/plugins/kirby-builder/index.php:351]
#13 Kirby\Cms\Api->{closure}()
#14 Closure->call() called at [/kirby/src/Api/Api.php:207]
#15 Kirby\Api\Api->call() called at [/kirby/src/Cms/Api.php:47]
#16 Kirby\Cms\Api->call() called at [/kirby/src/Api/Api.php:587]
#17 Kirby\Api\Api->render() called at [/kirby/config/routes.php:45]
#18 Kirby\Http\Route->{closure}()
#19 Closure->call() called at [/kirby/src/Http/Router.php:107]
#20 Kirby\Http\Router->call() called at [/kirby/src/Cms/App.php:323]
#21 Kirby\Cms\App->call() called at [/kirby/src/Cms/App.php:1004]
#22 Kirby\Cms\App->render() called at [/index.php:5]

Step #4 is where it gets interesting: The ModelWithContent::query() method gets called with the query pages/blueprint, which is the name of the blueprint. Of course the blueprint name is not a valid object that can be queried, so the query fails.

We didn't change anything about form handling, so the same issue also occurred before 3.4. But 3.4 added the explicit Exception when a non-existing object is queried, so this issue only now became visible.

The best way to fix this issue would be to track down why a query for pages/modules is attempted. Pinging @timoetting as the developer of the builder field.

However we could also fix this by silencing the Exception. I'm not a huge fan of this though to be honest as we would lose the advantage of the Exception – it immediately points out that there's an issue somewhere.

If we do decide that silencing the Exception is the best way to go, these would be the required changes that "fix" the original issue:

diff --git a/src/Cms/ModelWithContent.php b/src/Cms/ModelWithContent.php
index 4d018a72..99c76e9f 100644
--- a/src/Cms/ModelWithContent.php
+++ b/src/Cms/ModelWithContent.php
@@ -4,6 +4,7 @@ namespace Kirby\Cms;

 use Closure;
 use Kirby\Data\Data;
+use Kirby\Exception\Exception;
 use Kirby\Exception\InvalidArgumentException;
 use Kirby\Toolkit\Str;
 use Throwable;
@@ -448,11 +449,15 @@ abstract class ModelWithContent extends Model
             return null;
         }

-        $result = Str::query($query, [
-            'kirby'             => $this->kirby(),
-            'site'              => is_a($this, 'Kirby\Cms\Site') ? $this : $this->site(),
-            static::CLASS_ALIAS => $this
-        ]);
+        try {
+            $result = Str::query($query, [
+                'kirby'             => $this->kirby(),
+                'site'              => is_a($this, 'Kirby\Cms\Site') ? $this : $this->site(),
+                static::CLASS_ALIAS => $this
+            ]);
+        } catch (Exception $e) {
+            return null;
+        }

         if ($expect !== null && is_a($result, $expect) !== true) {
             return null;
bastianallgeier commented 4 years ago

Ughh, this is a hard one. In order to keep the upgrade less painful I'd say we silence the exception, even though I agree that the exception is a much better way to discover and debug such issues.

bastianallgeier commented 4 years ago

✅