Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Include children pages" when copying to subsite does not work #384

Closed
gavynj opened this issue Aug 20, 2018 · 2 comments
Closed

"Include children pages" when copying to subsite does not work #384

gavynj opened this issue Aug 20, 2018 · 2 comments

Comments

@gavynj
Copy link

gavynj commented Aug 20, 2018

It seems that the "Include children pages" option when copying a page to a subsite does not do anything.

Within the copytosubsite function in LeftAndMainSubsites.php there is this code:

$newPage = $page->duplicateToSubsite($subsite->ID, $includeChildren);

However, the duplicateToSubsite function within SiteTreeSubsites.php only accepts one argument:

public function duplicateToSubsite($subsiteID = null)

It looks like the ability to copy children pages has vanished? I've tested this feature and it definitely only copies the parent page across.

@robbieaverill
Copy link
Contributor

Thanks for raising the issue @gavynj

Yes, it looks like possibly this happened during some refactoring. LeftAndMainSubsites::copytosubsites looks for the CopyToSubsiteWithChildren key being sent in the data and passes it to duplicateToSubsite method(in SiteTreeSubsites). If you look at that method you'll notice that it doesn't have that second parameter, but calls duplicateSubsiteRelations which does its own internal check based on configuration values (rather than posted field data).

The method signature in subsites 1.4.0 (SilverStripe 3) does accept this value:

https://github.com/silverstripe/silverstripe-subsites/blob/1.4.0/code/extensions/SiteTreeSubsites.php#L221

This happened due to a combination of these commits: 1975861 3587bc6 in various merge ups between branches.

Since CopyToSubsiteWithChildren is a checkbox that gets added by SiteTreeSubsites, the fact that it's no longer honouring the value of this checkbox is a valid bug.

The configuration property inspected by SiteTreeSubsites is documented anywhere at all, but performs some custom logic for duplicating relations (note that we should be using cascade_duplicates for this now, so we could deprecate this method).

I think the fix here is to add the missing bool $includeChildren property to the duplicateToSubsite and call ->duplicateWithChildren() rather than $duplicate. Doing this would require it to always write, since SiteTree::duplicateWithChildren doesn't let you pass false into duplicate.

The whole duplicateToSubsitePrep method should really be rewritten to be inside a SubsiteState::singleton()->withState() callback that sets the desired subsite ID then calls $this->owner->duplicate() or $this->owner->duplicateWithChildren() depending on the input $includeChildren.

Would you want to have a go at making a pull request for this?

@robbieaverill
Copy link
Contributor

Hopefully this is fixed now with #387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants