Resolve mediaModel also for classes that extend Layout.

[dinandmentink]

This is an edgecase that I came across that is easy to allow. Situation:

1. I have an ApplicationLayout class which I use as a base class instead of Layout to add some application specific functionality which I use in my Layout classes.
2. I have a ImageColumns layout which has a flexible field of ImageColumn, the image column has an Images::make("image") field.

This does not correctly resolve because getMediaModel() checks agains Layout, not ApplicationLayout. Using self fixes this, but also still works for the regular case.

[toonvandenbos]

Hi @dinandmentink, thanks for the PR. However, if your ApplicationLayout extends Layout then $model instanceof Layout should still work. I'm assuming your layout class does not extend the package's default Layout, what is your reason behind this choice?

[dinandmentink]

Thanks for the response. So this is the full case (code removed where irrelevant).

I have an ApplicationLayout which adds some boilerplate that I use in this application for every layout:

<?php

namespace App\Nova\Flexible\Layouts;

use App\Nova\Fields;
use App\Services\Blocks as BlocksService;
use DinandMentink\Markdown\Markdown;
use Laravel\Nova\Fields\Select;
use Spatie\MediaLibrary\HasMedia;
use Whitecube\NovaFlexibleContent\Concerns\HasMediaLibrary;
use Whitecube\NovaFlexibleContent\Flexible;
use Whitecube\NovaFlexibleContent\Layouts\Layout;

abstract class ApplicationLayout extends Layout implements HasMedia
{
    use HasMediaLibrary;

    protected function backgroundField()
    {
        return Select::make('Achtergrond', 'background')
            ->options(BlocksService::BACKGROUNDS)
            ->stacked()
            ->displayUsingLabels();
    }

    protected function markdownField()
    {
        return Markdown::make('Content')
            ->stacked();
    }
}

Then I have an ImageColumns layout which extends this ApplicationLayout. The goal of this layout is to add sublayouts which have an Image and some content:

<?php

namespace App\Nova\Flexible\Layouts;

use Whitecube\NovaFlexibleContent\Concerns\HasFlexible;
use Whitecube\NovaFlexibleContent\Flexible;

class ImageColumns extends ApplicationLayout
{
    use HasFlexible;

    /**
     * The layout's unique identifier
     *
     * @var string
     */
    protected $name = 'image-columns';

    /**
     * The displayed title
     *
     * @var string
     */
    protected $title = 'Kolommen met afbeeldingen';

    /**
     * Get the fields displayed by the layout.
     *
     * @return array
     */
    public function fields()
    {
        return [
            self::backgroundField(),

            Flexible::make('Kolommen', 'columns')
                ->stacked()
                ->addLayout(ImageColumn::class)
                ->button('Kolom toevoegen'),
        ];
    }

    public function getColumnsAttribute()
    {
        return $this->flexible(
            'columns',
            [
                'image-column' => ImageColumn::class,
            ]
        );
    }
}

And then the ImageColumn layout:

<?php

namespace App\Nova\Flexible\Layouts;

use Ebess\AdvancedNovaMediaLibrary\Fields\Images;

class ImageColumn extends ApplicationLayout
{
    /**
     * The layout's unique identifier
     *
     * @var string
     */
    protected $name = 'image-column';

    /**
     * The displayed title
     *
     * @var string
     */
    protected $title = 'Kolom met afbeelding';

    /**
     * Get the fields displayed by the layout.
     *
     * @return array
     */
    public function fields()
    {
        return [
            Images::make('Image', 'image-column_image')
                ->stacked()
                ->rules(['required']),

            self::markdownField(),
        ];
    }
}

In this case the ImageColumn->getMediaModel() resolves to ImageColumns because ImageColumns doesn't extend Layout but ApplicationLayout.

Or am I missing something?

Post scriptum. I can get it to work by overwriting the getMediaModel() on ApplicationLayout and change the while to either ApplicationLayout or self.

[dinandmentink]

After some more consideration. Would it be better to change the pr to something like:

while ($model instanceof Layout || is_subclass_of($model, Layout))

[toonvandenbos]

In this case the ImageColumn->getMediaModel() resolves to ImageColumns because ImageColumns doesn't extend Layout but ApplicationLayout.

Ok, but ImageColumns extends ApplicationLayout which extends Layout, meaning it is extending Layout indirectly and therefore $model instanceof Layout should return true. If it's not, I'm missing something here or $model is not one of your ApplicationLayout instances at some point in the loop.

[dinandmentink]

Ok. Thanks for the reply. Let me do some more digging to see what I'm missing. Somewhere along the chain I end up with a Layout instance and no medialibrary.

Edit.

Honestly, I have no idea:

>>> $this->model
=> App\Nova\Flexible\Layouts\ImageColumns {#4860
     +mediaConversions: [],
     +mediaCollections: [],
   }

>>> get_parent_class($this->model)
=> "App\Nova\Flexible\Layouts\ApplicationLayout"

>>> get_parent_class(get_parent_class($this->model))
=> "Whitecube\NovaFlexibleContent\Layouts\Layout"

But

>>> $this->model instanceof Layout
=> false

>>> is_subclass_of($this->model, Layout::class)
=> false

>>> class_parents($this->model)
=> [
     "App\Nova\Flexible\Layouts\ApplicationLayout" => "App\Nova\Flexible\Layouts\ApplicationLayout",
     "Whitecube\NovaFlexibleContent\Layouts\Layout" => "Whitecube\NovaFlexibleContent\Layouts\Layout",
   ]

>>> is_subclass_of($this->model, \App\Nova\Flexible\Layouts\ApplicationLayout::class)
=> true

From my understanding instanceof or is_subclass_of should go multiple levels deep, but now they don't.

[voidgraphics]

All I can think of is a namespace problem somewhere. It's hard to say from your example:

$this->model instanceof Layout;

This check would make it clearer:

$this->model instanceof Whitecube\NovaFlexibleContent\Layouts\Layout;

This is pretty obvious, but as a quick sanity check, I have confirmed that it does work through multiple levels of inheritance. At least with these dummy classes.

[dinandmentink]

I will retry with the explicitly named class. Probably I am making a mistake somewhere and ending up with a wrong Layout instance. For now we can close this, I'll reply later on if I find something more substantial.

Thanks for the reply and the help.

[dinandmentink]

Lightbulb. I think I found it. Just checking to see if it makes sense.

In HasMediaLibrary.php:

namespace Whitecube\NovaFlexibleContent\Concerns;

use Spatie\MediaLibrary\InteractsWithMedia;
use Spatie\MediaLibrary\MediaCollections\MediaRepository;
use Whitecube\NovaFlexibleContent\FileAdder\FileAdder;
use Whitecube\NovaFlexibleContent\FileAdder\FileAdderFactory;
use Whitecube\NovaFlexibleContent\Flexible;
use Spatie\MediaLibrary\Downloaders\DefaultDownloader;
use Spatie\MediaLibrary\HasMedia;
use Spatie\MediaLibrary\MediaCollections\Exceptions\InvalidUrl;
use Laravel\Nova\Http\Requests\NovaRequest;
use Laravel\Nova\Nova;
use Illuminate\Support\Collection;
use Illuminate\Support\Str;
use Ebess\AdvancedNovaMediaLibrary\Fields\Media;
use Whitecube\NovaFlexibleContent\Http\ScopedRequest;

trait HasMediaLibrary {

    use InteractsWithMedia;

    protected function getMediaModel() : HasMedia
    {
        $model = Flexible::getOriginModel() ?? $this->model;

        while ($model instanceof Layout) {
          $model = $model->getMediaModel();
        }

        if(is_null($model) || !($model instanceof HasMedia)) {
            throw new \Exception('Origin HasMedia model not found.');
        }

        return $model;
    }

There is no use Whitecube\NovaFlexibleContent\Layouts\Layout so while ($model instanceof Layout) actually references Whitecube\NovaFlexibleContent\Concerns\Layout. This raises only an error in the specific case of using sub-layouts. Adding the use statement fixes this issue.

I'll reopen the pr with the use statement.