Delete function in Django not working

Question

I am trying to create a delete function for my Workout model. This is the model:

class Workout(models.Model):
    workoutID = models.AutoField(primary_key=True)
    name = models.CharField(max_length=40)
    created_by = models.ForeignKey(User)
    description = models.TextField()   
    created_at = models.DateTimeField(auto_now_add=True)

    def delete(self):
        return reverse("delete_workout", kwargs = {'workout_id': self.workoutID})

Next I have the view:

def delete_workout(request, workout_id):
    workout = get_object_or_404(Workout, workoutID = workout_id)
    print(workout)
    if request.user != workout.created_by:
        return HttpResponse('Not ur workout')
    else:
        workout.delete()
        return HttpResponseRedirect('/')

This is the url:

url(r'^(?P<workout_id>\d+)/delete/$', views.delete_workout, name='delete_workout'),

And finally the html:

<a href='{{ instance.delete }}'>
    <button>Delete Workout</button> 
</a>

I'm not getting any errors in the console, which is why I don't know what is going wrong.

I'd strongly advise you to use the class based views (DeleteView) and read the documentation for the models. Your method delete in Workout is a big mess and total antipattern. — cezar
– cezar, Commented Feb 21, 2018 at 8:32

Aneesh R S · Accepted Answer · 2018-02-21 09:12:11Z

1

You are overriding delete method of the class just for getting the delete url. You will get the url by url function in the template like {% url delete_workout instance.workoutID %}. So remove the delete function from the model change your html href url. Leave the view and url as the same. No issues there

class should be

class Workout(models.Model):
    workoutID = models.AutoField(primary_key=True)
    name = models.CharField(max_length=40)
    created_by = models.ForeignKey(User)
    description = models.TextField()   
    created_at = models.DateTimeField(auto_now_add=True)

And your html should be

<a href='{% url delete_workout instance.workoutID %}'>
    <button>Delete Workout</button> 
</a>

NOTE: django model itself adds id for each table, so you dont have to specify it as you did workoutID = models.AutoField(primary_key=True). By default each model will have a id field just like id = models.AutoField(primary_key=True)

If you consider removing the workoutID then the model becomes

class Workout(models.Model):
        name = models.CharField(max_length=40)
        created_by = models.ForeignKey(User)
        description = models.TextField()   
        created_at = models.DateTimeField(auto_now_add=True)

and the html will be

<a href='{% url delete_workout instance.id %}'>
    <button>Delete Workout</button> 
</a>

edited Feb 21, 2018 at 9:12

answered Feb 21, 2018 at 9:00

Aneesh R S

3,8174 gold badges25 silver badges35 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

cezar Over a year ago

Very good point about the primary key. I didn't notice it at first sight. Many beginners reinvent the wheel when using frameworks, which defeats the purpose of using a framework.

Aneesh R S Over a year ago

ya, absolutely @cezar

cezar · Accepted Answer · 2018-02-21 21:23:28Z

Django has all the tools for you under the hood. Don't reinvent the wheel. You can refactor and simplify your code.

First remove the method delete in Workout.

Second, replace your function-based-view with a class-based-view:

from django.views.generic.edit import DeleteView
from django.urls import reverse_lazy
from django.http import Http404

from .models import Workout

class WorkoutDeleteView(DeleteView):
    model = Workout
    success_url = reverse_lazy('delete_workout')

    def get_object(self):
        obj = super().get_object()
        if obj.created_by != self.request.user:
            raise Http404
        return obj

A workout can be deleted only by its author. In success_url you specify the target where the user should be redirected after deleting.

Just adapt slightly your urls.py (pay attention to the emphasised part):

url(r'^(?P<pk>\d+)/delete/$', views.WorkoutDeleteView.as_view(), name='delete_workout'),

EDIT: You can name your views as you please, however it would be better to follow already well established conventions. Thus the names for the class based views should be workout-list, workout-detail, workout-create, workout-update and workout-delete.

Collectives™ on Stack Overflow

Delete function in Django not working

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related