Code

Opened 6 years ago

Closed 4 years ago

Last modified 3 years ago

#6735 closed (fixed)

Class-based generic views

Reported by: jkocherhans Owned by: david
Component: Generic views Version:
Severity: Keywords:
Cc: DaNmarner, floguy@…, david, carljm, jezdez, sciyoshi@…, vbmendes@…, anossov@…, mjmalone@…, taavi@…, narma.nsk@…, obeattie, darkpixel, Ciantic, brian@…, ssteinerX@…, drbob@…, ben@…, alexkoshelev, chrischambers, bkonkle, chromano Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by jkocherhans)

Rewrite generic views to be class based, but leave the existing generic views as-is for now.

Attachments (6)

new-generic-views.diff (11.8 KB) - added by jkocherhans 6 years ago.
new-generic-views.2.diff (26.9 KB) - added by jkocherhans 6 years ago.
new-generic-views.3.diff (12.4 KB) - added by jkocherhans 6 years ago.
generic-views.diff (21.6 KB) - added by jkocherhans 6 years ago.
Now list_detail views can play too.
generic-views.2.diff (21.3 KB) - added by jkocherhans 6 years ago.
DjangoGenericViews.png (89.9 KB) - added by telenieko 6 years ago.
class hierarchy

Download all attachments as: .zip

Change History (79)

Changed 6 years ago by jkocherhans

comment:1 Changed 6 years ago by jkocherhans

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

This ticket refs #3639 use newforms in generic create_update view

comment:2 Changed 6 years ago by jkocherhans

  • Needs documentation set
  • Needs tests set

The tests need to be fleshed out a little more, and we need a plan for backwards compatibility, but the classes themselves are fairly close, I think.

comment:3 Changed 6 years ago by floguy

  • Cc floguy@… added

comment:5 Changed 6 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

Changed 6 years ago by jkocherhans

Changed 6 years ago by jkocherhans

comment:6 Changed 6 years ago by jkocherhans

post_save is now on_success and I've added hook to return the rendered template.

comment:7 Changed 6 years ago by jkocherhans

  • Needs tests unset
  • Triage Stage changed from Design decision needed to Accepted

Sorry about the mess of patch files. I forgot to check the "replace me" box. Anyhow, Adrian and Jacob are both in favor after a discussion today, so I'm changing status to Accepted.

Changed 6 years ago by jkocherhans

Now list_detail views can play too.

comment:8 Changed 6 years ago by jkocherhans

  • Description modified (diff)
  • Summary changed from Class-based generic create/update/delete views to Class-based generic views

comment:9 Changed 6 years ago by jkocherhans

  • Description modified (diff)

Changed 6 years ago by jkocherhans

comment:10 Changed 6 years ago by daveyjoe

The EditView class is passing back a exception when I use it with a model that has a field with editable set to False (editable=False).
I'm only starting out with python so please ignore this if the problem is at my end.

Traceback is here... http://dpaste.com/41195/

comment:11 Changed 6 years ago by david

  • Cc larlet@… added

comment:12 Changed 6 years ago by bailey@…

This is great. it looks like one of the frequent patterns I'll be seeing is having to add some context vars a la extra_context. Presently I'm adding a render_response to the subclassed view and doing it there, might be nice to support a get_context_vars()

I think views as classes is a huge feature, might be nice to have the generic views inherit from a BaseView and BasePaginate view, etc. Personally the application I'm working on has a lot of pages with forms that filter the view somehow, so there will be a huge number of view subclasses.

comment:13 Changed 6 years ago by jezdez

  • Cc jannis@… added

comment:14 Changed 6 years ago by telenieko

  • milestone set to post-1.0
  • Owner changed from nobody to telenieko
  • Status changed from new to assigned

Is anybody working on the docs for this? If not I'll try to give it some time later.
(No need to reply "no", just reply if "yes" ;))

Changed 6 years ago by telenieko

class hierarchy

comment:15 Changed 6 years ago by telenieko

Attached a possible class hierarchy. Not that what is drawn as class members are what would be passed to call() as parameters (it was easier to draw it this way).

comment:16 Changed 6 years ago by telenieko

For anyone interested in this ticket (scheduled post-1.0) current code (in-progress) can be taken from my git branch, raw, non-colored diff here.

Current code should be usable (passes tests). And provides wrappers to the old views, so if you can; Try the patch without touching your urls and let me know what happens.

comment:17 Changed 6 years ago by benjixx

  • Cc benjixx added

comment:18 follow-up: Changed 6 years ago by telenieko

Question of the day, mostly for jacob and jkocherhans (it's DDN):

  • Would you agree to add to DetailView and ListView (see current diff) an extra option: "allow_serialization" (and maybe "serialization_format='json'") and then, in the call() do:
  if request.is_ajax() and self.allow_serialization:
    return serialized_stuff()

This is a pretty common pattern (if the request is AJAX return the object_list, or the object serializaed) and actually requires doing wrappers,
and with this ticket would mean people needing to do their own classes, so it would be nice to provide this (disabled by default).

It's generic! and simple, and disabled by default, but avoids needing wrappers/inheritance to get such a common thing ;) (Would also allow YUI, dojo, etc to provide better integration as there would be a standard way to get data for grids, etc from django).

comment:19 in reply to: ↑ 18 Changed 6 years ago by jkocherhans

Replying to telenieko:

Question of the day, mostly for jacob and jkocherhans (it's DDN):

  • Would you agree to add to DetailView and ListView (see current diff) an extra option: "allow_serialization" (and maybe "serialization_format='json'") and then, in the call() do:

This seems more in the realm of the various REST projects going on, so I'd be -0. People are going to want to control which fields are serialized, and that is definitely out of scope for DetailView and ListView.

comment:20 follow-up: Changed 6 years ago by R. Bailey <bailey@…>

is the definition for ObjectListView not committed to base.py? I don't know how to use git yet, perhaps I'm missing something.

comment:21 in reply to: ↑ 20 Changed 6 years ago by telenieko

Replying to R. Bailey <bailey@akamai.com>:

is the definition for ObjectListView not committed to base.py? I don't know how to use git yet, perhaps I'm missing something.

I assume you refer to the imports in django/views/generic/list_detail.py, that was a typo I renamed ObjectListView and ObjectDetailView to ListView and DetailView to make them shorter.

Thanks for spotting the typo, that means there are more tests needed ;)

comment:22 Changed 6 years ago by R. Bailey <bailey@…>

good deal, thanks. I'll sync up.

comment:23 follow-up: Changed 6 years ago by telenieko

With 1.0 on the streen it's time to start with the killer features for 1.1 (yes I intend this one to go for 1.1 ehehe).

Question one: Would you love a ListEditView() which used FormSets to edit/create multiple objects? (Just like an EditView but for more than one form).

Question mostly for Jacob:
The documentation for the class based generic views (see current docs diff) has to document all the methods available for inheritance which makes the document itself a bit cumbersome (lots of details that people wanting to "just use" those views are not interested in). So, would it be reasonable to do a topics/generic-views.txt with the introduction and the usage details; And then put on ref/generic-views.txt the implementation details and how can one inherit/develop generic views on top of the new ones?

comment:24 Changed 6 years ago by david

That's my main goal for 1.1 too. I hardly can find time for now but I keep an eye on it. Thanks for taking this.

comment:25 Changed 6 years ago by carljm

  • Cc carl@… added

comment:26 Changed 6 years ago by jezdez

  • Cc jezdez added; jannis@… removed

comment:27 in reply to: ↑ 23 Changed 6 years ago by telenieko

For the record, from IRC conversation:

Replying to telenieko:

Would you love a ListEditView() which used FormSets to edit/create multiple objects? (Just like an EditView but for more than one form).


That would be another ticket after this one gets in.

Would it be reasonable to do a topics/generic-views.txt with the introduction and the usage details; And then put on ref/generic-views.txt the implementation details and how can one inherit/develop generic views on top of the new ones?

Jacob is planning to do so for current generic views, so I'll do so for those ones.

comment:28 Changed 6 years ago by anonymous

  • Cc kyle.fox@… added

comment:29 Changed 6 years ago by sciyoshi

  • Cc sciyoshi@… added

comment:30 Changed 5 years ago by carljm

  • Cc carljm added; carl@… removed

comment:31 Changed 5 years ago by vbmendes

  • Cc vbmendes@… added

comment:32 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:33 Changed 5 years ago by jacob

  • milestone set to 1.1 beta

comment:34 Changed 5 years ago by Anossov

  • Cc anossov@… added

comment:35 Changed 5 years ago by david

  • Cc david added; larlet@… removed

comment:36 Changed 5 years ago by jacob

  • milestone changed from 1.1 beta to 1.2

Looks like this won't make it; I didn't get it done.

comment:37 follow-up: Changed 5 years ago by mmalone

  • Cc mjmalone@… added

I think documenting the use of class-based views and providing a simple implementation would be a great addition to Django, but I don't think this is the right approach.

I've seen two methods of implementing class-based views, __call__ based and __init__ based. The attached patch uses the __call__ approach. Here's a quick example of an __init__ based class-view:

from django.http import HttpResponse

class BaseView(HttpRespones):
    def __init__(self, request, *args, **kwargs):
        content = self.get(*args, **kwargs)
        super(MyView, self).__init__(content)

    def get(self):
        return 'Hello, world!'

The big gain here is that Django will instantiate a view instances per request, so you can actually store state in view objects. The __call__ based approach can be misleading/dangerous/confusing in this respect since the view instance is a global object that will be retained between requests. Even if you reset the state of the object at the beginning of each request, it's process local so it isn't thread safe.

The only thing the __init__ approach complicates is passing default arguments in during instantiation (e.g., model and post_save_redirect in the attached patch), but the url() function already solves that problem for function-based views by providing the kwargs argument, which works equally well here.

Unless anyone can think of some benefit to using the __call__ approach, I'd suggest we use __init__ instead.

comment:38 in reply to: ↑ 37 Changed 5 years ago by telenieko

Replying to mmalone:

I've seen two methods of implementing class-based views, __call__ based and __init__ based. The attached patch uses the __call__ approach. Here's a quick example of an __init__ based class-view:

Hi mmalone,
This is getting a major re-thing/re-write by Jacob, taking an approach much more like Model classes, if I remember correctly, the thing was something like:

from django.views.generic import ListView

class MyListView(ListView): 
    # Base view for views that list objects
    queryset = MyModel.objects.all() 
    paginate_by = 30    
    # You may override some methods here.

Then you'd pass the MyListView class to the URL patterns. init() would get URL parameters, while view configuration (what the current code here passes to init() would be on the class definition.

It's much more like Model classes, the only drawback is that you need to define you view class even for really simple stuff. But complex things get much nicer ;)

Hope Jacob can push current code sometime soon for everyone to see.

Thanks for commenting!

comment:39 follow-up: Changed 5 years ago by Alex

Jacob's code is on github account.

comment:40 follow-up: Changed 5 years ago by mmalone

Anyone have a link to Jacob's code on github? I checked his projects but couldn't find it easily.

telenieko: re simple things becoming harder, you'll still be able to make function-based views so that's not really true!

comment:42 in reply to: ↑ 39 Changed 5 years ago by telenieko

Replying to Alex:

Jacob's code is on github account.

Thanks! I should do "git remote update" more often :)

comment:43 in reply to: ↑ 40 ; follow-up: Changed 5 years ago by telenieko

Replying to mmalone:

telenieko: re simple things becoming harder, you'll still be able to make function-based views so that's not really true!

I think those will be deprecated in favour of the class based ones.

comment:44 in reply to: ↑ 43 Changed 5 years ago by mmalone

Replying to telenieko:

I think those will be deprecated in favour of the class based ones.

That's kind of dumb. Django has always supported both, why explicitly "deprecate" one method when they're both well supported.

Also, I just took a look at Jacob's code and it looks like he's using the __call__ based approach, so my comments from above still apply. If you tell someone to write a class they're going to want to store state in instances, and I think that's going to be problematic.

comment:45 Changed 5 years ago by Alex

Mike, he means the function generic views will be deprecated, not views in general.

comment:46 Changed 5 years ago by mmalone

Ah, ok. That makes more sense.

comment:47 Changed 5 years ago by anonymous

  • Cc taavi@… added

comment:48 Changed 5 years ago by narma

  • Cc narma.nsk@… added

comment:49 Changed 5 years ago by Nate

I've just been poking through the branch on GitHub and it looks great, but just in the name of API consistency it looks like the API for the class based views does not match the API for Syndication Feeds.

Namely, in a feed, if you want to override (for example) 'title' you can either provide a method or a string and the feed will handle either value intelligently.

In the class based views (so far), if you want to override (for example) 'items' you can either set 'items' or you can override the method get_items. This isn't consistent.

I'm not sure if this is an issue but I thought I'd point it out.

comment:50 Changed 5 years ago by obeattie

  • Cc obeattie added

comment:51 Changed 5 years ago by kylefox

  • Cc kyle.fox@… removed

comment:52 Changed 5 years ago by darkpixel

  • Cc darkpixel added

comment:53 Changed 4 years ago by Ciantic

  • Cc Ciantic added

comment:54 Changed 4 years ago by unbracketed

  • Cc brian@… added

comment:55 Changed 4 years ago by russellm

  • milestone 1.2 deleted
  • Version SVN deleted

comment:56 Changed 4 years ago by ssteiner

  • Cc ssteinerX@… added

comment:57 Changed 4 years ago by anonymous

  • Cc drbob@… added

comment:58 Changed 4 years ago by bfirsh

  • Cc ben@… added

comment:59 Changed 4 years ago by david

  • Owner changed from telenieko to david
  • Status changed from assigned to new

Stealing that one for the djangocon.eu's sprint.

comment:60 Changed 4 years ago by david

  • Status changed from new to assigned

Our current work in progress is at http://github.com/bfirsh/django-class-based-views (forked from Jacob's one).

comment:61 Changed 4 years ago by russellm

#13753 describes a reasonable feature request for generic views that should be integrated into this work, if at all possible.

comment:62 Changed 4 years ago by alexkoshelev

  • Cc alexkoshelev added

comment:63 Changed 4 years ago by chrischambers

  • Cc magma.chambers@… added

comment:64 Changed 4 years ago by chrischambers

  • Cc chrischambers added; magma.chambers@… removed

comment:65 Changed 4 years ago by bkonkle

  • Cc bkonkle added

comment:66 Changed 4 years ago by DaNmarner

  • Cc DaNmarner added

comment:67 Changed 4 years ago by chromano

  • Cc chromano added

comment:68 Changed 4 years ago by bfirsh

  • milestone set to 1.3

Wiki page with the current status of this ticket: http://code.djangoproject.com/wiki/ClassBasedViews

comment:69 Changed 4 years ago by mk

XViewMiddleware should be fixed if class-based views are added to Django: #13842

comment:70 Changed 4 years ago by anonymous

  • Cc benjixx removed

comment:72 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [14254]) Fixed #6735 -- Added class-based views.

This patch is the result of the work of many people, over many years.
To try and thank individuals would inevitably lead to many people
being left out or forgotten -- so rather than try to give a list that
will inevitably be incomplete, I'd like to thank *everybody* who
contributed in any way, big or small, with coding, testing, feedback
and/or documentation over the multi-year process of getting this into
trunk.

comment:73 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.