cashier-mollie icon indicating copy to clipboard operation
cashier-mollie copied to clipboard

Respect custom morph types

Open mmachatschek opened this issue 4 years ago • 6 comments

This adds support for custom morph map settings in Laravel.

Could be something for v2 release.

Currently, the morphed types are saved with the FQDN.

Now a configuration like this:

use Illuminate\Database\Eloquent\Relations\Relation;

Relation::morphMap([
    'user' => \App\User::class,
    'subscription' => \Laravel\Cashier\Subscription::class,
]);

..will result in database entries like, e.g order_items:

+----+----------------+--------------+--------------+----------+
| id | orderable_type | orderable_id | owner_type   | owner_id |
+----+----------------+--------------+--------------+----------+
|  7 | subscription   |            3 | user         |        1 |
+----+----------------+--------------+--------------+----------+

mmachatschek avatar Apr 13 '20 13:04 mmachatschek

@sandervanhooft in case you consider this PR, there should be a additional 2.x branch. I can add a UPGRADE file if necessary to mention that the data in the tables have to be updated if morph maps are in use.

mmachatschek avatar Apr 14 '20 17:04 mmachatschek

Hi @mmachatschek ,

Can you elaborate on what issue this is solving / feature this is adding? Haven't worked with MorphMaps before.

sandervanhooft avatar Apr 15 '20 10:04 sandervanhooft

@sandervanhooft Let's say you set the following configuration in Laravel:

use Illuminate\Database\Eloquent\Relations\Relation;

Relation::morphMap([
    'user' => \App\User::class,
]);

This tells Eloquent to use the string user as *_type on the morph table instead of App\User

In cashier-mollie you set the *_type e.g. owner_type with the get_class($owner) instruction directly. This way the tables contain the FQDN so App\User.

When querying these tables with Eloquent, you wont be able to find any entries because the owner_type is set to the FQDN.

A even better solution would be to not directly Model::create instead create a new instance of the model and associate the corresponding morph relation. I like this approach much more, so I gladly update the PR again.

e.g.

use App\User;
use Laravel\Cashier\Credit\Credit;

$owner = User::first();

$credit = new Credit([
  'currency' => 'EUR',
  'value' => 1000
]);

$credit->owner()->associate($owner);
$credit->save();

Custom morph types https://laravel.com/docs/master/eloquent-relationships#custom-polymorphic-types

mmachatschek avatar Apr 15 '20 10:04 mmachatschek

@sandervanhooft

Simply put, the problem that the MorphMap solves the problem is the pain of refactoring model names (or namespaces) when using morph relationships.

Example: I move my App\User to the App\Models\User namespace. Using the get_class() implementation, my order items will now not be associated with my users anymore 💥 .

With a MorphMap, I can change

Relation::morphMap([
    'user' => \App\User::class,
]);

to

Relation::morphMap([
    'user' => \App\Models\User::class,
]);

and then everything will work again (of course, granted that I used the MorphMap when the OrderItems etc. were created).

jelleroorda avatar May 09 '20 20:05 jelleroorda

@sandervanhooft If you want me to pick up the work mentioned here, let me know:

A even better solution would be to not directly Model::create instead create a new instance of the model and associate the corresponding morph relation. I like this approach much more, so I gladly update the PR again.

mmachatschek avatar Jun 18 '20 07:06 mmachatschek

@sandervanhooft If you want me to pick up the work mentioned here, let me know:

A even better solution would be to not directly Model::create instead create a new instance of the model and associate the corresponding morph relation. I like this approach much more, so I gladly update the PR again.

Can you explain why you think this is better? I only see two DB calls instead of one?

sandervanhooft avatar Jul 21 '20 16:07 sandervanhooft