1

I have a page that shows a list of objects and I want to add a button beside each one to enable the user to delete it. I've found a few questions about similar scenarios online but for some reason I keep getting errors when I try to replicate the solutions.

Here is the delete function in views.py:

def delete_dish(request, pk):
    query = Dish.objects.get_object_or_404(pk)
    supermenu = query['supermenu'].pk
    query.delete()
    return redirect('foodmenu:menu', supermenu)

Here is the form in the HTML template:

            {% if not supermenu.dish_set.count == 0 %}
            <ul>
            {% for dish in supermenu.dish_set.all %}


                <li>
                {{ dish.name }} - {{ dish.description }}
                <form action="{%  url 'foodmenu:delete_dish' dish.id %}" method="POST">
                    {% csrf_token %}
                    <button type="submit">X</button>
                </form>

                </li>


                    {% if not dish.price_set.count == 0 %}
                    <ul>
                    {% for price in dish.price_set.all %}
                        {% if price.label %}
                            <li>{{ price.label }}: {{ price.cost }}</li>
                        {% else %}
                            <li>{{ price.cost }}</li>
                        {% endif %}
                    {% endfor %}
                    </ul>
                    {% endif %}


                {% endfor %}
            </ul>

            {% else %}
            <p>No dishes on this menu!</p>

        {% endif %}

And here is the urls.py:

app_name = 'foodmenu'
urlpatterns = [
    ...
    path('dish/delete/<int:dish.id>', views.delete_dish, name="delete_dish")
]

When I click the button, the browser goes to ./dish/delete/1 (1 being the pk of the object), but Django returns a 404 error.

John Allie
  • 245
  • 2
  • 10
  • Are you sure there is an object with `pk=1`? Note that usually a database does *not* take the smallest unassigned integer, it keeps iterating. – Willem Van Onsem May 13 '18 at 17:47
  • just `query = get_object_or_404(pk); query.delete()`. Because `query` is your model. – dani herrera May 13 '18 at 17:52
  • I'm not sure. 1 is just the number that gets returned from `dish.id` where it is called in the HTML template. I'm very new at this so I'm a bit lost still. – John Allie May 13 '18 at 17:52

1 Answers1

2

Instead of query = Dish.objects.get_object_or_404(pk) you should use get_object_or_404 shortcut:

from django.shortcuts import get_object_or_404

from django.views.decorators.http import require_POST

@require_POST
def delete_dish(request, pk):
    if request.method
    query = get_object_or_404(Dish, pk=pk)
    supermenu = query.supermenu.pk
    query.delete()
    return redirect('foodmenu:menu', supermenu)

Also change your url pattern to this:

path('dish/delete/<int:pk>/', views.delete_dish, name="delete_dish")

UPD

As @daniherrera mentioned in his comment, you probably want to check request's method, to prevent accidental deletions. For this you can use require_POST decorator.

neverwalkaloner
  • 46,181
  • 7
  • 92
  • 100
  • fix your answer. remove `supermenu = query['supermenu'].pk` also `return redirect('foodmenu:menu', query.pk)` is ok. – dani herrera May 13 '18 at 17:55
  • after `query.delete()`, query does not exist. this will raise error here `return redirect('foodmenu:menu', query.pk)` – Lemayzeur May 13 '18 at 17:56
  • @daniherrera I've tested it, `query.pk` after deletion return None. Probably this is because of old version of Django, currently I'm able to use only 1.10, but I rollback my answer. – neverwalkaloner May 13 '18 at 18:00
  • This `supermenu = query['supermenu'].pk` looks weird, I can't understand how you can pass value to an model instance that way. – Lemayzeur May 13 '18 at 18:02
  • @Lemayzeur you are right. I missed this part. Probably OP means something like this: `query.supermenu.pk`. – neverwalkaloner May 13 '18 at 18:03
  • 1
    @neverwalkaloner, my bad. When you delete a model the pk becomes null. Nice. – dani herrera May 13 '18 at 18:03
  • I tried this but it somehow is preventing the template from rendering. Now if I try to load a page that includes delete buttons, I get a "NoReverseMatch" error: `Reverse for 'delete_dish' with arguments '(1,)' not found. 1 pattern(s) tried: ['dish\\/delete\\/\\$']` – John Allie May 13 '18 at 18:09
  • 1
    @neverwalkaloner, to avoid anti patterns, may be it is a good idea to check action is a POST: https://stackoverflow.com/a/46614 – dani herrera May 13 '18 at 18:20
  • 1
    @neverwalkaloner, may be a good opportunity to use [require_POST()](https://docs.djangoproject.com/en/2.0/topics/http/decorators/#django.views.decorators.http.require_POST) – dani herrera May 13 '18 at 18:23
  • @daniherrera good catch! Added this to the answer, thank you! – neverwalkaloner May 13 '18 at 18:30