artkonekt / concord

Laravel extension for building modular applications where modules are decoupled, re-usable and easily customizable
MIT License
211 stars 13 forks source link

[question] How to use custom models? #5

Closed enzonotario closed 5 years ago

enzonotario commented 5 years ago

Hi! I'm trying to use a custom model throw the ModelProxy, but I can't figure out how to do that.

I have created a repo that shows what I'm doing and a test that fails. I'm registering the custom model in the AppServiceProvider

Can you help me, please?

Thanks in advance!

fulopattila122 commented 5 years ago

The SomeProxy::modelClass() should give you what is the current class the original interface is bound to.

Your test case fails, because assertInstanceOf() expects the second parameter to be an object (instance, as it says) and you're giving the class name.

So in your case, the test shoud just simply check if the class names equal:

$this->assertEquals(PriceList::class, PriceListProxy::modelClass());
fulopattila122 commented 5 years ago

So, in general you're using Concord's polymorphic models correctly.

enzonotario commented 5 years ago

sorry, the test were bad.. I have fixed now, please check again.

In my custom model I have setted protected $table = 'customTable';. In the base model it is setted as protected $table = 'pricesLists';.

So, when I do:

$model = PriceListProxy::modelClass();
$instance = new $model;
$this->assertEquals('customTable', $instance->getTable());

it should pass, no?, but $instance->getTable() is still pricesLists. I want to get the overrides that App\Modules\Price\Models\PriceList does.

enzonotario commented 5 years ago

I also have added the assertion $this->assertEquals(PriceList::class, $model); and doesn't pass.

enzonotario commented 5 years ago

I have found a solution... before I had in PriceListProxy:

<?php

namespace Hawk\Price\Models;

use Konekt\Concord\Proxies\ModelProxy;

class PriceListProxy extends ModelProxy
{

    public function targetClass(): string
    {
        return PriceList::class;
    }

}

If I remove the targetClass function, leaving it as:

<?php

namespace Hawk\Price\Models;

use Konekt\Concord\Proxies\ModelProxy;

class PriceListProxy extends ModelProxy
{

}

the tests passes.. is it how I should use it?

enzonotario commented 5 years ago

ahh, in ModelProxy you call $this->targetClass(). If it is not defined, this call $this->concord->model($this->contract), so overrides are taken. But if I want to define a base model through targetClass and use $this->concord->model($this->contract) when I want to override through AppServiceProvider?

Or I mussunderstanding something?

fulopattila122 commented 5 years ago

Honestly.. I'm completely lost 😁 What do you want to achieve?

Some notes:

1.) Here you are comparing a string to an object, it won't be equal:

$this->assertEquals(PriceList::class, $model);

2.) Don't modify the proxy, they were not designed for that. The proxy only needs to be defined when the base model+contract in the library gets defined. In your library you should use PriceListProxy::modelClass() in HasOne, HasMany, etc relationships. This way if the application overrides the model, the library will use the overridden one.

fulopattila122 commented 5 years ago

I've checked your code. Here are some problems:

If you put all the files to their appropriate folder, Concord will make everything (migrations, config and default values, views, routes, etc) available to your applicationl

Here's a working example for your use case:

Base Module

https://github.com/vanilophp/product/

Base Model: src/Models/Product.php Proxy: src/Models/ProductProxy.php Interface: src/Contracts/Product.php

Extending The Model

https://github.com/vanilophp/framework

Custom Model: src/Models/Product.php The registration of the custom model: Provider

fulopattila122 commented 5 years ago

Another note:

Proxies are usually only needed in modules that define the base models. A very common use case is defining relationships like this: https://github.com/vanilophp/category/blob/0.4.0/src/Models/Taxon.php#L87

This way, if you later override the Taxonomy model, the $taxon->taxonomy relationship will return an instance to the overridden Taxonomy model without needing to modify the Taxon class.

Applications typically shouldn't deal with these model proxies at all.

enzonotario commented 5 years ago

Hey, thanks so much! I will dig deeper on that resources.

The problem for me was that I wanted to put the

Realtion::morphMap([
    'pricesLists' => PriceListProxy::modelClass(),
]);

in PriceServiceProvider. So I had to overwrite the targetClass of PriceListProxy (if not it throws error).

But it hast to be in AppServiceProvider, as you have in (https://github.com/vanilophp/framework/blob/678788d4625a7229407a2f24ffb5231009011e38/src/Providers/ModuleServiceProvider.php#L79).

But in this case, if I understand correctly, every app that uses this module will have to define the morphMap? There is a way to do it in the module side? Or you will just define the morphMap with the base model and, if the app overrides the base model, will have to also override the morphMap?

fulopattila122 commented 5 years ago

Yes, I would leave the definition of morph maps to the application. Generally speaking, the "smarter" your base module is, the harder it becomes the application to customize it.

enzonotario commented 5 years ago

ok perfect, thanks!!