hoping for a quick code review of a few simple class-based generic views

[Howard Edson]

I am working on a simple "to do list" app in django (two models: List and Item). Trying to learn and make use of class-based generic views. I have the following three display views working, but am requesting a quick code review to see if there's anything I can do to improve my usage/understanding of django's ListView and DetailView before I move on to creating, updating and deleting. Thanks in advance for any advice.

# class-based generic views for my lists, a list's items, and item detail...

class MyListsView(ListView):

    """Display the current user's Lists"""

    template_name = 'cmv_app/my_lists.html'

    context_object_name = 'my_lists'

    @method_decorator(login_required)

    def dispatch(self, *args, **kwargs):

        return super(MyListsView, self).dispatch(*args, **kwargs)

    def get_queryset(self):

        return List.objects.filter(user=self.request.user).order_by('-last_modified')

class ListItemsView(ListView):

    """Display a list's items"""

    template_name = 'cmv_app/list_items.html'

    context_object_name = 'list_items'

    @method_decorator(login_required)

    def dispatch(self, request, *args, **kwargs):

        self.list = get_object_or_404(List, pk=kwargs['list_id'])

        request = list_valid_for_user(request, self.list) # security check

        return super(ListItemsView, self).dispatch(request, *args, **kwargs)

    def get_queryset(self):

        return Item.objects.filter(list=self.list).order_by('-created_date')

    def get_context_data(self, **kwargs):

        context = super(ListItemsView, self).get_context_data(**kwargs)

        context['list'] = self.list

        return context

class ItemDetailView(DetailView):

    """Display detail for one list item"""

    template_name = 'cmv_app/item_detail.html'

    @method_decorator(login_required)

    def dispatch(self, request, *args, **kwargs):

        self.list = get_object_or_404(List, pk=kwargs['list_id'])

        self.item = get_object_or_404(Item, pk=kwargs['item_id'])

        request = list_valid_for_user(request, self.list)  # security check

        request = item_valid_for_list(request, self.item, self.list)  # security check

        return super(ItemDetailView, self).dispatch(request, *args, **kwargs)

    def get_object(self):

        return self.item

    def get_context_data(self, **kwargs):

        context = super(ItemDetailView, self).get_context_data(**kwargs)

        context['list'] = self.list

        return context

[Kelvin Wong]

I'd recommend removing the login_required checks from the views. Put them in the UrlConf instead:

from django.contrib.auth.decorators import login_required

from yourapp.views import MyListsView

urlpatterns = patterns('',

    url(r'^$', login_required(MyListsView.as_view()), name='yourapp_list'),

)

This will allow you to write simpler tests for your views.

http://tech.novapost.fr/django-unit-test-your-views-en.html#don-t-decorate-views-in-place

Also, if you restrict your queryset to display only objects for the current user then the generic view will be smart enough to return Http404 for requests for object pks that they do not own. Depending on your models, you might need to write a Manager for it.

class ItemDetailView(DetailView):

    model = ItemModel

    template_name = 'cmv_app/item_detail.html'

    def get_queryset(self):

        q = self.model.lists.for_user(self.request.user)

        return q

K

[Howard Edson]

Thank you - that helps!