From dff3e8aaebe7af040f134e3fb19bf8da26a416cd Mon Sep 17 00:00:00 2001 From: Ryan Cramer Date: Mon, 1 Jul 2024 10:59:20 -0400 Subject: [PATCH] Refactor the Templates::getParentPage() method. This should hopefully fix processwire/processwire-issues#1929 ... also removed the Template::noShortcut check from that method since it didn't really belong there, and moved it to ProcessPageList and ProcessPageLister. Some phpdoc updates as well. --- wire/core/Template.php | 6 +- wire/core/Templates.php | 89 +++++++------------ .../ProcessPageAdd/ProcessPageAdd.module | 1 + .../ProcessPageLister.module | 3 + 4 files changed, 41 insertions(+), 58 deletions(-) diff --git a/wire/core/Template.php b/wire/core/Template.php index c519b179..2a43ad8e 100644 --- a/wire/core/Template.php +++ b/wire/core/Template.php @@ -1299,8 +1299,8 @@ class Template extends WireData implements Saveable, Exportable { * This is based on family settings, when applicable. * It also takes into account user access, if requested (see arg 1). * - * If there is no shortcut parent, NULL is returned. - * If there are multiple possible shortcut parents, a NullPage is returned. + * If there is no defined parent, NULL is returned. + * If there are multiple defined parents, a NullPage is returned. * * #pw-group-family * @@ -1313,7 +1313,7 @@ class Template extends WireData implements Saveable, Exportable { } /** - * Return all possible parent pages for this template + * Return all defined parent pages for this template * * #pw-group-family * diff --git a/wire/core/Templates.php b/wire/core/Templates.php index fcb90a9c..5e40aea8 100644 --- a/wire/core/Templates.php +++ b/wire/core/Templates.php @@ -5,7 +5,7 @@ * * Manages and provides access to all the Template instances * - * ProcessWire 3.x, Copyright 2022 by Ryan Cramer + * ProcessWire 3.x, Copyright 2024 by Ryan Cramer * https://processwire.com * * #pw-summary Manages and provides access to all the Templates. @@ -693,8 +693,8 @@ class Templates extends WireSaveableItems { * * - This is based on family settings, when applicable. * - It also takes into account user access, if requested (see arg 1). - * - If there is no shortcut parent, NULL is returned. - * - If there are multiple possible shortcut parents, a NullPage is returned. + * - If there is no defined parent, NULL is returned. + * - If there are multiple defined parents, a NullPage is returned (use $getAll to get them). * * @param Template $template * @param bool $checkAccess Whether or not to check for user access to do this (default=false). @@ -706,84 +706,63 @@ class Templates extends WireSaveableItems { public function getParentPage(Template $template, $checkAccess = false, $getAll = false) { $pages = $this->wire()->pages; - $user = $this->wire()->user; - $foundParent = null; - $foundParents = $getAll ? $pages->newPageArray() : null; - $foundParentQty = 0; + $foundParents = $pages->newPageArray(); $maxStatus = is_int($getAll) && $getAll ? ($getAll * 2) : 0; + $earlyExit = false; - if($template->noShortcut || !count($template->parentTemplates)) return $foundParents; if($template->noParents == -1) { // only 1 page of this type allowed - if($this->getNumPages($template) > 0) return $foundParents; + if($this->getNumPages($template) > 0) $earlyExit = true; } else if($template->noParents == 1) { - return $foundParents; + // no parents allowed + $earlyExit = true; + } else if(!count($template->parentTemplates)) { + // no parent templates defined + $earlyExit = true; } + + if($earlyExit) return $getAll ? $foundParents : null; + + $childTestPage = $checkAccess ? $pages->newPage($template) : null; foreach($template->parentTemplates as $parentTemplateID) { $parentTemplate = $this->get((int) $parentTemplateID); - if(!$parentTemplate) continue; + + // if parent template does not exist or not allow children, skip it + if(!$parentTemplate || $parentTemplate->noChildren) continue; - // if the parent template doesn't have this as an allowed child template, exclude it - if($parentTemplate->noChildren) continue; + // if the parent template doesn't have this as an allowed child template, skip it if(!in_array($template->id, $parentTemplate->childTemplates)) continue; // sort=status ensures that a non-hidden page is given preference to a hidden page $include = $checkAccess ? "unpublished" : "all"; $selector = "templates_id=$parentTemplate->id, include=$include, sort=status"; + if($maxStatus) { $selector .= ", status<$maxStatus"; - } else if(!$getAll) { + } else if(!$getAll && !$checkAccess) { $selector .= ", limit=2"; } - $parentPages = $pages->find($selector); - $numParentPages = count($parentPages); - - // undetermined parent - if(!$numParentPages) continue; - - if($getAll) { - // build list of all parents (will check access outside loop) - $foundParents->add($parentPages); - continue; - } else if($numParentPages > 1) { - // multiple possible parents, we can early-exit - $foundParentQty += $numParentPages; - break; - } else { - // one possible parent - $parentPage = $parentPages->first(); + + foreach($pages->find($selector) as $parentPage) { + if($checkAccess && !$parentPage->addable($childTestPage)) continue; + $foundParents->add($parentPage); + $earlyExit = !$getAll && $foundParents->count() > 1; + if($earlyExit) break; } - - if($checkAccess) { - if($parentPage->id) { - // single defined parent - $p = $pages->newPage($template); - if(!$parentPage->addable($p)) continue; - } else { - // multiple possible parents - if(!$user->hasPermission('page-create', $template)) continue; - } - } - - if($parentPage && $parentPage->id) $foundParentQty++; - $foundParent = $parentPage; - if($foundParentQty > 1) break; + + if($earlyExit) break; } - if($checkAccess && $getAll && $foundParents && $foundParents->count()) { - $p = $pages->newPage($template); - foreach($foundParents as $parentPage) { - if(!$parentPage->addable($p)) $foundParents->remove($parentPage); - } - } + if($getAll) return $foundParents; // always returns PageArray (populated or not) - if($getAll) return $foundParents; - if($foundParentQty > 1) return $pages->newNullPage(); + $qty = $foundParents->count(); + if($qty > 1) return $pages->newNullPage(); // multiple possible parents + if($qty === 1) return $foundParents->first(); // one possible parent - return $foundParent; + return null; // no parents } /** diff --git a/wire/modules/Process/ProcessPageAdd/ProcessPageAdd.module b/wire/modules/Process/ProcessPageAdd/ProcessPageAdd.module index ba53dd6e..6e4b9ac9 100644 --- a/wire/modules/Process/ProcessPageAdd/ProcessPageAdd.module +++ b/wire/modules/Process/ProcessPageAdd/ProcessPageAdd.module @@ -185,6 +185,7 @@ class ProcessPageAdd extends Process implements ConfigurableModule, WirePageEdit if(!$user->isGuest() && $user->hasPermission('page-edit')) { foreach($templates as $template) { + if($template->noShortcut) continue; $parent = $template->getParentPage(true); if(!$parent) continue; if($parent->id) { diff --git a/wire/modules/Process/ProcessPageLister/ProcessPageLister.module b/wire/modules/Process/ProcessPageLister/ProcessPageLister.module index a10ec019..0510df1e 100644 --- a/wire/modules/Process/ProcessPageLister/ProcessPageLister.module +++ b/wire/modules/Process/ProcessPageLister/ProcessPageLister.module @@ -2221,6 +2221,9 @@ class ProcessPageLister extends Process implements ConfigurableModule { if($this->parent && $this->parent->id && $this->parent->addable()) { $action = "?parent_id={$this->parent->id}"; + } else if($this->template && $this->template->noShortcut) { + // not allowed in add-new list + } else if($this->template && ($parent = $this->template->getParentPage(true))) { if($parent->id) { $action = "?parent_id=$parent->id"; // defined parent