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?

You should create foreign keys in your database so the row is deleted automatically. — Tim, Dec 08 '17 at 16:11
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. — andi79h, Dec 08 '17 at 16:16
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? — Tim, Dec 08 '17 at 16:21
What I mean: you are accessing the same data via these two queries. There is no additional security at this point I think. — Tim, Dec 08 '17 at 16:22
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)... — andi79h, Dec 08 '17 at 16:29
You trying to use `$channel->labels()->find($id)->delete();` but it will delete only 1 channel? What about just `$channel->labels()->delete();`? — Makashov Nurbol, Dec 08 '17 at 17:18
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. — andi79h, Dec 08 '17 at 17:26
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 — Makashov Nurbol, Dec 08 '17 at 17:34
Yeah, that's what I wanted, just one Label at a time but thread safe and if possible with some nice ORM tools :) — andi79h, Dec 08 '17 at 17:43
Arrrrgh you are right, I didnt recognize this one. I just checked: https://www.dropbox.com/s/cz7s9qzt8w1ldeu/laravel_orm1.gif?dl=0 and it works perfectly (meaning it does nothing in this case) — andi79h, Dec 08 '17 at 17:58

Makashov Nurbol · Accepted Answer · 2021-10-12T09:50:11.443

22

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.

edited Oct 12 '21 at 09:50

answered Dec 08 '17 at 16:34

Makashov Nurbol

574
3
13

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) – Ray Coder Nov 04 '20 at 08:35

score 1 · Answer 2 · answered Dec 08 '17 at 16:36

1

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();
}

answered Dec 08 '17 at 16:36

Raza Mehdi

923
5
7

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). – andi79h Dec 08 '17 at 16:49

score 0 · Answer 3 · answered Dec 08 '17 at 17:45

0

You can use findorfail:

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

answered Dec 08 '17 at 17:45

Javid Karimov

435
5
22

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

Update:

3 Answers3