[7.x] Fix MorphPivot::delete for models with primary key

[ThomasTrost]

Hi,

I encounter problems when I try to delete models that extend Illuminate\Database\Eloquent\Relations\MorphPivot. I am by no means experienced with Laravel core development but I think I figured out what the root cause of the issue is. It is just a few lines of code, so instead of filing a bug report I offer this pull request with my solution for the experienced Laravel developers to check.

If new tests are required for this situation, I would also kindly ask those of you who are familiar with developing for the framework for help because I did not find much information on the standards of testing in the contribution guide.

Thanks for your help!

The problem from a user's perspective

Consider the following two pivot models:

use Illuminate\Database\Eloquent\SoftDeletes;
use Illuminate\Database\Eloquent\Relations\MorphPivot;

class MyMorphPivotModel extends MorphPivot
{
    use SoftDeletes;

    // ...
}

use Illuminate\Database\Eloquent\SoftDeletes;
use Illuminate\Database\Eloquent\Relations\Pivot;

class MyPivotModel extends Pivot
{
    use SoftDeletes;

    // ...
}

Both models are equipped with their own primary key id. The table structures look something like this:

my_morph_pivot_models
    id - integer
    foreign_id - integer
    my_morph_pivot_model_id - integer
    my_morph_pivot_model_type - string
    ...

my_pivot_models
    id - integer
    foreign_id - integer
    other_foreign_id - integer
    ...

Now, when I instantiate the models and try to delete them, their behavior differs:

$my_pivot_model->delete(); // works as expected
$my_morph_pivot_model->delete(); // throws an exception

The exception looks like this:

Illuminate\Database\QueryException: SQLSTATE[HY000]:
General error: 25 column index out of range 
(SQL: update "my_morph_pivot_models" set "deleted_at" = 2020-04-17 11:02:27, "updated_at" = 2020-04-17 11:02:27 where ("" = 1))

Apparently, the name of the primary id column does not find its way into the query for the MyMorphPivotModel while the equivalent delete-operation for MyPivotModel works just fine.

Tentative solution

When trying to track the problem down, I compared the delete methods of Pivot and MorphPivot. Pivot get its delete method from Illuminate\Database\Eloquent\Relations\Concerns\AsPivot, which looks like this:

public function delete()
{
    if (isset($this->attributes[$this->getKeyName()])) {
        return (int) parent::delete();
    }

    if ($this->fireModelEvent('deleting') === false) {
        return 0;
    }

    $this->touchOwners();

    return tap($this->getDeleteQuery()->delete(), function () {
        $this->fireModelEvent('deleted', false);
    });
}

MorphPivot has its own delete method:

public function delete()
{
    if ($this->fireModelEvent('deleting') === false) {
        return 0;
    }

    $query = $this->getDeleteQuery();

    $query->where($this->morphType, $this->morphClass);

    return tap($query->delete(), function () {
        $this->fireModelEvent('deleted', false);
    });
}

The obvious difference is that Pivot::delete has a fallback to parent::delete if a primary key attribute is set, while MorphPivot::delete lacks this construction. In the light of the aforementioned problems with the name of the primary key, this difference is worth a closer look.

Adding the lines

if (isset($this->attributes[$this->getKeyName()])) {
    return (int) parent::delete();
}

at the beginning of MorphPivot::delete makes the problem disappear and makes Pivot::delete and MorphPivot::delete more symmetric.

The addition of these lines is just what this pull request is about.