From 1d6f1cea27480d920854dd74040a6235d04b2f22 Mon Sep 17 00:00:00 2001 From: coder4life Date: Sun, 19 Mar 2017 13:11:55 -0700 Subject: [PATCH 1/5] Ancestor and inherited functions for Page & Pages Addes both helper methods to `Grav\Common\Page` and Grav instance utility functions to `Grav\Common\Pages`. --- system/src/Grav/Common/Page/Page.php | 41 +++++++++++++++++ system/src/Grav/Common/Page/Pages.php | 63 +++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) diff --git a/system/src/Grav/Common/Page/Page.php b/system/src/Grav/Common/Page/Page.php index c5adb80ef3..904bf7b412 100644 --- a/system/src/Grav/Common/Page/Page.php +++ b/system/src/Grav/Common/Page/Page.php @@ -2263,6 +2263,47 @@ public function root() } } + /** + * Helper method to return an ancestor page. + * + * @param string $url The url of the page + * @param bool $lookup Name of the parent folder + * + * @return \Grav\Common\Page\Page page you were looking for if it exists + */ + public function ancestor($url, $lookup = null) + { + /** @var Pages $pages */ + $pages = Grav::instance()['pages']; + + return $pages->ancestor($url, $lookup); + } + + /** + * Helper method to return an ancestor field to inherit from. The first + * occurrence of an ancestor field will be returned if at all. + * + * @param string $url The url of the page + * @param bool $field Name of the parent folder + * + * @return array + */ + public function inherited($url, $field) + { + /** @var Pages $pages */ + $pages = Grav::instance()['pages']; + + $inheritedParams = $pages->inherited($url, $field); + + $currentParams = (array) $this->value('header.' . $field); + + if($inheritedParams && is_array($inheritedParams)) { + $currentParams = array_replace_recursive($inheritedParams, $currentParams); + } + + return $currentParams; + } + /** * Helper method to return a page. * diff --git a/system/src/Grav/Common/Page/Pages.php b/system/src/Grav/Common/Page/Pages.php index 4e8614aec6..0bbfee079a 100644 --- a/system/src/Grav/Common/Page/Pages.php +++ b/system/src/Grav/Common/Page/Pages.php @@ -291,6 +291,69 @@ public function children($path) return new Collection($children, [], $this); } + /** + * Get a page ancestor. + * + * @param string $url The relative URL of the page + * @param string $path The relative path of the ancestor folder + * + * @return Page|null + */ + public function ancestor($url, $path = null) + { + if (!is_null($path)) { + + // Fetch page if there's a defined route to it. + $page = isset($this->routes[$url]) ? $this->get($this->routes[$url]) : null; + + // Try without trailing slash + if (!$page && Utils::endsWith($url, '/')) { + $page = isset($this->routes[rtrim($url, '/')]) ? $this->get($this->routes[rtrim($url, '/')]) : null; + } + + if ($page->path() == $path) { + return $page; + } elseif (!$page->parent()->root()) { + return $this->ancestor($page->parent()->url(), $path); + } + } + + return null; + } + + + /** + * Get a page ancestor trait. + * + * @param string $url The relative URL of the page + * @param string $field The field name of the ancestor to query for + * + * @return array|null + */ + public function inherited($url, $field = null) + { + if (!is_null($field)) { + + // Fetch page if there's a defined route to it. + $page = isset($this->routes[$url]) ? $this->get($this->routes[$url]) : null; + + // Try without trailing slash + if (!$page && Utils::endsWith($url, '/')) { + $page = isset($this->routes[rtrim($url, '/')]) ? $this->get($this->routes[rtrim($url, '/')]) : null; + } + + $ancestorField = $page->parent()->value('header.' . $field); + + if ($ancestorField != null) { + return $ancestorField; + } elseif (!$page->parent()->root()) { + return $this->inherited($page->parent()->url(), $field); + } + } + + return null; + } + /** * alias method to return find a page. * From de6aef9dfced6f84ec55b1321c23018030f9e185 Mon Sep 17 00:00:00 2001 From: coder4life Date: Sun, 19 Mar 2017 22:57:01 -0700 Subject: [PATCH 2/5] Ancestor Page return considering field criteria --- system/src/Grav/Common/Page/Page.php | 37 ++++++++++++++++++++++++--- system/src/Grav/Common/Page/Pages.php | 5 ++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/system/src/Grav/Common/Page/Page.php b/system/src/Grav/Common/Page/Page.php index 904bf7b412..74baae88d2 100644 --- a/system/src/Grav/Common/Page/Page.php +++ b/system/src/Grav/Common/Page/Page.php @@ -2280,21 +2280,50 @@ public function ancestor($url, $lookup = null) } /** - * Helper method to return an ancestor field to inherit from. The first - * occurrence of an ancestor field will be returned if at all. + * Helper method to return an ancestor page to inherit from. The current + * page object is returned. * * @param string $url The url of the page * @param bool $field Name of the parent folder * - * @return array + * @return Page */ public function inherited($url, $field) { /** @var Pages $pages */ $pages = Grav::instance()['pages']; - $inheritedParams = $pages->inherited($url, $field); + $inherited = $pages->inherited($url, $field); + + $inheritedParams = (array) $inherited->value('header.' . $field); + $currentParams = (array) $this->value('header.' . $field); + + if($inheritedParams && is_array($inheritedParams)) { + $currentParams = array_replace_recursive($inheritedParams, $currentParams); + } + + $this->modifyHeader($field, $currentParams); + + return $inherited; + } + + /** + * Helper method to return an ancestor field only to inherit from. The + * first occurrence of an ancestor field will be returned if at all. + * + * @param string $url The url of the page + * @param bool $field Name of the parent folder + * + * @return array + */ + public function inheritedField($url, $field) + { + /** @var Pages $pages */ + $pages = Grav::instance()['pages']; + + $inherited = $pages->inherited($url, $field); + $inheritedParams = (array) $inherited->value('header.' . $field); $currentParams = (array) $this->value('header.' . $field); if($inheritedParams && is_array($inheritedParams)) { diff --git a/system/src/Grav/Common/Page/Pages.php b/system/src/Grav/Common/Page/Pages.php index 0bbfee079a..59ae7a349d 100644 --- a/system/src/Grav/Common/Page/Pages.php +++ b/system/src/Grav/Common/Page/Pages.php @@ -321,14 +321,13 @@ public function ancestor($url, $path = null) return null; } - /** * Get a page ancestor trait. * * @param string $url The relative URL of the page * @param string $field The field name of the ancestor to query for * - * @return array|null + * @return Page|null */ public function inherited($url, $field = null) { @@ -345,7 +344,7 @@ public function inherited($url, $field = null) $ancestorField = $page->parent()->value('header.' . $field); if ($ancestorField != null) { - return $ancestorField; + return $page->parent(); } elseif (!$page->parent()->root()) { return $this->inherited($page->parent()->url(), $field); } From cfe80201e0abcf23da4a25e895e4c316e628617d Mon Sep 17 00:00:00 2001 From: coder4life Date: Thu, 23 Mar 2017 21:41:14 -0700 Subject: [PATCH 3/5] Updates according to code review issue Made changes to act on current Page object only for the Page functions Changed variable name to better reflect the intention of the passed param for the Pages class functions --- system/src/Grav/Common/Page/Page.php | 14 ++++++-------- system/src/Grav/Common/Page/Pages.php | 24 ++++++++++++------------ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/system/src/Grav/Common/Page/Page.php b/system/src/Grav/Common/Page/Page.php index 45077fb804..a3f569b9b4 100644 --- a/system/src/Grav/Common/Page/Page.php +++ b/system/src/Grav/Common/Page/Page.php @@ -2290,29 +2290,28 @@ public function root() * * @return \Grav\Common\Page\Page page you were looking for if it exists */ - public function ancestor($url, $lookup = null) + public function ancestor($lookup = null) { /** @var Pages $pages */ $pages = Grav::instance()['pages']; - return $pages->ancestor($url, $lookup); + return $pages->ancestor($this->route, $lookup); } /** * Helper method to return an ancestor page to inherit from. The current * page object is returned. * - * @param string $url The url of the page * @param bool $field Name of the parent folder * * @return Page */ - public function inherited($url, $field) + public function inherited($field) { /** @var Pages $pages */ $pages = Grav::instance()['pages']; - $inherited = $pages->inherited($url, $field); + $inherited = $pages->inherited($this->route, $field); $inheritedParams = (array) $inherited->value('header.' . $field); $currentParams = (array) $this->value('header.' . $field); @@ -2330,17 +2329,16 @@ public function inherited($url, $field) * Helper method to return an ancestor field only to inherit from. The * first occurrence of an ancestor field will be returned if at all. * - * @param string $url The url of the page * @param bool $field Name of the parent folder * * @return array */ - public function inheritedField($url, $field) + public function inheritedField($field) { /** @var Pages $pages */ $pages = Grav::instance()['pages']; - $inherited = $pages->inherited($url, $field); + $inherited = $pages->inherited($this->route, $field); $inheritedParams = (array) $inherited->value('header.' . $field); $currentParams = (array) $this->value('header.' . $field); diff --git a/system/src/Grav/Common/Page/Pages.php b/system/src/Grav/Common/Page/Pages.php index 59ae7a349d..2f8ce30955 100644 --- a/system/src/Grav/Common/Page/Pages.php +++ b/system/src/Grav/Common/Page/Pages.php @@ -294,27 +294,27 @@ public function children($path) /** * Get a page ancestor. * - * @param string $url The relative URL of the page + * @param string $route The relative URL of the page * @param string $path The relative path of the ancestor folder * * @return Page|null */ - public function ancestor($url, $path = null) + public function ancestor($route, $path = null) { if (!is_null($path)) { // Fetch page if there's a defined route to it. - $page = isset($this->routes[$url]) ? $this->get($this->routes[$url]) : null; + $page = isset($this->routes[$route]) ? $this->get($this->routes[$route]) : null; // Try without trailing slash - if (!$page && Utils::endsWith($url, '/')) { - $page = isset($this->routes[rtrim($url, '/')]) ? $this->get($this->routes[rtrim($url, '/')]) : null; + if (!$page && Utils::endsWith($route, '/')) { + $page = isset($this->routes[rtrim($route, '/')]) ? $this->get($this->routes[rtrim($route, '/')]) : null; } if ($page->path() == $path) { return $page; } elseif (!$page->parent()->root()) { - return $this->ancestor($page->parent()->url(), $path); + return $this->ancestor($page->parent()->route(), $path); } } @@ -324,21 +324,21 @@ public function ancestor($url, $path = null) /** * Get a page ancestor trait. * - * @param string $url The relative URL of the page + * @param string $route The relative route of the page * @param string $field The field name of the ancestor to query for * * @return Page|null */ - public function inherited($url, $field = null) + public function inherited($route, $field = null) { if (!is_null($field)) { // Fetch page if there's a defined route to it. - $page = isset($this->routes[$url]) ? $this->get($this->routes[$url]) : null; + $page = isset($this->routes[$route]) ? $this->get($this->routes[$route]) : null; // Try without trailing slash - if (!$page && Utils::endsWith($url, '/')) { - $page = isset($this->routes[rtrim($url, '/')]) ? $this->get($this->routes[rtrim($url, '/')]) : null; + if (!$page && Utils::endsWith($route, '/')) { + $page = isset($this->routes[rtrim($route, '/')]) ? $this->get($this->routes[rtrim($route, '/')]) : null; } $ancestorField = $page->parent()->value('header.' . $field); @@ -346,7 +346,7 @@ public function inherited($url, $field = null) if ($ancestorField != null) { return $page->parent(); } elseif (!$page->parent()->root()) { - return $this->inherited($page->parent()->url(), $field); + return $this->inherited($page->parent()->route(), $field); } } From d18ba44bb507be352b32d79a2892a570899bc1d5 Mon Sep 17 00:00:00 2001 From: coder4life Date: Fri, 24 Mar 2017 16:04:02 -0700 Subject: [PATCH 4/5] Simplify duplicate code --- system/src/Grav/Common/Page/Page.php | 38 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/system/src/Grav/Common/Page/Page.php b/system/src/Grav/Common/Page/Page.php index a3f569b9b4..6bfff300c9 100644 --- a/system/src/Grav/Common/Page/Page.php +++ b/system/src/Grav/Common/Page/Page.php @@ -2302,52 +2302,52 @@ public function ancestor($lookup = null) * Helper method to return an ancestor page to inherit from. The current * page object is returned. * - * @param bool $field Name of the parent folder + * @param string $field Name of the parent folder * * @return Page */ public function inherited($field) { - /** @var Pages $pages */ - $pages = Grav::instance()['pages']; - - $inherited = $pages->inherited($this->route, $field); - - $inheritedParams = (array) $inherited->value('header.' . $field); - $currentParams = (array) $this->value('header.' . $field); - - if($inheritedParams && is_array($inheritedParams)) { - $currentParams = array_replace_recursive($inheritedParams, $currentParams); - } + list($inherited, $currentParams) = $this->getInheritedParams($field); $this->modifyHeader($field, $currentParams); return $inherited; } - /** * Helper method to return an ancestor field only to inherit from. The * first occurrence of an ancestor field will be returned if at all. * - * @param bool $field Name of the parent folder + * @param string $field Name of the parent folder * * @return array */ public function inheritedField($field) { - /** @var Pages $pages */ + list($inherited, $currentParams) = $this->getInheritedParams($field); + + return $currentParams; + } + + /** + * Method that contains shared logic for inherited() and inheritedField() + * + * @param string $field Name of the parent folder + * + * @return array + */ + protected function getInheritedParams($field) + { $pages = Grav::instance()['pages']; + /** @var Pages $pages */ $inherited = $pages->inherited($this->route, $field); - $inheritedParams = (array) $inherited->value('header.' . $field); $currentParams = (array) $this->value('header.' . $field); - if($inheritedParams && is_array($inheritedParams)) { $currentParams = array_replace_recursive($inheritedParams, $currentParams); } - - return $currentParams; + return [$inherited, $currentParams]; } /** From edf401c4227fb86b42a17fe88d059cd5ec6c318d Mon Sep 17 00:00:00 2001 From: coder4life Date: Fri, 24 Mar 2017 23:02:52 -0700 Subject: [PATCH 5/5] Simplify Pages logic Simpilfied duplicate code. Also clarified variable naming for similar functions. --- system/src/Grav/Common/Page/Pages.php | 53 +++++++++++++-------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/system/src/Grav/Common/Page/Pages.php b/system/src/Grav/Common/Page/Pages.php index 2f8ce30955..9d5fd0ec1a 100644 --- a/system/src/Grav/Common/Page/Pages.php +++ b/system/src/Grav/Common/Page/Pages.php @@ -303,13 +303,7 @@ public function ancestor($route, $path = null) { if (!is_null($path)) { - // Fetch page if there's a defined route to it. - $page = isset($this->routes[$route]) ? $this->get($this->routes[$route]) : null; - - // Try without trailing slash - if (!$page && Utils::endsWith($route, '/')) { - $page = isset($this->routes[rtrim($route, '/')]) ? $this->get($this->routes[rtrim($route, '/')]) : null; - } + $page = $this->getPage($route); if ($page->path() == $path) { return $page; @@ -333,13 +327,7 @@ public function inherited($route, $field = null) { if (!is_null($field)) { - // Fetch page if there's a defined route to it. - $page = isset($this->routes[$route]) ? $this->get($this->routes[$route]) : null; - - // Try without trailing slash - if (!$page && Utils::endsWith($route, '/')) { - $page = isset($this->routes[rtrim($route, '/')]) ? $this->get($this->routes[rtrim($route, '/')]) : null; - } + $page = $this->getPage($route); $ancestorField = $page->parent()->value('header.' . $field); @@ -356,34 +344,29 @@ public function inherited($route, $field = null) /** * alias method to return find a page. * - * @param string $url The relative URL of the page + * @param string $route The relative URL of the page * @param bool $all * * @return Page|null */ - public function find($url, $all = false) + public function find($route, $all = false) { - return $this->dispatch($url, $all, false); + return $this->dispatch($route, $all, false); } /** * Dispatch URI to a page. * - * @param string $url The relative URL of the page + * @param string $route The relative URL of the page * @param bool $all * * @param bool $redirect * @return Page|null * @throws \Exception */ - public function dispatch($url, $all = false, $redirect = true) + public function dispatch($route, $all = false, $redirect = true) { - // Fetch page if there's a defined route to it. - $page = isset($this->routes[$url]) ? $this->get($this->routes[$url]) : null; - // Try without trailing slash - if (!$page && Utils::endsWith($url, '/')) { - $page = isset($this->routes[rtrim($url, '/')]) ? $this->get($this->routes[rtrim($url, '/')]) : null; - } + $page = $this->getPage($route); // Are we in the admin? this is important! $not_admin = !isset($this->grav['admin']); @@ -402,13 +385,13 @@ public function dispatch($url, $all = false, $redirect = true) $config = $this->grav['config']; // See if route matches one in the site configuration - $route = $config->get("site.routes.{$url}"); + $route = $config->get("site.routes.{$route}"); if ($route) { $page = $this->dispatch($route, $all); } else { // Try Regex style redirects $uri = $this->grav['uri']; - $source_url = $url; + $source_url = $route; $extension = $uri->extension(); if (isset($extension) && !Utils::endsWith($uri->url(), $extension)) { $source_url.= '.' . $extension; @@ -451,6 +434,22 @@ public function dispatch($url, $all = false, $redirect = true) return $page; } + /** + * Retrieve page instance based on the route + * + * @return Page + */ + protected function getPage($route) { + // Fetch page if there's a defined route to it. + $page = isset($this->routes[$route]) ? $this->get($this->routes[$route]) : null; + // Try without trailing slash + if (!$page && Utils::endsWith($route, '/')) { + $page = isset($this->routes[rtrim($route, '/')]) ? $this->get($this->routes[rtrim($route, '/')]) : null; + } + + return $page; + } + /** * Get root page. *