alanjds / drf-nested-routers

Nested Routers for Django Rest Framework
https://pypi.org/project/drf-nested-routers/
Apache License 2.0
1.67k stars 159 forks source link

Nested resource does not work as expected. #7

Closed danielniccoli closed 10 years ago

danielniccoli commented 10 years ago

Maybe I am getting it wrong, but I have the following problem:

/customer/1/ is (as expected) returning:

{
    "id": 1, 
    "user": 1, 
    "favourite_restaurants": [
        1
    ], 
    "favourite_dishes": [
        1, 
        2
    ]
}

However /customer/1/favourite_dishes/ is unexpectedly returning every existing dish:

[
    {
        "id": 1, 
        "name": "Pizza Mozarella"
    }, 
    {
        "id": 2, 
        "name": "Pizza Funghi"
    }, 
    {
        "id": 3, 
        "name": "Spaghetti Carbonara"
    }, 
    {
        "id": 4, 
        "name": "Spaghetti con Pesto"
    }
]

instead of what I expect:

[
    {
        "id": 1, 
        "name": "Pizza Mozarella"
    }, 
    {
        "id": 2, 
        "name": "Pizza Funghi"
    }
]

So, did I misunderstand the provided functionality, or is this a bug?

This is my code: urls.py

# […]
from rest_framework_nested import routers
from app1.rest_views import CustomerViewSet, DishViewSet
router = routers.SimpleRouter()
router.register(r'customer', CustomerViewSet)
customer_router = routers.NestedSimpleRouter(router, r'customer', lookup='favourite_dish')
customer_router.register(r'favourite_dishes', DishViewSet)

urlpatterns = patterns(
    #url(r'^', include(router.urls)),
    #url(r'^api-auth/', include('rest_framework.urls', namespace='rest_framework')),

    url(r'^', include(router.urls)),
    url(r'^', include(customer_router.urls)),
)

models.py

# […]

class Restaurant(models.Model):
    name = models.TextField()

class Dish(models.Model):
    name = models.TextField()

class Customer(models.Model):
    user = models.OneToOneField(User)
    favourite_restaurants = models.ManyToManyField(Restaurant, blank=True)
    favourite_dishes = models.ManyToManyField(Dish, blank=True)
alanjds commented 10 years ago

@Borkason You misunderstood what it does. It just creates the new routes and passes the "parent" PK to the "children" view.

On views.py you probable have something like:

# views.py
class FavouriteDishViewSet(viewsets.ViewSet):
    def list(self, request):
        dishes = self.queryset.all()
        (...)
        return Response([...])

    def retrieve(self, request, pk=None):
        dishes = self.queryset.get(pk=pk)
        (...)
        return Response(serializer.data)

but you want something more like this:

# views.py
class FavouriteDishViewSet(viewsets.ViewSet):
    def list(self, request, customer_pk=None):
        dishes = self.queryset.filter(customer=customer_pk)
        (...)
        return Response([...])

    def retrieve(self, request, pk=None, customer_pk=None):
        dish = self.queryset.get(pk=pk, customer=customer_pk)
        (...)
        return Response(serializer.data)

In fact, I saw some good idea from @thedrow at https://github.com/tomchristie/django-rest-framework/issues/1149#issuecomment-25702826, but had no time try to automate this use-case.

Truth is that I use ViewSet mainly with non-ORM resources, so is kind of natural for me to manually filter the nested resource at def list and def retrieve, but I am open to suggestions :)

pozorvlak commented 10 years ago

You probably want to override get_queryset() instead, as described in the DRF docs under "Filtering". This handles update, retrieve and list for you, though I'm still having to manually override create to pass in the parent PK to the child constructor.

alanjds commented 10 years ago

@pozorvlak Hum... do you think this get_queryset() can be made automatically? Maybe drf-nested-routers could provide a mixin to ease this use case

pozorvlak commented 10 years ago

I think so. Here's the one I used, from the app I described in #18:

def get_queryset(self):
        order_id = self.kwargs['order_pk']
        return TrafficInstructions.objects.filter(order=order_id)
cancan101 commented 10 years ago

The filter in the get_queryset works for GETting the list and the detail view, but does not solve POSTing. Presumably when you add a new object, you will want to take the union of the fields the user POSTed and the fields from the URL.

alanjds commented 10 years ago

drf-nested-routers right now does not do effort to change the internal behaviours of the views, only the routers.

Right now, you should augment your create/update codes on the view level. Should drf-nested-routers provide mixins for the views?

cpbotha commented 10 years ago

Does anyone have a good example of overriding .create() in ModelViewSet for the nested model?

alanjds commented 10 years ago

README updated with minimal ViewSet example. No .create() example yet.