Deleting by id from child deletes all records associated with that id

[tighten]

Deleting by id from child deletes all records associated with that id

[dallincoons]

@robbydooo I'm curious about what you database schema looks like. This package is meant to introduce Single Table Inheritance to Eloquent, which means that Content, Review, Posts, and Pages should all be in the same table where each row would have a unique primary key. Review::destroy(1) should only delete one record if this is the case.

[robbydooo]

@dallincoons I am attempting to use this on a table where the id and the type field are unique. The table is to store content related to locations so the id field is a foreign key to a locations table that all the STI relate to.

E.g London (Location Model)--->Content (parent model) -> Reviews, Posts, Pages (Child Models)

There is only one child type per location as its just holding JSON data imported from an API.

My understanding was that the global scope being applied to all queries would have meant Review::destroy(1) or the delete() should have added the "where type = xxx" clause but that clearly isn't the case :-).

Sounds like i might need to move the location id to a separate field and keep id unique for this package to work? I can then have a unique constraint on the location_id and type fields.

Thanks,
Rob

[calebporzio]

Hey @robbydooo - yes, currently this package only supports the standard: id's as unique primary keys.

I'd be open to supporting non-unique ids like your case. Let me know if you can do some exploration on this feature and maybe PR it - thanks.

[robbydooo]

@calebporzio thanks for your work on this. I will try and do some research on this in the new year. I need to understand global scopes a lot better than I currently do to submit any PR.

Thanks

[robbydooo]

I have been working on this issue for some time now and finally understand whats happening.
In general, this library supports composite keys.

The area where it does not is in saving/deleting of data to the database. This is because the global scopes and event listeners used in the trait only apply to "SELECT" queries.

Update/delete queries are currently ignored in these scopes/listeners.

To solve this i have found that adding this code to the parent model fixes this issues.

/**
     * REQUIRED FOR Tighten/PARENTAL library to work with composite keys.
     *
     * The parental library does not support composite keys out of the box but overriding this function solves the problem.
     * The global scopes update the queries but not the commits to the database. This sets the keys in the write queries to the composite key.
     *
     * If this is removed, all records for an id will get overwritten each time a save is executed.
     *
     * See https://github.com/tightenco/parental/issues/25
     *
     * @param \Illuminate\Database\Eloquent\Builder $query
     * @return \Illuminate\Database\Eloquent\Builder
     */
    protected function setKeysForSaveQuery($query)
    {
        $query->where($this->getKeyName(), '=', $this->getKeyForSaveQuery())
            ->where($this->getInhertanceColumn(), '=', $this->classToAlias(get_class($this)));

        return $query;
    }

This function overrides the setKeysForSaveQuery method that normally only returns the models id field and instead adds the type field to the where clause.

I am not sure if the best route forward is to just document this somewhere so other people can find it if they are using composite keys or whether we should have a boolean flag for composite keys that then modifies the setKeysForSaveQuery method.

Just to be clear, the issue this solves is when you are using the type field and the id field as a composite key so you can store different types of data on a related table. E.g Products, Users etc.

[calebporzio]

Thanks for posting the solution. Yeah, I'm not sure what to do with this. I'm going to close the issues for now. You are welcome to PR something to the bottom of the docs as a caveat, or something to package that wouldn't cost anything in performance or cause other issues.

At least someone will be able to find this fix by googling for it though. Thanks again.