django-mptt 0.4 release soon, admin issues

[Craig de Stigter]

Hi guys

I'd like to release 0.4 ASAP, since master is currently backwards incompatible with 0.3.1 due to the abstract models refactor.

However first I would like to have some basic tree-like changelist view for the admin, preferably not depending on feincms or similar.

I don't think we should impose lots of fancy drag-drop javascript stuff at this stage, because it will take lots of time and testing and it's questionable whether that really belongs in django-mptt anyway. We can consider it for 0.5 maybe.

For 0.4, the solution should:

 - force changelist ordering by (tree_id, left) always.

 - indent (or nest? if possible) list items.

In light of those pretty simple requirements I've created this branch

Feedback please? It seems a bit hax to me - the django admin site is pretty confusing :s What can we improve?

Assuming we can nail this one, I'll tag and release 0.4 in a few days.

Craig de Stigter

[Matjaž Črnko]

Hello,

I think that the optimal solution is not to output the whole tree, as this can take too much time on bigger trees. I'm kinda biased, but I think a JS implementation (either jsTree as used in django-cms or the custom hacked feincms tree view) would be a good starting point, but probably not managable in the proposed timeline. So I would suggest you merge the branch and ship that for now, then a bit better approach with JS will be needed in my opinion. As I'm already working on forward porting the django-cms's jsTree fork, I'm ofcourse willing to share the work or/and help you with a full blown JS based tree admin integration.
The implementation doesen't look like a hack to me, but I didn't dive into it.

Again, thanks for your work,
Črnko

2010/9/25 Craig de Stigter <crai...@gmail.com>

[Craig de Stigter]

Thanks Črnko

I'm a bit loathe to make django-mptt dependent on a CMS package, especially since there are already multiple CMSs using django-mptt, and making them depend on a competitor seems unfair.

If you can take a smallish section of code from django-cms and put it in django-mptt, that would be great. It should be tree stuff only though, not containing any CMS-specific functionality, (i.e. no 'Include in site navigation?' switches etc.)

 

I think that the optimal solution is not to output the whole tree, as this can take too much time on bigger trees.

 

Not too sure what you mean here - all I'm doing in better-admin is forcing the queryset to be sorted, and indenting each node that the paginator would output anyway...

There are a couple of things I still want to look at before I merge and release 0.4:

 - the columns in the admin changelist appear to be sortable, but clicking them does nothing. Need to remove the sortable-ness.

 - the parent dropdown in add/change views should be a TreeNodeChoiceField. (Not sure if this is fixable, caller might need to specify it when they create their parent ForeignKey)

Regards

Craig de Stigter

2010/9/26 Matjaž Črnko <matjaz...@gmail.com>

[Matjaž Črnko]

Hello,

you misunderstood me, I wouldn't like a dependency on a CMS package - but their work can be a base for the default tree functionality that everyone could swap out if needed.
Yes that is what I was thinking, taking out a small portion of the tree functionality and pair it with list_display/list_filter configuration directives to display the appropriate columns.

Regarding the implementation, oh I didn't saw the paginator object, sorry for that.

2010/9/26 Craig de Stigter <crai...@gmail.com>

[Matthias Kestenholz]

Sorry for chiming in late. I think it's a pity to reinvent the wheel
and develop _another_ competing and incompatible tree editing
infrastructure. I know editing isn't possible (yet) but I fear
duplication of effort and code here.

I think it would be better to rethink this approach. I have a hard
time offering other possibilites because I don't want to advocate my
own code, I'll leave that to others.

Thanks
Matthias

[Craig de Stigter]

Matthias

Valid concerns. I agree that reinventing the wheel is a bad idea.

My reasoning behind the MPTTModelAdmin I've put in django-mptt is:

 - Not having any displayed hierarchy at all makes the admin completely useless for tree models.

 - As usual this requires the caller to opt-in by calling site.register(MyModel, MPTTModelAdmin), so we're not forcing anything on anyone.

 - We can replace this in 0.5 with a different implementation which uses a ModelAdmin imported from a CMS, if we decide to go down that route.

It wasn't my intention to add editing to the current approach. It's intended to be a bare-bones thing, if people want more then they can use a different ModelAdmin supplied by their CMS of choice.

Does that clear up your concerns?

Craig

[Craig de Stigter]

Matthias

On second thoughts, I just had another look at your patch for using FeinCMS if installed (on http://code.google.com/p/django-mptt/issues/detail?id=72 )

It looks pretty good actually. Perhaps we could use that, but define a FeinCMSModelAdmin instead of changing MPTTModelAdmin? That gives the caller the option of which admin class to use.

If FeinCMS isn't installed, at least we'll still have tree view changelists.

Črnko, would you be on board with that?

Craig

[Matthias Kestenholz]

On Tue, Sep 28, 2010 at 2:21 AM, Craig de Stigter <crai...@gmail.com> wrote:
> Matthias

> On second thoughts, I just had another look at your patch for using FeinCMS
> if installed (on http://code.google.com/p/django-mptt/issues/detail?id=72 )
> It looks pretty good actually. Perhaps we could use that, but define a
> FeinCMSModelAdmin instead of changing MPTTModelAdmin? That gives the caller
> the option of which admin class to use.
> If FeinCMS isn't installed, at least we'll still have tree view changelists.

Yes, I'm fine with that.

Btw, that isn't my patch :-) I'm not exactly sure why the
SafeMPTTAdminForm is required. The _actions_column changes aren't
applicable to any mptt model either. But the general direction of the
patch looks good.

Thanks for your efforts here!
Matthias

[Craig de Stigter]

Thanks Matthias

Not sure why I thought that was your patch...

It turns out the form just causes a validation error if you try to move a node to be a child of one of it's descendants. Without it we just throw an InvalidMove, so I left the form in.

I *think* 0.4 is pretty much ready. Just need to add some docs around this admin stuff. If you guys get a chance to test some stuff out, it'd be appreciated :)

Craig

[Craig de Stigter]

0.4 is good to go. I added some last minute changes to fix multi-table inheritance, thanks to Kyle Dodson for the patch.

I'll be tagging the release tomorrow. if you guys can try out the current master and report any bugs or confusing bits in docs that'd be appreciated.

Craig