silverstripe / silverstripe-cms

Silverstripe CMS - this is a module for Silverstripe Framework rather than a standalone app. Use https://github.com/silverstripe/silverstripe-installer/ to set this up.
http://silverstripe.org/
BSD 3-Clause "New" or "Revised" License
515 stars 332 forks source link

Delete parent page, child pages cannot be found anymore #1049

Closed stojg closed 10 years ago

stojg commented 10 years ago

How to recreate this bug:

First ensure that you have enough pages in the site tree so it doesn't pre-load all of your level two pages.

Observe that in the minimised site-tree shows A has a “Deleted” label, but no label on B

BUG: Observe that A and B is still showing (bug?)

Observe that A is showing, but not B

BUG: Observe that “A” doesn’t show up anymore

BUG: A shows up, but not “B” B cannot be found and restored any more?

tractorcow commented 10 years ago

https://github.com/silverstripe/silverstripe-cms/commit/3204ab5af35796ef5aa28331b821bea1549a4c2d#commitcomment-7007242

Perhaps this is an issue with the isOrphaned check? CMS users should still be able to view orphaned pages though, so maybe it's just a permission tweak.

stojg commented 10 years ago

No, the parent page is still there, but everything is for sure everything is connected in versioned / hierarchy in someway.. so who knows.

It might be related to Hierarchy::numChildren() that returns 0 number of childrens for a page with deleted children... might..

stojg commented 10 years ago

Yeah, I found the problem, will make a PR for this. As a side note, we need more automatic test that actually uses the hierarchy.

stojg commented 10 years ago

This seems to be an issue when the framework is using the default numChildren() when the code should use numHistoricalChildren()

stojg commented 10 years ago

Work in progress:

stojg commented 10 years ago

To help reproduce this issue I have created a SS 3.1 MySQL database dump that can be downloaded from https://stojg.se/assets/SS_sandboxdev.sql.gz (warning, it uses delete tables before import).

The live published site tree look like this:

Normal sitetree

Observe section 08 in the above image

If I in the 3.1 branch filters for all "Deleted pages" it looks like this:

Deleted Pages

As it can be seen, it's not possible to open Section 08 because it calculates the wrong number of children.

In the patches provided the same view looks like this:

Delete Pages - Patch

Section 08 can now be opened and the deleted child page can be restored.

stojg commented 10 years ago

I have tested Groups, Folders and fields like TreeDropDown field.

stojg commented 10 years ago

Even if not all things marked as BUG in the description, the most important one was fixed (BUG: A shows up, but not “B”).

stojg commented 10 years ago

thanks @tractorcow for doing the peer review and merging.