Skip to content

Conversation

digi604
Copy link
Contributor

@digi604 digi604 commented Sep 18, 2014

Switches the cms from MPTT (Modified Preorder Tree Traversal) to MP (Materialized Path).
This should vastly reduce the number of corrupt trees and make moving trees around way more performant.
Uses currently django-treebeard MP-Tree. An optimized MP-Tree implementation could be build with gaps between nodes.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.85%) when pulling 4c6cea6 on digi604:treebeard into 8210ebd on divio:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.59%) when pulling 3d2e0b8 on digi604:treebeard into dd501b7 on divio:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.58%) when pulling 1fa9b18 on digi604:treebeard into dd501b7 on divio:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.58%) when pulling 1fa9b18 on digi604:treebeard into dd501b7 on divio:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.59%) when pulling 1fa9b18 on digi604:treebeard into dd501b7 on divio:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.55%) when pulling 086e3b2 on digi604:treebeard into 75d88bd on divio:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.42%) when pulling 73ed0de on digi604:treebeard into 75d88bd on divio:develop.

@digi604 digi604 added this to the 3.1 milestone Oct 1, 2014
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.41%) when pulling 9a51ec0 on digi604:treebeard into 75d88bd on divio:develop.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.27%) when pulling 948728d on digi604:treebeard into 75d88bd on divio:develop.

@digi604
Copy link
Contributor Author

digi604 commented Oct 2, 2014

@yakky the copying pages was broken before... but fixed it now... the sub plugin adding was fixed as well and found some other bugs. Please re-review :)

@digi604
Copy link
Contributor Author

digi604 commented Oct 8, 2014

@mkoistinen please review

@yakky
Copy link
Member

yakky commented Oct 8, 2014

LGTM

@yakky
Copy link
Member

yakky commented Oct 11, 2014

@mkoistinen can you have a look at this?

@mkoistinen
Copy link
Contributor

OK, I've had a chance to install this and run it through the paces. I created a fresh Django 1.7 install and used the repo @ digi604:treebeard. I was surprised when it wouldn't start with mptt in the INSTALLED_APPS, which of course meant it had to be pip installed too. I was concerned perhaps I didn't install the correct repo, but two things convinced me that I did:

  1. there was no longer a manage.py fix-mptt command, but instead there is a fix-tree command and;
  2. CMS tree operations are noticeably FASTER!

I created the deepest page tree I've ever made (12 levels?) and tried all sorts of re-arranging in hope of finding a way to break it, but was unable. I've also ensured my templates had {% show_breadcrumb %} and inspected its output, again looking for any issues to no avail.

I also ran the new fix-tree command after all this, but it didn't seem to complain about anything being out of whack, so, it was rather uneventful (good thing).

So at this point, I can say 👍 , and even: ❇️ AWESOME JOB PATRICK. ❇️

Now all we need to do is fix the pre-run checks that are looking for mptt and the documentation and I think this is certainly ready for 3.1b or whatever.

I'm guessing with this one change, we could even brand 3.1 a "stability" release, 'cause I think people are going to really notice an improvement here.

@yakky yakky mentioned this pull request Oct 15, 2014
@coveralls
Copy link

coveralls commented Oct 15, 2014

Coverage Status

Coverage decreased (-0.4%) to 86.899% when pulling a0b4ade on digi604:treebeard into 13b4cff on divio:develop.

yakky added a commit that referenced this pull request Oct 15, 2014
@yakky yakky merged commit c6b8b6c into django-cms:develop Oct 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants