Support for multiple polymorphic relations against the same model

philo23 commented 4 years ago

The Problem

Currently you can't have more than one polymorphic relationship against a single "type" of model at a time. I propose some how the option to include an optional "group" to the morphable_type value.

For example: An Order model that has a delivery address and an invoice address, each represented by a polymorphic Address model. Without manually adding an extra column to the Address model to track the 'delivery' or 'invoice' subtype the two relations end up getting merged together.

Aside: I have figured out how to patch the Models with a trait to get something like this working, which I've included at the bottom of this post, but I think actually using it as-is would be a ~~bad~~ terrible idea as it depends on backtracing to get the relationship name and would also break the related morphs if you ever change their method names.

Example

The problem is probably clearest with an actual example so heres one using an Order with two Addresses for delivery and invoice from earlier:

// Migration
Schema::create('addresses', function ($table) {
    $table->bigIncrements('id');
    $table->morphs('addressable'); // addressable_type, addressable_id
});

Schema::create('orders', function ($table) {
    $table->bigIncrements('id');
});

// Models
class Address extends Model
{
    public function addressable () {
        return $this->morphTo('addressable');
    }
}

class Order extends Model
{
    public function deliveryAddress()
    {
        return $this->morphOne(Address::class, 'addressable');
    }
    public function collectionAddress()
    {
        return $this->morphOne(Address::class, 'addressable');
    }
}

// Example
$delivery_address = new Address([/* ... */]);
$collection_address = new Address([/* ... */]);

$order = Order::create([/* ... */]);
$order->deliveryAddress()->create($delivery_address);
$order->collectionAddress()->create($collection_address);

// $order->collectionAddress will actually return the
// $delivery_address model, this is because both Addresses have
// the same addressable_type, 'App\Order' with a MorphOne relation
dump($order->collectionAddress);

Current Solutions

The two solutions that can work around this right now are:

Creating a new custom column to store the "group" and then including a ->where() on the end of the morphOne() relation. While this will solve fetching the related models, it "breaks" saving/creating them through the relation because Laravel doesn't know it needs to automatically fill out the extra column.

public function deliveryAddress() {
    return $this->morphOne(Address::class, 'addressable')->where('addressable_group', 'delivery');
}

// if you forget to include your custom addressable_group value 
// the related model won't really be related anymore...
$order->deliveryAddress()->create([ 'addressable_group' => 'delivery', /* ... */]);

The other solution would be including an intermediate one-to-one Model, something like:
```
// Order model
public function deliveryAddress()
{
    return $this->hasOne(DeliveryAddress::class);
}

// DeliveryAddress model
public function address()
{
    return $this->morphOne(Address::class, 'addressable')
}
```
This second way seems like the "correct" Laravel way to do it right now, but also seems like overkill to create a whole new intermediate model/table just to get a unique identifier for the relation.

Proposed Solution

When defining a polymorphic relationship include the option to set a group/subtype or whatever you decide to call it, eg:

public function deliveryAddress()
{
    return $this->morphOne(Address::class, 'addressable')->group('delivery');
}

public function collectionAddress()
{
    return $this->morphOne(Address::class, 'addressable')->group('collection');
}

Which would then give you the option to store the addressable_type MorphClass value as something like App\Address::delivery instead of just App\Address. Doing it this would mean theres no backward compatibility break by adding a new parameter to the internals of morphOne() and morphMany() and would also still let you customise the morph class with Relation::morphMap.

Not including a ->group() or setting it to null would mean morphs work exactly as they do now, but setting one would effectively group all of those morphs into the same bucket. I also see no reason why morphMany() couldn't benefit from this too.

Proof of Concept

I've included two traits below that you can apply to a model to get a very crude version this functionality working, but as I mentioned the implementation is more of a hack than something I'd want to actually use in production and depend on always working.

As far as I can tell though these are the only two affected methods, getMorphClass on the parent model and newRelatedInstance of the morphing model, even if its not actually the right place to be injecting these new checks.

To be clear though, I don't think this should be the exact solution implemented, its more-so proof that its possible and the current code isn't a mile away from being able to do this.

https://gist.github.com/philo23/9d023f2e15705a5202a2c80e2ac3342a

If there's any interest in this concept, I'd be happy to have a go at actually implementing it, but I'll probably have a few questions. Thanks for reading through this wall of text!

wmjasonward commented 4 years ago

Typically, if an Address is serving a specific role within the context of the Order, I would just add the id of the address to the order:

Schema::create('orders', function ($table) {
    $table->bigIncrements('id');
    ...
    $table->unsignedBigInteger('delivery_address_id')->nullable();
    $table->unsignedBigInteger('collection_address_id')->nullable();
    ...

  $table->foreign('delivery_address_id')->references('id')->on('addresses');
  $table->foreign('collection_address_id')->references('id')->on('addresses');
});

// On the Order model
public function deliveryAddress() : BelongsTo
{
    return $this->belongsTo(Address::class, 'delivery_address_id');
}

public function collectionAddress() : BelongsTo
{
    return $this->belongsTo(Address::class, 'collection_address_id');
}

If you need dynamic address roles (attaching more addresses to the order than just the delivery and collection address), you would need a third table with the role there:

Schema::create('order_address', function($table) {
    $table->bigIncrements('id');
    $table->unsignedBigInteger('order_id');
    $table->unsignedBigInteger('address_id');

    // I'm showing an enum here just to clarify the purpose of this column
   // that may or may not work depending on your business rules
    $table->enum('role', ['delivery_address', 'collection_address', 'shipping_address']);

    // how you index (and even the primary key) depends on your business rules
}

Note: the code above was typed in directly without any testing :)

reza-akbari commented 4 years ago

using morphMap and getMorphClass like:

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

// Illuminate\Database\Eloquent\Relations\Relation;
Relation::morphMap([
  'DeliveryOrder' => Order::class,
  'CollectionOrder' => Order::class,
]);

class Address extends Model
{
  public function addressable () {
    return $this->morphTo('addressable');
  }
}

class Order extends Model
{
  protected $morphClass = null;

  public function getMorphClass () {
    return $this->morphClass ?: static::class;
  }

  public function deliveryAddress () {
    $this->morphClass = 'DeliveryOrder';
    return $this->morphOne(Address::class, 'addressable');
  }

  public function collectionAddress () {
    $this->morphClass = 'CollectionOrder';
    return $this->morphOne(Address::class, 'addressable');
  }
}

mewejo commented 3 years ago

This would be nice, because there (that I can find) doesn't seem to be a way to link models together without defining the names of the relationships explicitly.

For example, if you have several models such as:

Article
Newsletter
Blog post
Event

If a user is looking at an article and you wanted to show them related content (could be any other model the admin has linked), such as a newsletter or event. You can't currently do that without a middle model as @philo23 suggests.

I was hoping to just create a trait called Relatable and then apply it to each of my models. Then I could just access $model->related and fetch a list of other models related to that one. Another method on the model class could be used to find the inverse.

It would work just like many-many polymorphic relationships do now, but it would just have one more column to store the model of the first ID column.

laravel / ideas