vanilophp / framework

The truly Laravel E-commerce Framework
https://vanilo.io
MIT License
818 stars 102 forks source link

Extending the foundation Product model loses images #127

Closed KodeStar closed 2 years ago

KodeStar commented 2 years ago
<?php
namespace App\Models;

use Vanilo\Foundation\Models\Product as BaseProduct;

class Product extends BaseProduct
{

}

Then I have

...
    public function service($slug)
    {
        $product = \App\Models\Product::findBySlug($slug);
        $product2 = \Vanilo\Foundation\Models\Product::findBySlug($slug);
        var_dump($product->hasImage());
        var_dump($product2->hasImage());
        die();
    }
...

Which is returning

bool(false) bool(true)

What am I missing here?

fulopattila122 commented 2 years ago
  1. Are you registering the updated model with Concord?
  2. Where is the public function service($slug) method?

See Using Custom Models and Registering the Custom Model

fulopattila122 commented 2 years ago

I guess it's using the Spatie media library. What's in the media table's model_type field? It is either the complete class name, or the short name of the model: image

If you register your extended model with Concord, then the product alias will be targeting to your App\Modules\Product class.

See Explicit Route Model Binding

fulopattila122 commented 2 years ago

Shortly, register your model in app/Providers/AppServiceProvider.php:

namespace App\Providers;

use Illuminate\Support\ServiceProvider;
use Vanilo\Product\Contracts\Product as ProductContract;

class AppServiceProvider extends ServiceProvider
{
    public function boot()
    {
        $this->app->concord->registerModel(
            ProductContract::class, \App\Models\Product::class
        );
    }
}
KodeStar commented 2 years ago

I'm already doing all that I think.

In app/Providers/AppServiceProvider.php

<?php

namespace App\Providers;

use Illuminate\Support\ServiceProvider;
use Vanilo\Order\Contracts\OrderStatus as OrderStatusContract;
use Vanilo\Product\Contracts\Product as ProductContract;
use App\Models\OrderStatus;

class AppServiceProvider extends ServiceProvider
{
    /**
     * Register any application services.
     *
     * @return void
     */
    public function register()
    {
        //
    }

    /**
     * Bootstrap any application services.
     *
     * @return void
     */
    public function boot()
    {
        $this->app->concord->registerModel(
            \Konekt\User\Contracts\User::class, 
            \App\Models\User::class,
            ProductContract::class,
            \App\Models\Product::class,
            \App\Models\Order::class
        );
        $this->app->concord->registerEnum(
            OrderStatusContract::class,
            OrderStatus::class
        );
        //
    }
}

In app\Http\Controllers\ProductController.php

<?php

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\Models\Product;
// use Vanilo\Product\Models\Product;
// use Vanilo\Foundation\Models\Product;
use Vanilo\Properties\Models\PropertyProxy;

class ProductController extends Controller
{
    /**
     * Create a new controller instance.
     *
     * @return void
     */
    public function __construct()
    {
    }

    /**
     * Show the service.
     *
     * @return \Illuminate\Contracts\Support\Renderable
     */
    public function service($slug)
    {
        $product = \App\Models\Product::findBySlug($slug);
        $product2 = \Vanilo\Foundation\Models\Product::findBySlug($slug);
        var_dump($product->hasImage());
        var_dump($product2->hasImage());
        die();
    }
}
image
fulopattila122 commented 2 years ago

Why did you think the registerModel method takes any number of arguments? Each model must be registered with a separate registerModel call

KodeStar commented 2 years ago

Because it wasn't clear in the docs, I just assumed you dropped them all in, the example adding an extra one would make it much clearer, either way though, it hasn't resolved the issue (though I do have a clearer understanding, thank you). I now have:

    public function boot()
    {
        $this->app->concord->registerModel(
            \Konekt\User\Contracts\User::class, \App\Models\User::class
        );
        $this->app->concord->registerModel(
            \Vanilo\Product\Contracts\Product::class, \App\Models\Product::class
        );
        $this->app->concord->registerModel(
            \Vanilo\Order\Contracts\Order::class, \App\Models\Order::class
        );
        $this->app->concord->registerEnum(
            OrderStatusContract::class,
            OrderStatus::class
        );
        //
    }

But still not getting images

fulopattila122 commented 2 years ago

Because it wasn't clear in the docs, I just assumed you dropped them all in

I'm sorry, but this is basic PHP, it's out of our scope to go into those details.

Nevertheless, if you could give me a working code sample I would be happy to take a look into it.

fulopattila122 commented 2 years ago

working code sample = I mean a zip file or a repo I can download, composer install and see what's going on there

KodeStar commented 2 years ago

Ok, it's now resolved, properly registering the model meant that it no longer showed in the admin and reuploading then worked (it shows in the model_type as App\Models\Product instead of product).

So the moral of the story is, if you switch to a custom model part way through building (I only switched because I decided I wanted to add a custom mutator), you need to re-upload the images again (and if you aren't a moron who registers the models wrong in the first place it will be more obvious because they will no longer show in the admin) or manually change the model_type in the database.

Thanks for the help.

image

KodeStar commented 2 years ago

Because it wasn't clear in the docs, I just assumed you dropped them all in

I'm sorry, but this is basic PHP, it's out of our scope to go into those details.

It's not really basic PHP as without knowing what the $this->app->concord->registerModel() does there's no reason you couldn't register them all within the single call, though it would make more sense for it to be an array of arrays in that case.

Trying to hit the road running with this framework does have a bit of a learning curve made a bit more frustrating by not everything seeming to work quite right.

That said, I love what you have so far and I understand it's a work in progress, keep up the good work and I'm sorry if I've come off as rude anywhere.

fulopattila122 commented 2 years ago

and if you aren't a moron who registers the models wrong in the first place

:laughing:

this framework does have a bit of a learning curve made a bit more frustrating

Yes, there are many parts not properly documented. Moreover, certain parts of the implementation (eg. channels) also have loose ends. But issue reports, questions and PRs like yours are helping a lot, so thank you for that!

if I've come off as rude anywhere.

No, not at all, it was helpful. This framework is dedicated to developers, so seeing how things don't work on your end gives us a lot of insight. :+1:

edotxxx commented 2 years ago

Ok, it's now resolved, properly registering the model meant that it no longer showed in the admin and reuploading then worked (it shows in the model_type as App\Models\Product instead of product).

Hi! By the way, you can use

Relation::morphMap([
            'product' => ProductProxy::modelClass(),
            'taxon' => TaxonProxy::modelClass(),
        ]);

in your AppServiceProvider to map your new class to shortname. You can find a little more info here: Vanilo Cart Doc section Buyable Morph Maps

KodeStar commented 2 years ago

Hi! By the way, you can use

Relation::morphMap([
            'product' => ProductProxy::modelClass(),
            'taxon' => TaxonProxy::modelClass(),
        ]);

in your AppServiceProvider to map your new class to shortname. You can find a little more info here: Vanilo Cart Doc section Buyable Morph Maps

Thanks for the tip, I'm not actually using the cart at all at the moment which is probably why I didn't stumble upon that nugget :)

I'm using Gumroad and someone makes a purchase on the site, I'm using the details from Gumroads ping to create an order (plus create a user + billpayer + customer if one doesn't exist) .

KodeStar commented 2 years ago

Also for reference it looks like since L8.x it's now Relation::enforceMorphMap - https://laravel.com/docs/8.x/eloquent-relationships#custom-polymorphic-types

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs. Thank you for your contributions.