Hi Andy and Iván, On May 28, 2012, at 12:38 PM, Iván Raskovsky wrote:
> Hi Andy, > > On Mon, May 28, 2012 at 7:30 AM, Andrew Ingram <a...@andrewingram.net> wrote: >> Hi Iván, >> >> I'd temporarily given up on getting extra_views into Django because of >> a blocking issue with pagination and formsets. >> >> Assuming a goal is to be able to build a new admin using class-based >> views, there is a prerequisite of being able to paginate, sort and >> filter on the querysets used for modelformsets. The problem is that >> with Django's class-based views as they are, pagination is handled at >> the template context stage of the logic, whereas the formsets are >> instantiated and validated much earlier. I had raised a ticket about >> fixing this, but it was closed as wontfix because it would almost >> certainly introduce backwards-incompatible changes > > I'm not sure how to work around the issue, I didn't spend much time on > it. But for sure we can either find a BC way to solve this or revisit > that wontfix. > >> On a related now, you mentioned that none of the libraries you've >> mentioned are sufficiently polished. What further work do they need? >> As I see it, my main omission right now is documentation, though that >> is in progress. > > I agree your implementation is very well done (nicer than mine), but > from a quick inspection I could find: > > * There are almost no docstrings > * One can abstract part of the code in BaseFormSetMixin and FormMixin > into a new Mixin > * get_initial() could get a .copy() as in > https://github.com/django/django/blob/master/django/views/generic/edit.py#L18 > * I don't agree with the get_success_url implementation (it defers > from the current ones) > * There's a get_context_data() in FormSetMixin that I think shouldn't live > there > * I think you are not taking into consideration formsets prefixes clashing > * I'm not sold on BaseInlineFormSetMixin inheriting from > BaseFormSetMixin: do you really need e.g. get_context_data in there if > you won't use it? > > This is just a quick list I made while skimming through the code and > what I remember from past months. I'm more than glad to be proven > wrong! I'm sure you can make a similar list quickly with my > implementation, I think the best thing we can do is work together, > this is a big ticket. Get the best of each implementation and merge it > together. It's time to get the ball rolling. > > Regards, > Iván Backwards-compatibility should be a primary goal, however it is also ok to consider breaking that compatibility if it becomes necessary in order to have a clean and more easily maintainable codebase, as long as the upgrade path remains simple. This is a significant piece of refactoring and it's hard to predict the full extent of the impact it will have on legacy projects and third-party apps. If you guys are keen to continue working on this then I encourage you to pursue. I'm also glad to help where I can. For a start, I've made a first pass at rewriting the admin views into CBVs in ticket #17208 (https://code.djangoproject.com/ticket/17208). Once good progress has been made, then it would also be very helpful to keep the maintainers of all major custom admin apps (Armstrong, FeinCMS, Django-CMS, Grapelli, Mezzanine, etc.) in the loop for feedback. Thanks a lot for your great work! Julien
smime.p7s
Description: S/MIME cryptographic signature
