How to delete single (many-) rows from one-to-many relations in Laravel 5.5

Question

I got two models in Laravel: Label

class Label extends \Eloquent
{
    protected $fillable = [
        'channel_id',
        'name',
        'color',
    ];

    public function channel()
    {
        return $this->belongsTo('App\Channel');
    }
}

And Channel

class Channel extends \Eloquent
{
    protected $fillable = [
        'name',
    ];

    public function labels()
    {
        return $this->hasMany('App\Label');
    }

}

Now when a label is deleted, I want to make sure, the label belongs to the channel.

This works pretty good, as it even is atomic, so the row will just be deleted, if the label really belongs to the channel.

LabelController:

/**
 * Remove the specified resource from storage.
 *
 * @param  int $id
 * @return \Illuminate\Http\Response
 */
public function destroy($id)
{
    $channel = $this->getChannel();

    Label::where('id', $id)
        ->where('channel_id', $channel->id)
        ->delete();

    return back();
}

And my question is now: How to build that with Eloquent, so it is elegant? Something like:

    $channel->labels()->destroy($id);

But there is no destroy function on the relation.

Update:

I managed to achieve something in the right direction:

$channel->labels()->find($id)->delete();

This deletes the label with $id BUT just if the label has the right channel_id assigned. If not, I get the following error, which I could catch and handle:

FatalThrowableError (E_ERROR) Call to a member function delete() on null

Still, as Apache is threaded, there could be the case that another thread changes the channel_id after I read it. So the only way besides my query is to run a transaction?

Hames

You should create foreign keys in your database so the row is deleted automatically. – Thatcher

I don't delete the Channel but the Label. In fact, there is a foreign key to cascade the channel, but that does not help as I don't delete the channel. In Laravel's docs most of the time there is just a Post:destroy($id) which is pretty ok, except you want to check for another flag (user_id, channel_id, etc) in terms of security issues. – Hames

I don't understand what you are trying to do. Why are you retrieving the channel $channel = $this->getChannel(); and also check in the query if the channel_id matches ->where('channel_id', $channel->id)? Shouldn't the first statement make the second one obsolete? – Thatcher

What I mean: you are accessing the same data via these two queries. There is no additional security at this point I think. – Thatcher

I just wanted to know if there is a more elegant way to achieve the same as my delete query using Laravel objects. The easy way would be to get the channel, check for the channel_id in the Label and then destroy the label. And yet, there could be another Apache process changing the channel_id inbetween so the select gives a go but the delete destroys a row that is not allowed to be deleted (anymore)... – Hames

You trying to use $channel->labels()->find($id)->delete(); but it will delete only 1 channel? What about just $channel->labels()->delete();? – Mercaptide

I was never trying to delete a channel. The called method is LabelController::destroy($id). I just want to additionally check whether or not the row can be deleted. And the check is simply if the channel_id fits. With $channel->labels()->find($id)->delete(); it will delete the Label if it is connected to channel. Think Laravel does something like select * from labels where channel_id = Channel->id and then find($id) on this result. If I force the destroy method with an $id that has a different channel I get the exception above. – Hames

Sorry when i said but it will delete only 1 channel I meant label, not channel. So $channel->labels()->delete(); will not delete channel, it will delete all labels related to this channel – Mercaptide

Yeah, that's what I wanted, just one Label at a time but thread safe and if possible with some nice ORM tools :) – Hames

I answered you hour ago but you didn't understand me :) – Mercaptide

Arrrrgh you are right, I didnt recognize this one. I just checked: dropbox.com/s/cz7s9qzt8w1ldeu/laravel_orm1.gif?dl=0 and it works perfectly (meaning it does nothing in this case) – Hames

Answer 1

If you want to delete related items of your model but not model itself you can do like this:

$channel->labels()->delete(); It will delete all labels related to the channel.

If you want to delete just some of labels you can use where()

$channel->labels()->where('id',1)->delete();

Also if your relation is many to many it will delete from third table too.

Mercaptide

I got an error 🧨 SQLSTATE[HY000]: General error: 1205 Lock wait timeout exceeded; try restarting transaction (SQL: delete from berkas where berkas.p_id = 3 and berkas.p_id is not null) – Dogwood

Answer 2

You mentioned that, you first want to check if a label has a channel. If so, then it should be deleted.

You can try something like this though:

$label = Label::find($id);
if ($label->has('channel')) {
    $label->delete();
}

Diphenyl

No, the labels always have a channel. Think of it as something like a user_id and some access management. There is always a channel but as you move the label to another channel (inbetween), the delete should fail (meaning doing nothing). – Hames

Answer 3

You can use findorfail:

$channel->labels()->findOrFail($id)->delete();

Nihilism