Skip to content
This repository has been archived by the owner on Feb 2, 2022. It is now read-only.

Respect custom morph types #190

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

mmachatschek
Copy link
Contributor

@mmachatschek mmachatschek commented Apr 13, 2020

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 mmachatschek marked this pull request as draft April 13, 2020 13:14
@mmachatschek mmachatschek changed the title [WIP] Custom morph types Respect custom morph types Apr 13, 2020
@mmachatschek mmachatschek marked this pull request as ready for review April 13, 2020 16:47
@mmachatschek
Copy link
Contributor Author

mmachatschek commented Apr 14, 2020

@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.

@sandervanhooft
Copy link
Collaborator

Hi @mmachatschek ,

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

@mmachatschek
Copy link
Contributor Author

@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 mmachatschek marked this pull request as draft April 15, 2020 11:52
@jelleroorda
Copy link
Contributor

@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).

@sandervanhooft sandervanhooft mentioned this pull request May 11, 2020
4 tasks
@mmachatschek
Copy link
Contributor Author

@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.

@sandervanhooft
Copy link
Collaborator

@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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants