getkirby / kirby

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

Can't edit page with model in panel #2393

Closed LeBenLeBen closed 4 years ago

LeBenLeBen commented 4 years ago

Describe the bug
When I try to save a page in the panel that uses a custom Model, the following error is thrown:

The form could not be saved

Exception: TypeError Argument 1 passed to Kirby\Cms\App::{closure}() must be an instance of Page, instance of ExpertisePage given

To Reproduce
Here's my page blueprint (expertise.yml):

title: Expertise

preset: page

icon: 📦

pages:
  template:
    - articles-list
    - contact-form
    - free-text
    - logos-list
    - products
    - product
    - project-card
    - quote

fields:
  seo: fields/seo

  icon:
    label: Icon
    type: text
    required: true
    width: 1/2

  headline:
    label: Headline
    type: text
    help: Displayed below the expertise title
    required: true
    width: 1/2

  teaser:
    label: Teaser
    type: text
    help: Displayed in the expertise teaser
    width: 1/2
    maxlength: 120

  teaserTitle:
    label: Teaser title
    type: text
    help: Expertise teaser “title” attribute (for SEO)
    width: 1/2
    maxlength: 120

  intro:
    label: Intro
    type: markdown
    required: true

  champion:
    label: Champion
    type: users
    max: 1
    default: false
    help: Display a picture & office phone number after the expertise introduction.

Model (expertise.php):

<?php

use Kirby\Cms\Page;

class ExpertisePage extends Page {

  public function metaTitleIncludesHierarchy(): bool
  {
    return false;
  }

}

Expected behavior
I should be able to save the page like any other.

Kirby Version
3.3.2

afbora commented 4 years ago

I can't reproduce this issue on 3.3.2 Starterkit. Could it be from plugins or hooks that wrong syntax?

texnixe commented 4 years ago

I haven't tested with Markdown field but can't reproduce this either. I don't think it has anything to do with the model and I've been using many page models in 3.3.2 without any issues.

LeBenLeBen commented 4 years ago

Indeed, the problem was coming from a page hook that was defined as function(Page $page) { … }. Changing it for function($page) { … } fixed the issue.

Sorry for the noise and thanks for the help!

distantnative commented 4 years ago

@LeBenLeBen We struggled with that issue ourselves: PHP in its current versions doesn't treat type hinting with objects very well. It doesn't matches withdecendants but only the exact class. We removed all our object type hints for this reason.

lukasbestle commented 4 years ago

@distantnative That's only the case for return types though (which was fixed in PHP 7.4, so we'll only need to wait a few more years until we can use that... :D).

Param type hints should actually work as your ExpertisePage is also a Page object and should therefore be supported by that Page type hint.

The only issue that comes to my mind is that the internal class alias of Kirby\Cms\Page to Page may cause issues. @LeBenLeBen Do you have the use Kirby\Cms\Page line also in the file where the hook is defined?

LeBenLeBen commented 4 years ago

No use Kirby\Cms\Page was not present in the file with the hook. I'm in the process of explicitely defining the use everywhere.

afbora commented 4 years ago

I tried with following type hints as ExpertisePage and Page, works properly. It should work anyway like @lukasbestle said.

'page.update:before' => function (ExpertisePage $page, $values, $strings) {
    // your code goes here
}

'page.update:before' => function (Page $page, $values, $strings) {
    // your code goes here
}

I think there is an incorrect ExpertisePage model file in the site or plugin models 🤔

lukasbestle commented 4 years ago

I have been able to reproduce the original issue with a missing use statement. Once I added it, the same code worked just fine. Seems to be a bug in PHP that makes aliases not work for type hinting.