View collections in the generic views

[Harro]

A class that handles multiple class based views and also handles the urls for it.

A base class that just handles the urls for the views.
And for instance a Crud collection does basic crud

"""
Note that REST framework has an existing implementation of view collections...
​​http://django-rest-framework.org/tutorial/6-viewsets-and-routers.html
​​http://django-rest-framework.org/api-guide/viewsets.html
​​http://django-rest-framework.org/api-guide/routers.html
If Django works towards a view collection implementation, probably worth reviewing that approach too.

"""

[Harro]

For good measure, to make this totally awesome #11559 would need to be fixed too.

[Jannis Leidel]

I love it!

[Luke Plant]

In the patch, there are various instances of things like "if not self.templates" when what is meant is "if self.templates is not None".

With the queryset attribute, things are more complicated, because you almost certainly don't want to be evaluating a queryset that is stored on a class at any point.

[anonymous]

Right.. I'll take that into account

[Harro]

Bah, wasn't logged in.

[Harro]

Added some changes, added a mixin for the crud views to add extra functionality.

[Harro]

Some cleanup

[Bas Peschier]

Awesome! but I have a couple of small remarks:

• Comments in the code are either really short or just "thank you captain obvious"-type, for example the constructor-comments. Remove the unnecessary ones and add some useful docstrings to the main classes to give the reader a good idea what their purpose is.
• PEP8 nit-picking: Crud "should be" CRUD in the class names to emphasise the acronym.

[Harro]

Right you are, didn't focus much on docs(trings) and tests yet, just on getting the skeleton api right. Installed the pep8 checker in sublime after the last changes so that should be fixed next time too.

[Jacob]

Milestone 1.4 deleted

[Amirouche]

It looks like ​AdminSite class

• «ViewCollection» should be outside views.generic , it is not a view and not a generic view, but I don't know where.
• The name is misleading, based on your description and the code it's more a «ClassBasedViewCollection», but I still believe that «ViewCollection» is a good idea, it should support any callable.
• Based on the example CRUDViewsCollection, «ViewCollection.current_app» class property is not relevant, it should be configurable per instance (init argument) moreover «current_app» variable nae is used only in reverse which I think is only used to override urlresolver namespace strategy so better use application_namespace/instance_namespace (IMO prefered) or respectively app_name/namespace the keyword arguments of include.
• I don't like the following code in constructor:

  for key, value in kwargs.iteritems():
      setattr(self, key, value)
  

• get_urls
  • patterns can take a list of urls
  • The code enforce django CBV type of class, this makes other callables impossible to register
• urls property
  • This is not a real property, you can not set it
  • It only returns urls, instance_namespace where the include also accepts another argument application_namespace
  • This is not a set of urls or urlpatterns, I better see it be url_include or the like and change the return to django.conf.urls.include(pattern_list, self.instance_namespace, self.application_namespace)
• get_urls_namespacestring I don't see why this is useful, the view should compute its namespace, the naming is not good and Python avoids «get_»

Related tickets:

• #16213
• #17208

[Tom Christie]

Note that REST framework has an existing implementation of view collections...

​http://django-rest-framework.org/tutorial/6-viewsets-and-routers.html
​http://django-rest-framework.org/api-guide/viewsets.html
​http://django-rest-framework.org/api-guide/routers.html

If Django works towards a view collection implementation, probably worth reviewing that approach too.

[Asif Saifuddin Auvi]

@tomchristie you idea is nice! i would love to have that on django proper!

[Tom Christie]

I'm going to suggest that this isn't realistically in-scope for being worked on at this point.
It could be picked up in core at some point, but doesn't feel likely right now.

[Asif Saifuddin Auvi]

Replying to tomchristie:

I'm going to suggest that this isn't realistically in-scope for being worked on at this point.
It could be picked up in core at some point, but doesn't feel likely right now.

ok thank your for your views