gearsdigital / enhanced-toolbar-link-dialog

Extend Kirby's default link dialog with search functionality, allowing you to easily create links to existing or external pages at your fingertips.
MIT License
62 stars 8 forks source link

Plugin breaks Panel of kirby v.3.4.0-rc.2 #15

Closed cgundermann closed 4 years ago

cgundermann commented 4 years ago

Describe the bug
Unfortunately Your plugin breaks the panel of the recent kirby version.

To Reproduce

Case 1:

  1. If the panel is already installed, go to panel/users
  2. No user is shown
  3. Creating a new User also fails

Console output:

GET http://localhost/my-project/api/roles 500 (Internal Server Error)
message: "Call to a member function value() on string"

GET http://localhost/my-project/api/translations 500 (Internal Server Error)
message: Call to undefined method Kirby\Cms\Translation::title()
enhanced-toolbar-link001

Case 2:

  1. Create a new kirby project
  2. Go to my-project/panel
  3. The panel installation page stays blank

Console output:

GET http://localhost/my-project/api/translations 500 (Internal Server Error)
message: Call to undefined method Kirby\Cms\Translation::title()

*enhanced-toolbar-link-dialog Version
1.2.3

Kirby Version
3.4.0-rc.2

Additional context
Many thanks in advance for having a look into it! 🙌🏼 😊

gearsdigital commented 4 years ago

Hey @CrisGraphics, thank you so much for bringing is up 🤯

I can confirm this issue and need to have a deeper look. The changelog states a bunch of kirby refactorings so it probably relates to that. Not sure what is going on...

cgundermann commented 4 years ago

Wish I could help You, but I (still) have absolutely no Vue skills, even though the issue seems to be php related...

gearsdigital commented 4 years ago

I appreciate that but no worries! I already found the root cause but I’m still not sure why it breaks something totally unrelated 😑

I’m going to fix some of the other issues and provide a release soon.

gearsdigital commented 4 years ago

@CrisGraphics This issue has now been fixed.

Sadly I'm not able to explain why this occurred at all. I'll try to shed some light by opening a bug ticket at kirby core. It seems wired to me, that something totally unrelated broke.

I'll keep you updated here...

cgundermann commented 4 years ago

Hey, many thanks for looking into it, I really appreciate it. Sadly it still doesn't work with kirby v3.4.0-rc2 (tried both installations, via composer and manually).

This is, what I get, when I want to install the panel:

starterkit-panel-install

And this, when a user already exists:

staterkit-panel-login

When I remove Your plugin from the plugins folder, everything works fine. Not sure, what I'm missing here 🤔

gearsdigital commented 4 years ago

@CrisGraphics Dang! Sorry for that... 🙊 Unfortunately I'm no longer able to reproduce this. Maybe it's a dumb question but have you tried to clear the browser cache?

It would be awesome if you could provide an example.

Bildschirmvideo aufnehmen 2020-07-06 um 11 08 25

cgundermann commented 4 years ago

Hm, that's weird,... Here's how I installed it:

Terminal:

Installed the starterkit (as described here): composer create-project getkirby/starterkit starterkit

then updated the require section of the composer.json to this:

"require": {
        "php": ">=7.1.0",
        "getkirby/cms": "dev-release/3.4.0",
        "gearsdigital/enhanced-toolbar-link-dialog": "^1.3"
    },

and then just hit composer update

  • Updating laminas/laminas-escaper (2.6.0 => 2.6.1): Downloading (100%)
    • Updating filp/whoops (2.3.1 => 2.7.2): Downloading (100%)
    • Updating phpmailer/phpmailer (v6.0.7 => v6.1.6): Downloading (100%)
    • Updating claviska/simpleimage (3.3.3 => 3.3.4): Downloading (100%)
    • Updating mustangostang/spyc (0.6.2 => 0.6.3): Downloading (100%)
    • Removing getkirby/cms (3.3.6)
    • Installing getkirby/cms (dev-release/3.4.0 9fa1114): Cloning 9fa111493c from cache
    • Installing gearsdigital/enhanced-toolbar-link-dialog (v1.3.1): Downloading (100%)

Cleared the browser cache and also tried it in another browser.

But,... no worries... I'll stick to kirby 3.3.6 for this project and give it another try, as soon as the stable version has been released :) A trillion thanks again!

janherman commented 4 years ago

I experience kind of similar issue after updating Kirby to 3.4.0 and Enhanced Toolbar Link Dialog to 1.3.1. When I open the panel, I get the error: Undefined index: type

Edit: Version 1.2.3 of the plugin seems to work fine.

gearsdigital commented 4 years ago

Thank you very much @janherman for reporting this issue. I’ll have a look an try to investigate what is going on…

gearsdigital commented 4 years ago

@janherman @CrisGraphics I reseted my kirby development environenment completely, installed a fresh starterkit and a the latest version of Enhanced Toolbar Link Dialog (which is namly 1.3.1) via composer and for gods sake I've no issues :(

Could you please try to manually remove Enhanced Toolbar Link Dialog from your plugins directory and then have it reinstalled?

Otherwise I would ask you to provide some details about your setup? Maybe it's a conflict somewhere else.

  1. What kind of other plugins have you installed?
  2. In which browser have you tested?

Thanks in advance.

janherman commented 4 years ago

I think the issue is not related to other plugins. At least in my case. I tried fresh Kirby Starterkit, updated Kirby to 3.4.0 and installed the latest version of Enhanced Toolbar Link Dialog (1.3.1). Still get the same error.

Image 1

I tested in latest Firefox and Chrome and I use Xampp with PHP 7.4.1 for development.

gearsdigital commented 4 years ago

Thanks @janherman! Maybe this relates to PHP itself. You're using7.4.1 while I'm on 7.2.2. Would you mind to provide the php log?

@CrisGraphics Which PHP Version are you running?

gearsdigital commented 4 years ago

@janherman @CrisGraphics Could you please try to remove 'type'=> null, from site/plugins/enhanced-toolbar-link-dialog/index.php:11 manually and tell me if that solves the issue?

janherman commented 4 years ago

Thanks for your suggestion, @gearsdigital. I removed the line but it did not solve the issue. I even do not get to the login screen, the error message appears as soon as I try to access panel/login page (I was able to login before, the error message showed right after I logged in).

By PHP log you mean something like phpinfo() output or anything else?

cgundermann commented 4 years ago

@gearsdigital Sorry for the late reply, as @janherman mentioned, the error persists by removing the line. If I give the type some value, I can at least fill in the login form but after hitting login, the same error shows up again: undefined index type. I'm also running php 7.4

gearsdigital commented 4 years ago

I mean... WTF?

@janherman Just the php-error.log. You can lookup the location using <?php phpinfo(); ?>. @CrisGraphics No worries. I'm glad you both are helping me out here :)

cgundermann commented 4 years ago

Unfotunately there's still no docs for custom models... Will try and look through Your code, maybe I'll find sth... 🙌🏼

janherman commented 4 years ago

@gearsdigital Ok, I found it and there are no errors at all. :shrug:

gearsdigital commented 4 years ago

Thanks for your effort!

cgundermann commented 4 years ago

@janherman, @gearsdigital So, I don't know why, but I think I got it 😊 You also have to declare a type for the simplepagecollection:

'collections' => [
            'simplepagecollection' => [
              'type'=> null,
              'model' => 'simplepagemodel',
            ],
        ],

Edit : This works for me (kirby 3.4.0, enhanced-toolbar-link-dialog 1.3.1, php7.4) Just don't know how to make pull requests... I'm not very familiar with GitHub yet.

gearsdigital commented 4 years ago

On step by the time. I'm now able tp reproduce this issue by adding error_reporting(E_ALL); to the top of the index.php. Now I only need to understand how custom models are really working.

gearsdigital commented 4 years ago

@CrisGraphics I tried that biut it didn't work. I got it working by additionally adding 'type'=> null to the models section.

Kirby::plugin('gearsdigital/enhanced-toolbar-link-dialog', [
    'api'          => [
        'models'      => [
            // a camelCased model name results in Kirby\Exception\NotFoundException
            'simplepagemodel' => [
++              'type'=> null,
                'fields' => [
                    'id'    => function ($page) {
                        return $page->id();
                    },
                    'title' => function ($page) {
                        $query = option('gearsdigital.enhanced-toolbar-link-dialog.link.title', '{{ page.title }}');
                        return Str::template($query, [
                            'page' => $page,
                            'site' => site(),
                            'kirby' => kirby(),
                        ]);
                    },
                    'slug'  => function ($page) {
                        return URL::makeAbsolute($page->parent().DS.$page->slug());
                    },
                ],
            ],
        ],
        'collections' => [
            'simplepagecollection' => [
++             'type'=> null,
                'model' => 'simplepagemodel',
            ],
        ],
        'routes'      => [
            [
                'pattern' => 'enhanced-toolbar-link-dialog/pages',
                'method'  => 'get',
                'action'  => function () {
                    $page = get('page');
                    $query = get('search');

                    if (empty($query)) {
                        $query = '*';
                    }

                    $pagedCollection = site()->search($query, 'title')->paginate([
                        'page'  => $page,
                        'limit' => 10,
                    ]);

                    return $this->collection('simplepagecollection', $pagedCollection);
                },
            ],
        ],
    ],
    'translations' => [
        'en' => [
            'gearsdigital.enhanced-toolbar-link-dialog.internal' => 'Internal Link',
            'gearsdigital.enhanced-toolbar-link-dialog.external' => 'External Link',
            'gearsdigital.enhanced-toolbar-link-dialog.empty'    => 'No pages found',
            'gearsdigital.enhanced-toolbar-link-dialog.target.title' => 'Link Target',
            'gearsdigital.enhanced-toolbar-link-dialog.target.help' => 'Specify where to open the linked document.'
        ],
        'de' => [
            'gearsdigital.enhanced-toolbar-link-dialog.internal' => 'Interner Link',
            'gearsdigital.enhanced-toolbar-link-dialog.external' => 'Externer Link',
            'gearsdigital.enhanced-toolbar-link-dialog.empty'    => 'Keine Seiten gefunden.',
            'gearsdigital.enhanced-toolbar-link-dialog.target.title' => 'Link Ziel',
            'gearsdigital.enhanced-toolbar-link-dialog.target.help' => 'Gibt an, wo das verknüpfte Dokument geöffnet werden soll.',
        ],
    ],
]);

Could you check that for me? Using ´error_reporting(E_ALL);´ no error is shown.

cgundermann commented 4 years ago

Yes, that's exactly what I wrote above =) Works fine now with type declarations for both, the model and the collection.

gearsdigital commented 4 years ago

My bad 🙈

I'ld love to have you as contributer here! https://opensource.com/article/19/7/create-pull-request-github

gearsdigital commented 4 years ago

@janherman Can you verify the solution works for you?

cgundermann commented 4 years ago

Thanks for the Tutorial Page (*embarrassed). I'll check it out in a minute 😀

janherman commented 4 years ago

Yes, I can confirm this solution works with my setup. 👍

gearsdigital commented 4 years ago

Sorry, I didn't mean to embarrass you @CrisGraphics! I assumed that you're not familiar with making pull requests... If you have any questions please let me know. I'll guide you through the process if you want to. Just want to make sure you get the credits here :)

gearsdigital commented 4 years ago

@janherman Awesome! Thanks for checking out.

I'll provide a fix as soon as @CrisGraphics made the PR (No pressure @CrisGraphics. Take your time! ☺️ )

cgundermann commented 4 years ago

I assumed that you're not familiar with making pull requests...

You were assuming right, haha. Ok, I tried to make a pull request. Hope I didn't blow up GitHub or the Internet now...

gearsdigital commented 4 years ago

How awesome :)

gearsdigital commented 4 years ago

Thank you very much @janherman and @CrisGraphics! As this issue seems to be resolved now I'm going to close it :)