Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #1551 from adaptivelogic/bugfix-reordering

Ensure that publishing a parent page does not reorder the child pages.
  • Loading branch information...
commit 6127148e3fd3f35d1f0969ea2333fd3fe981ea29 2 parents 06ede47 + f5b7f9a
@digi604 digi604 authored
View
253 cms/models/pagemodel.py
@@ -45,6 +45,7 @@ class Page(MPTTModel):
parent = models.ForeignKey('self', null=True, blank=True, related_name='children', db_index=True)
creation_date = models.DateTimeField(auto_now_add=True)
changed_date = models.DateTimeField(auto_now=True)
+
publication_date = models.DateTimeField(_("publication date"), null=True, blank=True, help_text=_('When the page should go live. Status must be "Published" for page to go live.'), db_index=True)
publication_end_date = models.DateTimeField(_("publication end date"), null=True, blank=True, help_text=_('When to expire the page. Leave empty to never expire.'), db_index=True)
in_navigation = models.BooleanField(_("in navigation"), default=True, db_index=True)
@@ -139,18 +140,18 @@ def move_page(self, target, position='first-child'):
def _copy_titles(self, target):
"""
- Copy all the titles to a new page.
+ Copy all the titles to a new page (which must have a pk).
:param target: The page where the new titles should be stored
"""
- # TODO: Make this into a "graceful" copy instead of deleting and overwriting
- target.title_set.all().delete()
- titles = list(self.title_set.all())
- for title in titles:
- title.pk = None # setting pk = None creates a new instance
- title.publisher_public_id = None
- title.published = False
+ old_titles = dict(target.title_set.values_list('language', 'pk'))
+ for title in self.title_set.all():
+ # If an old title exists, overwrite. Otherwise create new
+ title.pk = old_titles.pop(title.language, None)
title.page = target
title.save()
+ if old_titles:
+ from titlemodels import Title
+ Title.objects.filter(id__in=old_titles.values()).delete()
def _copy_contents(self, target):
"""
@@ -181,6 +182,8 @@ def _copy_attributes(self, target):
target.publication_date = self.publication_date
target.publication_end_date = self.publication_end_date
target.in_navigation = self.in_navigation
+ target.login_required = self.login_required
+ target.limit_visibility_in_menu = self.limit_visibility_in_menu
target.soft_root = self.soft_root
target.reverse_id = self.reverse_id
target.navigation_extenders = self.navigation_extenders
@@ -188,7 +191,7 @@ def _copy_attributes(self, target):
target.site_id = self.site_id
def copy_page(self, target, site, position='first-child',
- copy_permissions=True, public_copy=False):
+ copy_permissions=True):
"""
Copy a page [ and all its descendants to a new location ]
Doesn't checks for add page permissions anymore, this is done in PageAdmin.
@@ -204,27 +207,22 @@ def copy_page(self, target, site, position='first-child',
page_copy = None
- if public_copy:
- # create a copy of the draft page - existing code loops through pages so added it to a list
- pages = [copy.copy(self)]
- else:
- pages = [self] + list(self.get_descendants().order_by('-rght'))
+ pages = [self] + list(self.get_descendants().order_by('-rght'))
- if not public_copy:
- site_reverse_ids = Page.objects.filter(site=site, reverse_id__isnull=False).values_list('reverse_id', flat=True)
+ site_reverse_ids = Page.objects.filter(site=site, reverse_id__isnull=False).values_list('reverse_id', flat=True)
- if target:
- target.old_pk = -1
- if position == "first-child":
- tree = [target]
- elif target.parent_id:
- tree = [target.parent]
- else:
- tree = []
+ if target:
+ target.old_pk = -1
+ if position == "first-child":
+ tree = [target]
+ elif target.parent_id:
+ tree = [target.parent]
else:
tree = []
- if tree:
- tree[0].old_pk = tree[0].pk
+ else:
+ tree = []
+ if tree:
+ tree[0].old_pk = tree[0].pk
first = True
# loop over all affected pages (self is included in descendants)
@@ -243,47 +241,31 @@ def copy_page(self, target, site, position='first-child',
page.published = False
page.publisher_public_id = None
# only set reverse_id on standard copy
- if not public_copy:
- if page.reverse_id in site_reverse_ids:
- page.reverse_id = None
- if first:
- first = False
- if tree:
- page.parent = tree[0]
- else:
- page.parent = None
- page.insert_at(target, position)
+ if page.reverse_id in site_reverse_ids:
+ page.reverse_id = None
+ if first:
+ first = False
+ if tree:
+ page.parent = tree[0]
else:
- count = 1
- found = False
- for prnt in tree:
- if prnt.old_pk == page.parent_id:
- page.parent = prnt
- tree = tree[0:count]
- found = True
- break
- count += 1
- if not found:
- page.parent = None
- tree.append(page)
+ page.parent = None
+ page.insert_at(target, position)
+ else:
+ count = 1
+ found = False
+ for prnt in tree:
+ if prnt.old_pk == page.parent_id:
+ page.parent = prnt
+ tree = tree[0:count]
+ found = True
+ break
+ count += 1
+ if not found:
+ page.parent = None
+ tree.append(page)
page.site = site
- # override default page settings specific for public copy
- if public_copy:
- page.published = (not page.parent) or page.parent.published
- page.publisher_is_draft = False
- # we need to set relate this new public copy to its draft page (self)
- page.publisher_public = self
-
- # code taken from Publisher publish() overridden here as we need to save the page
- # before we are able to use the page object for titles, placeholders etc.. below
- # the method has been modified to return the object after saving the instance variable
-
- page = self._publisher_save_public(page)
- page_copy = page # create a copy used in the return
- else:
- # only need to save the page if it isn't public since it is saved above otherwise
- page.save()
+ page.save()
# copy permissions if necessary
if settings.CMS_PERMISSION and copy_permissions:
@@ -293,9 +275,7 @@ def copy_page(self, target, site, position='first-child',
permission.page = page
permission.save()
- # update moderation message for standard copy
- if not public_copy:
- update_moderation_message(page, unicode(_('Page was copied.')))
+ update_moderation_message(page, unicode(_('Page was copied.')))
# copy titles of this page
for title in titles:
@@ -303,10 +283,7 @@ def copy_page(self, target, site, position='first-child',
title.page = page
# create slug-copy for standard copy
- if not public_copy:
- title.save() # We need to save the title in order to
- # retrieve it in get_available_slug
- title.slug = page_utils.get_available_slug(title)
+ title.slug = page_utils.get_available_slug(title)
title.save()
# copy the placeholders (and plugins on those placeholders!)
@@ -395,24 +372,28 @@ def publish(self):
if not self.pk:
self.save()
if self._publisher_can_publish():
- ########################################################################
- # Assign the existing public page in old_public and mark it as
- # PUBLISHER_STATE_DELETE
- # the draft version was being deleted if I replaced the save()
- # below with a delete() directly so the deletion is handle at the end
- old_public = self.get_public_object()
- if old_public:
- old_public.publisher_state = self.PUBLISHER_STATE_DELETE
- # store old public on self, pass around instead
- self.old_public = old_public
- # remove the one-to-one references between public and draft
- old_public.publisher_public = None
- old_public.save()
-
- # we hook into the modified copy_page routing to do the heavy lifting of copying the draft page to a new public page
- new_public = self.copy_page(target=None, site=self.site,
- position=None,
- copy_permissions=False, public_copy=True)
+ if self.publisher_public_id:
+ # Ensure we have up to date mptt properties
+ public_page = Page.objects.get(pk=self.publisher_public_id)
+ else:
+ public_page = Page(created_by=self.created_by)
+
+ self._copy_attributes(public_page)
+ # we need to set relate this new public copy to its draft page (self)
+ public_page.publisher_public = self
+ public_page.publisher_is_draft = False
+
+ # Ensure that the page is in the right position and save it
+ public_page = self._publisher_save_public(public_page)
+ public_page.published = (public_page.parent_id is None or public_page.parent.published)
+ public_page.save()
+
+ # The target page now has a pk, so can be used as a target
+ self._copy_titles(public_page)
+ self._copy_contents(public_page)
+
+ # invalidate the menu for this site
+ menu_pool.clear(site_id=self.site_id)
# taken from Publisher - copy_page needs to call self._publisher_save_public(copy) for mptt insertion
# insert_at() was maybe calling _create_tree_space() method, in this
@@ -422,7 +403,7 @@ def publish(self):
me = self._default_manager.get(pk=self.pk)
self.tree_id = me.tree_id
- self.publisher_public = new_public
+ self.publisher_public = public_page
published = True
else:
# Nothing left to do
@@ -438,8 +419,10 @@ def publish(self):
self.save()
# If we are publishing, this page might have become a "home" which
# would change the path
- for title in self.title_set.all():
- title.save()
+ if self.is_home():
+ for title in self.title_set.all():
+ if title.path != '':
+ title.save()
# clean moderation log
self.pagemoderatorstate_set.all().delete()
@@ -448,30 +431,15 @@ def publish(self):
# was not published, escape
return
- # we delete the old public page - this only deletes the public page as we
- # have removed the old_public.publisher_public=None relationship to the draft page above
- if old_public:
- # reparent public child pages before delete so they don't get purged as well
- for child_page in old_public.children.order_by('lft'):
- child_page.move_to(new_public, 'last-child')
- child_page.save()
- # reload old_public to get correct tree attrs
- old_public = Page.objects.get(pk=old_public.pk)
- old_public.move_to(None, 'last-child')
- # moving the object out of the way berore deleting works, but why?
- # finally delete the old public page
- old_public.delete()
-
- for title in new_public.title_set.all():
- title.save()
# Check if there are some children which are waiting for parents to
# become published.
- publish_set = self.get_descendants().filter(published=True)
+ publish_set = self.get_descendants().filter(published=True).select_related('publisher_public')
for page in publish_set:
if page.publisher_public:
- if page.publisher_public.parent.published:
- page.publisher_public.published = True
- page.publisher_public.save()
+ if page.publisher_public.parent.published:
+ if not page.publisher_public.published:
+ page.publisher_public.published = True
+ page.publisher_public.save()
if page.publisher_state == Page.PUBLISHER_STATE_PENDING:
page.publisher_state = Page.PUBLISHER_STATE_DEFAULT
page._publisher_keep_state = True
@@ -1045,56 +1013,39 @@ def _publisher_save_public(self, obj):
obj - public variant of `self` to be saved.
"""
- prev_sibling = self.get_previous_filtered_sibling(publisher_public__isnull=False)
+ public_parent = self.parent.publisher_public if self.parent_id else None
+ filters = dict(publisher_public__isnull=False)
+ if public_parent:
+ filters['publisher_public__parent__in'] = [public_parent]
+ else:
+ filters['publisher_public__parent__isnull'] = True
+ prev_sibling = self.get_previous_filtered_sibling(**filters)
+ public_prev_sib = prev_sibling.publisher_public if prev_sibling else None
- if not self.publisher_public_id:
+ if not self.publisher_public_id: # first time published
# is there anybody on left side?
- if prev_sibling:
- obj.insert_at(prev_sibling.publisher_public, position='right', save=False)
+ if public_prev_sib:
+ obj.insert_at(public_prev_sib, position='right', save=False)
else:
- # it is a first time published object, perform insert_at:
- parent, public_parent = self.parent, None
- if parent:
- public_parent = parent.publisher_public
if public_parent:
- obj.insert_at(public_parent, save=False)
+ obj.insert_at(public_parent, position='first-child', save=False)
else:
# check if object was moved / structural tree change
- prev_public_sibling = self.old_public.get_previous_filtered_sibling()
-
- if not self.level == self.old_public.level or \
- not (self.level > 0 and self.parent.publisher_public == self.old_public.parent) or \
- not prev_sibling == prev_public_sibling == None or \
- (prev_sibling and prev_sibling.publisher_public_id == prev_public_sibling.id):
-
- if prev_sibling:
- obj.insert_at(prev_sibling.publisher_public, position="right")
- elif self.parent:
+ prev_public_sibling = obj.get_previous_filtered_sibling()
+ if self.level != obj.level or \
+ public_parent != obj.parent or \
+ public_prev_sib != prev_public_sibling:
+ if public_prev_sib:
+ obj.move_to(public_prev_sib, position="right")
+ elif public_parent:
# move as a first child to parent
- target = self.parent.publisher_public
- obj.insert_at(target, position='first-child')
+ obj.move_to(public_parent, position='first-child')
else:
# it is a move from the right side or just save
- next_sibling = self.get_next_filtered_sibling()
+ next_sibling = self.get_next_filtered_sibling(**filters)
if next_sibling and next_sibling.publisher_public_id:
- obj.insert_at(next_sibling.publisher_public, position="left")
- else:
- # insert at last public position
- prev_sibling = self.old_public.get_previous_filtered_sibling()
+ obj.move_to(next_sibling.publisher_public, position="left")
- if prev_sibling:
- obj.insert_at(prev_sibling, position="right")
- elif self.old_public.parent:
- # move as a first child to parent
- target = self.old_public.parent
- obj.insert_at(target, position='first-child')
- else:
- # it is a move from the right side or just save
- next_sibling = self.old_public.get_next_filtered_sibling()
- if next_sibling and next_sibling.publisher_public_id:
- obj.insert_at(next_sibling, position="left")
- # or none structural change, just save
- obj.save()
return obj
def rescan_placeholders(self):
View
12 cms/models/titlemodels.py
@@ -33,25 +33,25 @@ def __unicode__(self):
def save(self, *args, **kwargs):
# Update the path attribute before saving
- self.update_path()
+ # No longer needed since this is done in signals
+ #self.update_path()
super(Title, self).save(*args, **kwargs)
def update_path(self):
# Build path from parent page's path and slug
- current_path = self.path
- parent_page = self.page.parent
-
slug = u'%s' % self.slug
if not self.has_url_overwrite:
self.path = u'%s' % slug
- if parent_page:
+ if self.page.parent_id:
+ parent_page = self.page.parent_id
+
parent_title = Title.objects.get_title(parent_page, language=self.language, language_fallback=True)
if parent_title:
self.path = u'%s/%s' % (parent_title.path, slug)
@property
def overwrite_url(self):
- """Return overrwriten url, or None
+ """Return overwritten url, or None
"""
if self.has_url_overwrite:
return self.path
View
39 cms/signals.py
@@ -44,13 +44,13 @@ def update_title_paths(instance, **kwargs):
def update_title(title):
- parent_page_id = title.page.parent_id
slug = u'%s' % title.slug
if title.page.is_home():
title.path = ''
elif not title.has_url_overwrite:
title.path = u'%s' % slug
+ parent_page_id = title.page.parent_id
if parent_page_id:
parent_title = Title.objects.get_title(parent_page_id,
@@ -61,18 +61,16 @@ def update_title(title):
def pre_save_title(instance, raw, **kwargs):
"""Save old state to instance and setup path
"""
-
- menu_pool.clear(instance.page.site_id)
-
- instance.tmp_path = None
- instance.tmp_application_urls = None
+ if not instance.page.publisher_is_draft:
+ menu_pool.clear(instance.page.site_id)
if instance.id and not hasattr(instance, "tmp_path"):
+ instance.tmp_path = None
+ instance.tmp_application_urls = None
try:
- tmp_title = Title.objects.get(pk=instance.id)
- instance.tmp_path = tmp_title.path
- instance.tmp_application_urls = tmp_title.application_urls
- except:
+ instance.tmp_path, instance.tmp_application_urls = \
+ Title.objects.filter(pk=instance.id).values_list('path', 'application_urls')[0]
+ except IndexError:
pass # no Titles exist for this page yet
# Build path from parent page's path and slug
@@ -87,8 +85,8 @@ def pre_save_title(instance, raw, **kwargs):
def post_save_title(instance, raw, created, **kwargs):
# Update descendants only if path changed
application_changed = False
-
- if instance.path != getattr(instance,'tmp_path',None) and not hasattr(instance, 'tmp_prevent_descendant_update'):
+ prevent_descendants = hasattr(instance, 'tmp_prevent_descendant_update')
+ if instance.path != getattr(instance,'tmp_path',None) and not prevent_descendants:
descendant_titles = Title.objects.filter(
page__lft__gt=instance.page.lft,
page__rght__lt=instance.page.rght,
@@ -104,21 +102,18 @@ def post_save_title(instance, raw, created, **kwargs):
application_changed = True
descendant_title.save()
- if not hasattr(instance, 'tmp_prevent_descendant_update') and \
+ if not prevent_descendants and \
(instance.application_urls != getattr(instance, 'tmp_application_urls', None) or application_changed):
# fire it if we have some application linked to this page or some descendant
application_post_changed.send(sender=Title, instance=instance)
# remove temporary attributes
- if getattr( instance, 'tmp_path', None):
- del(instance.tmp_path)
- if getattr( instance, 'tmp_application_urls' , None):
- del(instance.tmp_application_urls)
-
- try:
- del(instance.tmp_prevent_descendant_update)
- except AttributeError:
- pass
+ if hasattr(instance, 'tmp_path'):
+ del instance.tmp_path
+ if hasattr(instance, 'tmp_application_urls'):
+ del instance.tmp_application_urls
+ if prevent_descendants:
+ del instance.tmp_prevent_descendant_update
signals.post_save.connect(post_save_title, sender=Title, dispatch_uid="cms.title.postsave")
View
40 cms/tests/publisher.py
@@ -226,6 +226,46 @@ def test_simple_publisher(self):
self.assertTrue(pageC.published)
self.assertEqual(len(Page.objects.public().published()), 3)
+ def test_publish_ordering(self):
+ page = self.create_page('parent', published=True)
+ pageA = self.create_page('pageA', parent=page, published=True)
+ pageC = self.create_page('pageC', parent=page, published=True)
+ pageB = self.create_page('pageB', parent=page, published=True)
+ pageB.move_page(pageA, 'right')
+ pageB.publish()
+ # pageC needs reload since B has swapped places with it
+ pageC.reload().publish()
+ pageA.publish()
+
+ drafts = Page.objects.drafts().order_by('tree_id', 'lft')
+ draft_titles = [(p.get_title('en'), p.lft, p.rght) for p in drafts]
+ self.assertEquals([('parent', 1, 8),
+ ('pageA', 2, 3),
+ ('pageB', 4, 5),
+ ('pageC', 6, 7)], draft_titles)
+ public = Page.objects.public().order_by('tree_id', 'lft')
+ public_titles = [(p.get_title('en'), p.lft, p.rght) for p in public]
+ self.assertEquals([('parent', 1, 8),
+ ('pageA', 2, 3),
+ ('pageB', 4, 5),
+ ('pageC', 6, 7)], public_titles)
+
+ page.publish()
+
+ drafts = Page.objects.drafts().order_by('tree_id', 'lft')
+ draft_titles = [(p.get_title('en'), p.lft, p.rght) for p in drafts]
+ self.assertEquals([('parent', 1, 8),
+ ('pageA', 2, 3),
+ ('pageB', 4, 5),
+ ('pageC', 6, 7)], draft_titles)
+ public = Page.objects.public().order_by('tree_id', 'lft')
+ public_titles = [(p.get_title('en'), p.lft, p.rght) for p in public]
+ self.assertEquals([('parent', 1, 8),
+ ('pageA', 2, 3),
+ ('pageB', 4, 5),
+ ('pageC', 6, 7)], public_titles)
+
+
def test_unpublish_unpublish(self):
name = self._testMethodName
page = self.create_page(name, published=True)
Please sign in to comment.
Something went wrong with that request. Please try again.