From c45499b8764320f1af023645c315ae900521a927 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 15 Oct 2019 15:35:47 +1300 Subject: [PATCH 01/11] Initial work on categorisation rewrite. Remove relation between blog and categories / tags Soft-resolve duplicates on query via URLSegment Remove and simplify tag / category management Code quality cleanup --- src/Admin/GridFieldCategorisationConfig.php | 45 +-- src/Model/Blog.php | 135 ++++--- src/Model/BlogCategory.php | 10 +- src/Model/BlogController.php | 391 +++++++++++--------- src/Model/BlogMemberExtension.php | 5 + src/Model/BlogObject.php | 42 ++- src/Model/BlogPost.php | 24 +- src/Model/BlogTag.php | 7 +- src/Model/CategorisationObject.php | 4 +- 9 files changed, 390 insertions(+), 273 deletions(-) diff --git a/src/Admin/GridFieldCategorisationConfig.php b/src/Admin/GridFieldCategorisationConfig.php index 05d36d9..6e9de8d 100644 --- a/src/Admin/GridFieldCategorisationConfig.php +++ b/src/Admin/GridFieldCategorisationConfig.php @@ -4,6 +4,7 @@ namespace SilverStripe\Blog\Admin; use SilverStripe\Blog\Forms\GridField\GridFieldAddByDBField; use SilverStripe\Blog\Model\CategorisationObject; +use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Forms\GridField\GridFieldAddNewButton; use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor; use SilverStripe\Forms\GridField\GridFieldDataColumns; @@ -12,13 +13,14 @@ use SilverStripe\ORM\SS_List; class GridFieldCategorisationConfig extends GridFieldConfig_RecordEditor { /** - * @param int $itemsPerPage + * @param int $itemsPerPage * @param array|SS_List $mergeRecords - * @param string $parentType - * @param string $parentMethod - * @param string $childMethod + * @param string $parentType + * @param string $parentMethod + * @param string $childMethod + * @param SiteTree $parent */ - public function __construct($itemsPerPage, $mergeRecords, $parentType, $parentMethod, $childMethod) + public function __construct($itemsPerPage, $mergeRecords, $parentType, $parentMethod, $childMethod, $parent) { parent::__construct($itemsPerPage); @@ -39,31 +41,24 @@ class GridFieldCategorisationConfig extends GridFieldConfig_RecordEditor $columns->setFieldFormatting( [ - 'BlogPostsCount' => function ($value, CategorisationObject $item) { + 'BlogPostsCount' => function ($value, CategorisationObject $item) use ($parent) { + return $item + ->BlogPosts() + ->filter(['ParentID' => $parent->ID]) + ->Count(); + }, + 'BlogPostsAllCount' => function ($value, CategorisationObject $item) { return $item->BlogPosts()->Count(); - } + }, ] ); - - $this->changeColumnOrder(); - } - - /** - * Reorders GridField columns so that Actions is last. - */ - protected function changeColumnOrder() - { - /** - * @var GridFieldDataColumns $columns - */ - $columns = $this->getComponentByType(GridFieldDataColumns::class); - $columns->setDisplayFields( [ - 'Title' => _t(__CLASS__ . '.Title', 'Title'), - 'BlogPostsCount' => _t(__CLASS__ . '.Posts', 'Posts'), - 'MergeAction' => 'MergeAction', - 'Actions' => 'Actions' + 'Title' => _t(__CLASS__ . '.Title', 'Title'), + 'BlogPostsCount' => _t(__CLASS__ . '.PostsThisBlog', 'Posts (This Blog)'), + 'BlogPostsAllCount' => _t(__CLASS__ . '.PostsAllBlogs', 'Posts (All Blogs)'), + 'MergeAction' => 'MergeAction', + 'Actions' => 'Actions' ] ); } diff --git a/src/Model/Blog.php b/src/Model/Blog.php index 7d8df6d..eaf33a3 100644 --- a/src/Model/Blog.php +++ b/src/Model/Blog.php @@ -2,10 +2,10 @@ namespace SilverStripe\Blog\Model; +use Exception; use Page; use SilverStripe\Blog\Admin\GridFieldCategorisationConfig; use SilverStripe\Blog\Forms\GridField\GridFieldConfigBlogPost; -use SilverStripe\CMS\Controllers\RootURLController; use SilverStripe\Control\Controller; use SilverStripe\Core\Convert; use SilverStripe\Forms\FieldList; @@ -14,13 +14,15 @@ use SilverStripe\Forms\GridField\GridFieldConfig; use SilverStripe\Forms\ListboxField; use SilverStripe\Forms\LiteralField; use SilverStripe\Forms\NumericField; +use SilverStripe\Forms\Tab; +use SilverStripe\Forms\TabSet; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DB; -use SilverStripe\ORM\HasManyList; use SilverStripe\ORM\ManyManyList; use SilverStripe\ORM\SS_List; use SilverStripe\ORM\UnsavedRelationList; +use SilverStripe\ORM\ValidationException; use SilverStripe\Security\Group; use SilverStripe\Security\Member; use SilverStripe\Security\Permission; @@ -31,8 +33,7 @@ use SilverStripe\View\Requirements; /** * Blog Holder * - * @method HasManyList Tags() List of tags in this blog - * @method HasManyList Categories() List of categories in this blog + * @property int $PostsPerPage * @method ManyManyList Editors() List of editors * @method ManyManyList Writers() List of writers * @method ManyManyList Contributors() List of contributors @@ -88,20 +89,12 @@ class Blog extends Page implements PermissionProvider 'PostsPerPage' => 'Int', ]; - /** - * @var array - */ - private static $has_many = [ - 'Tags' => BlogTag::class, - 'Categories' => BlogCategory::class, - ]; - /** * @var array */ private static $many_many = [ - 'Editors' => Member::class, - 'Writers' => Member::class, + 'Editors' => Member::class, + 'Writers' => Member::class, 'Contributors' => Member::class, ]; @@ -134,6 +127,52 @@ class Blog extends Page implements PermissionProvider private static $icon_class = 'font-icon-p-posts'; + /** + * Gets the list of all tags attached to this blog + * + * @param bool $hideEmpty Set to false to include tags without posts + * @return DataList|BlogTag[] + */ + public function Tags($hideEmpty = true) + { + $tags = BlogTag::get(); + if ($this->ID) { + $tags->setDataQueryParam('BlogID', $this->ID); + + // Conditionally hide empty tags + if ($hideEmpty) { + $tags = $tags->filter([ + 'BlogPosts.ParentID' => $this->ID, + ]); + } + } + $this->extend('updateBlogTags', $tags); + return $tags; + } + + /** + * Gets the list of all categories attached to this blog + * + * @param bool $hideEmpty Set to false to include categories without posts + * @return DataList|BlogCategory[] + */ + public function Categories($hideEmpty = true) + { + $tags = BlogCategory::get(); + if ($this->ID) { + $tags->setDataQueryParam('BlogID', $this->ID); + + // Conditionally hide empty categories + if ($hideEmpty) { + $tags = $tags->filter([ + 'BlogPosts.ParentID' => $this->ID, + ]); + } + } + $this->extend('updateBlogCategories', $tags); + return $tags; + } + /** * {@inheritdoc} */ @@ -141,7 +180,7 @@ class Blog extends Page implements PermissionProvider { $this->addCMSRequirements(); - $this->beforeUpdateCMSFields(function ($fields) { + $this->beforeUpdateCMSFields(function (FieldList $fields) { if (!$this->canEdit()) { return; } @@ -149,43 +188,47 @@ class Blog extends Page implements PermissionProvider $categories = GridField::create( 'Categories', _t(__CLASS__ . '.Categories', 'Categories'), - $this->Categories(), + $this->Categories(false), GridFieldCategorisationConfig::create( 15, - $this->Categories()->sort('Title'), + $this->Categories(false)->sort('Title'), BlogCategory::class, 'Categories', - 'BlogPosts' + 'BlogPosts', + $this ) ); $tags = GridField::create( 'Tags', _t(__CLASS__ . '.Tags', 'Tags'), - $this->Tags(), + $this->Tags(false), GridFieldCategorisationConfig::create( 15, - $this->Tags()->sort('Title'), + $this->Tags(false)->sort('Title'), BlogTag::class, 'Tags', - 'BlogPosts' + 'BlogPosts', + $this ) ); - /** - * @var FieldList $fields - */ - $fields->addFieldsToTab( + $fields->addFieldToTab( + 'Root', + TabSet::create('Categorisation', _t(__CLASS__ . '.Categorisation', 'Categorisation')) + ->addExtraClass('blog-cms-categorisation') + ); + $fields->addFieldToTab( 'Root.Categorisation', - [ - $categories, - $tags - ] + Tab::create('Categories', _t(__CLASS__ . '.Categories', 'Categories')) + ); + $fields->addFieldToTab( + 'Root.Categorisation', + Tab::create('Tags', _t(__CLASS__ . '.Tags', 'Tags')) ); - $fields->fieldByName('Root.Categorisation') - ->addExtraClass('blog-cms-categorisation') - ->setTitle(_t(__CLASS__ . '.Categorisation', 'Categorisation')); + $fields->addFieldToTab('Root.Categorisation.Categories', $categories); + $fields->addFieldToTab('Root.Categorisation.Tags', $tags); }); return parent::getCMSFields(); @@ -199,6 +242,7 @@ class Blog extends Page implements PermissionProvider Requirements::css('silverstripe/blog:client/dist/styles/main.css'); Requirements::javascript('silverstripe/blog:client/dist/js/main.bundle.js'); } + /** * {@inheritdoc} */ @@ -249,7 +293,7 @@ class Blog extends Page implements PermissionProvider /** * Determine if the given member belongs to the given relation. * - * @param Member $member + * @param Member $member * @param DataList $relation * * @return bool @@ -527,7 +571,7 @@ class Blog extends Page implements PermissionProvider /** * Returns BlogPosts for a given date period. * - * @param int $year + * @param int $year * @param null|int $month * @param null|int $day * @@ -592,13 +636,7 @@ class Blog extends Page implements PermissionProvider */ public function ProfileLink($urlSegment) { - $baseLink = $this->Link(); - if ($baseLink === '/') { - // Handle homepage blogs - $baseLink = RootURLController::get_homepage_link(); - } - - return Controller::join_links($baseLink, 'profile', $urlSegment); + return Controller::join_links($this->Link('Profile'), $urlSegment); } /** @@ -628,22 +666,22 @@ class Blog extends Page implements PermissionProvider { return [ Blog::MANAGE_USERS => [ - 'name' => _t( + 'name' => _t( __CLASS__ . '.PERMISSION_MANAGE_USERS_DESCRIPTION', 'Manage users for individual blogs' ), - 'help' => _t( + 'help' => _t( __CLASS__ . '.PERMISSION_MANAGE_USERS_HELP', 'Allow assignment of Editors, Writers, or Contributors to blogs' ), 'category' => _t(__CLASS__ . '.PERMISSIONS_CATEGORY', 'Blog permissions'), - 'sort' => 100 + 'sort' => 100 ] ]; } /** - * {@inheritdoc} + * @throws ValidationException */ protected function onBeforeWrite() { @@ -653,6 +691,9 @@ class Blog extends Page implements PermissionProvider /** * Assign users as necessary to the blog group. + * + * @throws ValidationException + * @throws Exception */ protected function assignGroup() { @@ -665,6 +706,7 @@ class Blog extends Page implements PermissionProvider // Must check if the method exists or else an error occurs when changing page type if ($this->hasMethod('Editors')) { foreach ([$this->Editors(), $this->Writers(), $this->Contributors()] as $levels) { + /** @var Member $user */ foreach ($levels as $user) { if (!$user->inGroup($group)) { $user->Groups()->add($group); @@ -678,13 +720,14 @@ class Blog extends Page implements PermissionProvider * Gets or creates the group used to assign CMS access. * * @return Group + * @throws ValidationException */ protected function getUserGroup() { $code = $this->config()->get('grant_user_group'); + /** @var Group $group */ $group = Group::get()->filter('Code', $code)->first(); - if ($group) { return $group; } diff --git a/src/Model/BlogCategory.php b/src/Model/BlogCategory.php index ae69c7d..7bbfdf5 100644 --- a/src/Model/BlogCategory.php +++ b/src/Model/BlogCategory.php @@ -3,16 +3,14 @@ namespace SilverStripe\Blog\Model; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\ManyManyList; /** * A blog category for generalising blog posts. * -* - * @method Blog Blog() - * + * @method ManyManyList|BlogPost[] BlogPosts() * @property string $Title * @property string $URLSegment - * @property int $BlogID */ class BlogCategory extends DataObject implements CategorisationObject { @@ -44,8 +42,8 @@ class BlogCategory extends DataObject implements CategorisationObject /** * @var array */ - private static $has_one = [ - 'Blog' => Blog::class, + private static $indexes = [ + 'URLSegment' => true, ]; /** diff --git a/src/Model/BlogController.php b/src/Model/BlogController.php index 9a8fd83..e27a195 100644 --- a/src/Model/BlogController.php +++ b/src/Model/BlogController.php @@ -6,14 +6,17 @@ use PageController; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\RSS\RSSFeed; -use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataList; use SilverStripe\ORM\FieldType\DBDatetime; +use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\ORM\PaginatedList; +use SilverStripe\ORM\SS_List; use SilverStripe\Security\Member; use SilverStripe\View\Parsers\URLSegmentFilter; -use SilverStripe\Control\HTTPRequest; +/** + * @method Blog data() + */ class BlogController extends PageController { /** @@ -31,17 +34,17 @@ class BlogController extends PageController * @var array */ private static $url_handlers = [ - 'tag/$Tag!/$Rss' => 'tag', - 'category/$Category!/$Rss' => 'category', + 'tag/$Tag!/$Rss' => 'tag', + 'category/$Category!/$Rss' => 'category', 'archive/$Year!/$Month/$Day' => 'archive', - 'profile/$URLSegment!' => 'profile' + 'profile/$Profile!' => 'profile' ]; /** * @var array */ private static $casting = [ - 'MetaTitle' => 'Text', + 'MetaTitle' => 'Text', 'FilterDescription' => 'Text' ]; @@ -60,27 +63,11 @@ class BlogController extends PageController */ protected $blogPosts; - /** - * @return string - */ - public function index(HTTPRequest $request) - { - /** - * @var Blog $dataRecord - */ - $dataRecord = $this->dataRecord; - - $this->blogPosts = $dataRecord->getBlogPosts(); - - return $this->render(); - } - /** * Renders a Blog Member's profile. * * @throws HTTPResponse_Exception - * - * @return string + * @return $this */ public function profile() { @@ -88,45 +75,59 @@ class BlogController extends PageController $this->httpError(404, 'Not Found'); } - $profile = $this->getCurrentProfile(); - - if (!$profile) { - return $this->httpError(404, 'Not Found'); + // Get profile posts + $posts = $this->getCurrentProfilePosts(); + if (!$posts) { + $this->httpError(404, 'Not Found'); } - $this->blogPosts = $this->getCurrentProfilePosts(); - - return $this->render(); + $this->setFilteredPosts($posts); + return $this; } /** * Get the Member associated with the current URL segment. * - * @return null|Member + * @return null|Member|BlogMemberExtension */ public function getCurrentProfile() { - $urlSegment = $this->request->param('URLSegment'); - if ($urlSegment) { - $filter = URLSegmentFilter::create(); - // url encode unless it's multibyte (already pre-encoded in the database) - // see https://github.com/silverstripe/silverstripe-cms/pull/2384 - if (!$filter->getAllowMultibyte()) { - $urlSegment = rawurlencode($urlSegment); - } - - return Member::get() - ->filter('URLSegment', $urlSegment) - ->first(); + $segment = $this->getCurrentProfileURLSegment(); + if (!$segment) { + return null; } - return null; + /** @var Member $profile */ + $profile = Member::get() + ->find('URLSegment', $segment); + return $profile; + } + + /** + * Get URL Segment of current profile + * + * @return null|string + */ + public function getCurrentProfileURLSegment() + { + $segment = isset($this->urlParams['Profile']) + ? $this->urlParams['Profile'] + : null; + if (!$segment) { + return null; + } + + // url encode unless it's multibyte (already pre-encoded in the database) + // see https://github.com/silverstripe/silverstripe-cms/pull/2384 + return URLSegmentFilter::singleton()->getAllowMultibyte() + ? $segment + : rawurlencode($segment); } /** * Get posts related to the current Member profile. * - * @return null|DataList + * @return null|DataList|BlogPost[] */ public function getCurrentProfilePosts() { @@ -142,166 +143,190 @@ class BlogController extends PageController /** * Renders an archive for a specified date. This can be by year or year/month. * - * @return null|string + * @return $this + * @throws HTTPResponse_Exception */ public function archive() { - /** - * @var Blog $dataRecord - */ - $dataRecord = $this->dataRecord; - $year = $this->getArchiveYear(); $month = $this->getArchiveMonth(); $day = $this->getArchiveDay(); - if ($this->request->param('Month') && !$month) { + // Validate all values + if ($year === false || $month === false || $day === false) { $this->httpError(404, 'Not Found'); } - if ($month && $this->request->param('Day') && !$day) { - $this->httpError(404, 'Not Found'); - } - - if ($year) { - $this->blogPosts = $dataRecord->getArchivedBlogPosts($year, $month, $day); - - return $this->render(); - } - - $this->httpError(404, 'Not Found'); - - return null; + $posts = $this->data()->getArchivedBlogPosts($year, $month, $day); + $this->setFilteredPosts($posts); + return $this; } /** * Fetches the archive year from the url. * - * @return int + * Returns int if valid, current year if not provided, false if invalid value + * + * @return int|false */ public function getArchiveYear() { - if ($this->request->param('Year')) { - if (preg_match('/^[0-9]{4}$/', $year = $this->request->param('Year'))) { - return (int) $year; - } - } elseif ($this->request->param('Action') == 'archive') { + if (isset($this->urlParams['Year']) + && preg_match('/^[0-9]{4}$/', $this->urlParams['Year']) + ) { + return (int)$this->urlParams['Year']; + } + + if ($this->urlParams['Action'] === 'archive') { return DBDatetime::now()->Year(); } - return null; + return false; } /** * Fetches the archive money from the url. * - * @return null|int + * Returns int if valid, null if not provided, false if invalid value + * + * @return null|int|false */ public function getArchiveMonth() { - $month = $this->request->param('Month'); + $month = isset($this->urlParams['Month']) + ? $this->urlParams['Month'] + : null; - if (preg_match('/^[0-9]{1,2}$/', $month)) { - if ($month > 0 && $month < 13) { - if (checkdate($month, 01, $this->getArchiveYear())) { - return (int) $month; - } - } + if (preg_match('/^[0-9]{1,2}$/', $month) + && $month > 0 + && $month < 13 + ) { + return (int)$month; } - return null; + return false; } /** * Fetches the archive day from the url. * - * @return null|int + * Returns int if valid, null if not provided, false if invalid value + * + * @return null|int|false */ public function getArchiveDay() { - $day = $this->request->param('Day'); + $day = isset($this->urlParams['Day']) + ? $this->urlParams['Day'] + : null; - if (preg_match('/^[0-9]{1,2}$/', $day)) { - if (checkdate($this->getArchiveMonth(), $day, $this->getArchiveYear())) { - return (int) $day; - } + // Cannot calculate day without month and year + $month = $this->getArchiveMonth(); + $year = $this->getArchiveYear(); + if (!$month || !$year) { + return false; } - return null; + if (preg_match('/^[0-9]{1,2}$/', $day) && checkdate($month, $day, $year)) { + return (int)$day; + } + + return false; } /** * Renders the blog posts for a given tag. * - * @return null|string + * @return DBHTMLText|$this + * @throws HTTPResponse_Exception */ public function tag() { + // Ensure tag exists $tag = $this->getCurrentTag(); - - if ($tag) { - $this->blogPosts = $tag->BlogPosts(); - - if ($this->isRSS()) { - return $this->rssFeed($this->blogPosts, $tag->getLink()); - } else { - return $this->render(); - } + if (!$tag) { + $this->httpError(404, 'Not Found'); } - $this->httpError(404, 'Not Found'); + // Get posts with this tag + $posts = $this + ->data() + ->getBlogPosts() + ->filter(['Tags.URLSegment' => $tag->URLSegment]); // Soft duplicate handling - return null; + $this->setFilteredPosts($posts); + + // Render as RSS if provided + if ($this->isRSS()) { + return $this->rssFeed($posts, $tag->getLink()); + } + + return $this; } /** - * Tag Getter for use in templates. + * Get BlogTag assigned to current filter * * @return null|BlogTag */ public function getCurrentTag() { - /** - * @var Blog $dataRecord - */ - $dataRecord = $this->dataRecord; - $tag = $this->request->param('Tag'); - if ($tag) { - $filter = URLSegmentFilter::create(); - // url encode unless it's multibyte (already pre-encoded in the database) - // see https://github.com/silverstripe/silverstripe-cms/pull/2384 - if (!$filter->getAllowMultibyte()) { - $tag = rawurlencode($tag); - } - - return $dataRecord->Tags() - ->filter('URLSegment', $tag) - ->first(); + $segment = $this->getCurrentTagURLSegment(); + if (!$segment) { + return null; } - return null; + + /** @var BlogTag $tag */ + $tag = $this + ->data() + ->Tags(false)// Show "no results" instead of "404" + ->find('URLSegment', $segment); + return $tag; + } + + /** + * Get URLSegment of selected category (not: URLEncoded based on multibyte) + * + * @return string|null + */ + public function getCurrentTagURLSegment() + { + $segment = isset($this->urlParams['Tag']) + ? $this->urlParams['Tag'] + : null; + + // url encode unless it's multibyte (already pre-encoded in the database) + // see https://github.com/silverstripe/silverstripe-cms/pull/2384 + return URLSegmentFilter::singleton()->getAllowMultibyte() + ? $segment + : rawurlencode($segment); } /** * Renders the blog posts for a given category. * - * @return null|string + * @return DBHTMLText|$this + * @throws HTTPResponse_Exception */ public function category() { $category = $this->getCurrentCategory(); - if ($category) { - $this->blogPosts = $category->BlogPosts(); - - if ($this->isRSS()) { - return $this->rssFeed($this->blogPosts, $category->getLink()); - } - return $this->render(); + if (!$category) { + $this->httpError(404, 'Not Found'); } - $this->httpError(404, 'Not Found'); + // Get posts with this category + $posts = $this + ->data() + ->getBlogPosts() + ->filter(['Categories.URLSegment' => $category->URLSegment]); // Soft duplicate handling + $this->setFilteredPosts($posts); - return null; + if ($this->isRSS()) { + return $this->rssFeed($posts, $category->getLink()); + } + return $this; } /** @@ -311,24 +336,35 @@ class BlogController extends PageController */ public function getCurrentCategory() { - /** - * @var Blog $dataRecord - */ - $dataRecord = $this->dataRecord; - $category = $this->request->param('Category'); - if ($category) { - $filter = URLSegmentFilter::create(); - // url encode unless it's multibyte (already pre-encoded in the database) - // see https://github.com/silverstripe/silverstripe-cms/pull/2384 - if (!$filter->getAllowMultibyte()) { - $category = rawurlencode($category); - } - - return $dataRecord->Categories() - ->filter('URLSegment', $category) - ->first(); + $segment = $this->getCurrentCategoryURLSegment(); + if (!$segment) { + return null; } - return null; + + /** @var BlogCategory $category */ + $category = $this + ->data() + ->Categories(false)// Show "no results" instead of "404" + ->find('URLSegment', $segment); + return $category; + } + + /** + * Get URLSegment of selected category + * + * @return string|null + */ + public function getCurrentCategoryURLSegment() + { + $segment = isset($this->urlParams['Category']) + ? $this->urlParams['Category'] + : null; + + // url encode unless it's multibyte (already pre-encoded in the database) + // see https://github.com/silverstripe/silverstripe-cms/pull/2384 + return URLSegmentFilter::singleton()->getAllowMultibyte() + ? $segment + : rawurlencode($segment); } /** @@ -406,13 +442,13 @@ class BlogController extends PageController ); } - if ($this->owner->getArchiveYear()) { - if ($this->owner->getArchiveDay()) { - $date = $this->owner->getArchiveDate()->Nice(); - } elseif ($this->owner->getArchiveMonth()) { - $date = $this->owner->getArchiveDate()->format('MMMM, y'); + if ($this->getArchiveYear()) { + if ($this->getArchiveDay()) { + $date = $this->getArchiveDate()->Nice(); + } elseif ($this->getArchiveMonth()) { + $date = $this->getArchiveDate()->format('MMMM, y'); } else { - $date = $this->owner->getArchiveDate()->format('y'); + $date = $this->getArchiveDate()->format('y'); } $items[] = _t( @@ -436,6 +472,28 @@ class BlogController extends PageController return $result; } + /** + * Get filtered blog posts + * + * @return DataList|BlogPost[] + */ + public function getFilteredPosts() + { + return $this->blogPosts ?: $this->data()->getBlogPosts(); + } + + /** + * Set filtered posts + * + * @param SS_List|BlogPost[] $posts + * @return $this + */ + public function setFilteredPosts($posts) + { + $this->blogPosts = $posts; + return $this; + } + /** * Returns a list of paginated blog posts based on the BlogPost dataList. * @@ -443,12 +501,12 @@ class BlogController extends PageController */ public function PaginatedList() { - $allPosts = $this->blogPosts ?: ArrayList::create(); + $allPosts = $this->getFilteredPosts(); $posts = PaginatedList::create($allPosts); // Set appropriate page size - if ($this->PostsPerPage > 0) { - $pageSize = $this->PostsPerPage; + if ($this->data()->PostsPerPage > 0) { + $pageSize = $this->data()->PostsPerPage; } elseif ($count = $allPosts->count()) { $pageSize = $count; } else { @@ -482,7 +540,7 @@ class BlogController extends PageController return null; } - /** + /** * Returns the absolute link to the previous page for use in the page meta tags. This helps search engines * find the pagination and index all pages properly. * @@ -507,14 +565,7 @@ class BlogController extends PageController */ public function rss() { - /** - * @var Blog $dataRecord - */ - $dataRecord = $this->dataRecord; - - $this->blogPosts = $dataRecord->getBlogPosts(); - - return $this->rssFeed($this->blogPosts, $this->Link()); + return $this->rssFeed($this->getFilteredPosts(), $this->Link()); } /** @@ -561,13 +612,18 @@ class BlogController extends PageController * Displays an RSS feed of the given blog posts. * * @param DataList $blogPosts - * @param string $link + * @param string $link * - * @return string + * @return DBHTMLText */ protected function rssFeed($blogPosts, $link) { - $rss = RSSFeed::create($blogPosts, $link, $this->MetaTitle, $this->MetaDescription); + $rss = RSSFeed::create( + $blogPosts, + $link, + $this->getMetaTitle(), + $this->data()->MetaDescription + ); $this->extend('updateRss', $rss); @@ -581,7 +637,6 @@ class BlogController extends PageController */ protected function isRSS() { - $rss = $this->request->param('Rss'); - return (is_string($rss) && strcasecmp($rss, 'rss') == 0); + return isset($this->urlParams['RSS']) && strcasecmp($this->urlParams['RSS'], 'rss') == 0; } } diff --git a/src/Model/BlogMemberExtension.php b/src/Model/BlogMemberExtension.php index 86f6cec..ee7e553 100644 --- a/src/Model/BlogMemberExtension.php +++ b/src/Model/BlogMemberExtension.php @@ -10,6 +10,7 @@ use SilverStripe\Forms\GridField\GridFieldAddNewButton; use SilverStripe\Forms\Tab; use SilverStripe\Forms\TextareaField; use SilverStripe\ORM\DataExtension; +use SilverStripe\ORM\ManyManyList; use SilverStripe\Security\Member; use SilverStripe\View\Parsers\URLSegmentFilter; use SilverStripe\View\Requirements; @@ -17,6 +18,10 @@ use SilverStripe\View\Requirements; /** * This class is responsible for add Blog specific behaviour to Members. * + * @property string $URLSegment + * @property string $BlogProfileSummary + * @method Image BlogProfileImage() + * @method ManyManyList|BlogPost[] BlogPosts() */ class BlogMemberExtension extends DataExtension { diff --git a/src/Model/BlogObject.php b/src/Model/BlogObject.php index ba83d9a..292efec 100644 --- a/src/Model/BlogObject.php +++ b/src/Model/BlogObject.php @@ -8,6 +8,7 @@ use SilverStripe\Forms\Tab; use SilverStripe\Forms\TabSet; use SilverStripe\Forms\TextField; use SilverStripe\ORM\DataList; +use SilverStripe\ORM\ManyManyList; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Member; use SilverStripe\Security\Permission; @@ -20,7 +21,7 @@ use SilverStripe\View\Parsers\URLSegmentFilter; trait BlogObject { /** - * @return DataList + * @return ManyManyList|BlogPost[] */ public function BlogPosts() { @@ -31,6 +32,22 @@ trait BlogObject return $blogPosts; } + /** + * Get blog this tag was queried from + * + * @return Blog|null + */ + public function Blog() + { + $blogID = $this->getSourceQueryParam('BlogID'); + if ($blogID) { + /** @var Blog $blog */ + $blog = Blog::get()->byID($blogID); + return $blog; + } + return null; + } + /** * {@inheritdoc} */ @@ -75,14 +92,18 @@ trait BlogObject } /** - * Returns a relative link to this category. + * Returns a relative link to this category or tag * * @return string */ public function getLink() { + $blog = $this->Blog(); + if (!$blog) { + return null; + } return Controller::join_links( - $this->Blog()->Link(), + $blog->Link(), $this->getListUrlSegment(), $this->URLSegment ); @@ -158,14 +179,12 @@ trait BlogObject return $this->Blog()->canEdit($member); } - /** - * {@inheritdoc} - */ protected function onBeforeWrite() { parent::onBeforeWrite(); + if ($this->exists() || empty($this->URLSegment)) { - return $this->generateURLSegment(); + $this->generateURLSegment(); } } @@ -178,7 +197,7 @@ trait BlogObject */ public function generateURLSegment($increment = 0) { - $increment = (int) $increment; + $increment = (int)$increment; $filter = URLSegmentFilter::create(); // Setting this to on. Because of the UI flow, it would be quite a lot of work @@ -209,12 +228,7 @@ trait BlogObject protected function getDuplicatesByField($field) { $duplicates = DataList::create(self::class) - ->filter( - [ - $field => $this->$field, - 'BlogID' => (int) $this->BlogID - ] - ); + ->filter([$field => $this->$field]); if ($this->ID) { $duplicates = $duplicates->exclude('ID', $this->ID); diff --git a/src/Model/BlogPost.php b/src/Model/BlogPost.php index 321f6a6..d03b0d5 100644 --- a/src/Model/BlogPost.php +++ b/src/Model/BlogPost.php @@ -313,14 +313,6 @@ class BlogPost extends Page } // Get categories and tags - $parent = $this->Parent(); - $categories = $parent instanceof Blog - ? $parent->Categories() - : BlogCategory::get(); - $tags = $parent instanceof Blog - ? $parent->Tags() - : BlogTag::get(); - // @todo: Reimplement the sidebar // $options = BlogAdminSidebar::create( $fields->addFieldsToTab( @@ -330,7 +322,7 @@ class BlogPost extends Page TagField::create( 'Categories', _t(__CLASS__ . '.Categories', 'Categories'), - $categories, + BlogCategory::get(), $this->Categories() ) ->setCanCreate($this->canCreateCategories()) @@ -338,7 +330,7 @@ class BlogPost extends Page TagField::create( 'Tags', _t(__CLASS__ . '.Tags', 'Tags'), - $tags, + BlogTag::get(), $this->Tags() ) ->setCanCreate($this->canCreateTags()) @@ -816,4 +808,16 @@ class BlogPost extends Page $this->Authors()->add($member); } } + + /** + * So that tags / categories queried through this post generate the correct Link() + * + * @return array + */ + public function getInheritableQueryParams() + { + $params = parent::getInheritableQueryParams(); + $params['BlogID'] = $this->ParentID; + return $params; + } } diff --git a/src/Model/BlogTag.php b/src/Model/BlogTag.php index 4180f67..2399880 100644 --- a/src/Model/BlogTag.php +++ b/src/Model/BlogTag.php @@ -3,6 +3,7 @@ namespace SilverStripe\Blog\Model; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\ManyManyList; /** * A blog tag for keyword descriptions of a blog post. @@ -10,9 +11,9 @@ use SilverStripe\ORM\DataObject; * * @method Blog Blog() * + * @method ManyManyList|BlogPost[] BlogPosts() * @property string $Title * @property string $URLSegment - * @property int $BlogID */ class BlogTag extends DataObject implements CategorisationObject { @@ -44,8 +45,8 @@ class BlogTag extends DataObject implements CategorisationObject /** * @var array */ - private static $has_one = [ - 'Blog' => Blog::class + private static $indexes = [ + 'URLSegment' => true, ]; /** diff --git a/src/Model/CategorisationObject.php b/src/Model/CategorisationObject.php index 827035b..3e29247 100644 --- a/src/Model/CategorisationObject.php +++ b/src/Model/CategorisationObject.php @@ -2,8 +2,10 @@ namespace SilverStripe\Blog\Model; +use SilverStripe\ORM\ManyManyList; + /** - * @method ManyManyList BlogPosts + * @method ManyManyList|BlogPost[] BlogPosts() */ interface CategorisationObject { From ff5a5a33d00e0caec2e63acc361f3878cb8d6b06 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 15 Oct 2019 15:45:14 +1300 Subject: [PATCH 02/11] Tag isn't written --- src/Forms/GridField/GridFieldAddByDBField.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Forms/GridField/GridFieldAddByDBField.php b/src/Forms/GridField/GridFieldAddByDBField.php index 43a0c43..e8b8503 100644 --- a/src/Forms/GridField/GridFieldAddByDBField.php +++ b/src/Forms/GridField/GridFieldAddByDBField.php @@ -99,6 +99,7 @@ class GridFieldAddByDBField implements GridField_ActionProvider, GridField_HTMLP $obj->setCastedField($dbField, $data['gridfieldaddbydbfield'][$obj->ClassName][$dbField]); if ($obj->canCreate()) { + $obj->write(); $id = $gridField->getList()->add($obj); if (!$id) { $gridField->setCustomValidationMessage( From 283f65880d61b8462a8d8221b490bea7cc686113 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 15 Oct 2019 16:59:30 +1300 Subject: [PATCH 03/11] Build task for de-duplicating tags and categories --- src/Tasks/FixBlogDuplicatesTask.php | 139 ++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 src/Tasks/FixBlogDuplicatesTask.php diff --git a/src/Tasks/FixBlogDuplicatesTask.php b/src/Tasks/FixBlogDuplicatesTask.php new file mode 100644 index 0000000..45b905b --- /dev/null +++ b/src/Tasks/FixBlogDuplicatesTask.php @@ -0,0 +1,139 @@ +dedupe(BlogTag::class, 'BlogPost_Tags', 'BlogTagID'); + $this->dedupe(BlogCategory::class, 'BlogPost_Categories', 'BlogCategoryID'); + } + + /** + * Log message to console / page + * + * @param string $message + */ + protected function message($message) + { + if (Director::is_cli()) { + echo "{$message}\n"; + } else { + echo HTML::createTag('p', [], Convert::raw2xml($message)); + } + } + + /** + * Progress used for CLI output + * + * @var int + */ + protected $printedDots = 0; + + /** + * Render progress dots (20 dots per column) + * + * @param $current + * @param int $total + * @param string $character + */ + protected function progress($current, $total = 0, $character = '.') + { + if (!Director::is_cli()) { + return; + } + while ($this->printedDots < $current) { + echo $character; + $this->printedDots++; + if ($this->printedDots % 80 === 0) { + if ($total) { + $len = strlen($total); + echo str_pad("{$this->printedDots}/{$total}", $len * 2 + 2, ' ', STR_PAD_LEFT); + } + echo "\n"; + } + } + } + + /** + * Deduplicate the given class + * + * @param string $class Class name to dedupe + * @param string $mappingTable Table name mapping + * @param string $relationField Name of foreign key relation field on mapping table + */ + protected function dedupe($class, $mappingTable, $relationField) + { + $this->printedDots = 0; + + // Find all duplicates + $itemTable = DataObject::getSchema()->tableName($class); + $duplicates = SQLSelect::create() + ->setSelect([ + 'Title' => '"Title"', + 'Count' => 'COUNT(*)', + 'UseID' => 'MIN("ID")', + ]) + ->setFrom($itemTable) + ->setGroupBy('"Title"') + ->setHaving('"Count" > 1'); + + $count = $duplicates->count(); + + $this->message("Found {$count} items with duplicates for type {$class}"); + + if (!$count) { + return; + } + + $done = 0; + foreach ($duplicates->execute() as $duplicate) { + $title = $duplicate['Title']; + $id = $duplicate['UseID']; + + DB::prepared_query(<<filter([ + 'Title' => $title, + 'ID:not' => $id, + ]); + /** @var DataObject $duplicateItem */ + foreach ($duplicateItems as $duplicateItem) { + $duplicateItem->delete(); + } + + // Update progress bar + $done++; + $this->progress($done, $count); + } + + $this->progress(""); + $this->progress("Completed cleaning duplicates for {$class}"); + } +} From 3772496f18d0c3cd1977734960a991a98beab030 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 15 Oct 2019 17:02:44 +1300 Subject: [PATCH 04/11] Null check for blog --- src/Model/BlogObject.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Model/BlogObject.php b/src/Model/BlogObject.php index 292efec..866840e 100644 --- a/src/Model/BlogObject.php +++ b/src/Model/BlogObject.php @@ -124,7 +124,8 @@ trait BlogObject return $extended; } - return $this->Blog()->canView($member); + $blog = $this->Blog(); + return $blog && $blog->canView($member); } /** @@ -158,7 +159,8 @@ trait BlogObject return $extended; } - return $this->Blog()->canDelete($member); + $blog = $this->Blog(); + return $blog && $blog->canDelete($member); } /** @@ -176,7 +178,8 @@ trait BlogObject return $extended; } - return $this->Blog()->canEdit($member); + $blog = $this->Blog(); + return $blog && $blog->canEdit($member); } protected function onBeforeWrite() From d815a4589d250c200ac4354b0cb8e0d7c7bf34cd Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 15 Oct 2019 17:06:17 +1300 Subject: [PATCH 05/11] PHPCS Linting fix --- src/Tasks/FixBlogDuplicatesTask.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Tasks/FixBlogDuplicatesTask.php b/src/Tasks/FixBlogDuplicatesTask.php index 45b905b..fe1462a 100644 --- a/src/Tasks/FixBlogDuplicatesTask.php +++ b/src/Tasks/FixBlogDuplicatesTask.php @@ -108,7 +108,8 @@ class FixBlogDuplicatesTask extends BuildTask $title = $duplicate['Title']; $id = $duplicate['UseID']; - DB::prepared_query(<< Date: Tue, 15 Oct 2019 17:37:06 +1300 Subject: [PATCH 06/11] Fix up tag cloud --- src/Admin/GridFieldCategorisationConfig.php | 11 +-- src/Model/Blog.php | 40 +++++------ src/Model/BlogObject.php | 18 +++++ src/Model/CategorisationObject.php | 7 +- src/Widgets/BlogTagsCloudWidget.php | 80 +++++++++------------ 5 files changed, 78 insertions(+), 78 deletions(-) diff --git a/src/Admin/GridFieldCategorisationConfig.php b/src/Admin/GridFieldCategorisationConfig.php index 6e9de8d..f2c5def 100644 --- a/src/Admin/GridFieldCategorisationConfig.php +++ b/src/Admin/GridFieldCategorisationConfig.php @@ -4,7 +4,6 @@ namespace SilverStripe\Blog\Admin; use SilverStripe\Blog\Forms\GridField\GridFieldAddByDBField; use SilverStripe\Blog\Model\CategorisationObject; -use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Forms\GridField\GridFieldAddNewButton; use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor; use SilverStripe\Forms\GridField\GridFieldDataColumns; @@ -18,9 +17,8 @@ class GridFieldCategorisationConfig extends GridFieldConfig_RecordEditor * @param string $parentType * @param string $parentMethod * @param string $childMethod - * @param SiteTree $parent */ - public function __construct($itemsPerPage, $mergeRecords, $parentType, $parentMethod, $childMethod, $parent) + public function __construct($itemsPerPage, $mergeRecords, $parentType, $parentMethod, $childMethod) { parent::__construct($itemsPerPage); @@ -41,11 +39,8 @@ class GridFieldCategorisationConfig extends GridFieldConfig_RecordEditor $columns->setFieldFormatting( [ - 'BlogPostsCount' => function ($value, CategorisationObject $item) use ($parent) { - return $item - ->BlogPosts() - ->filter(['ParentID' => $parent->ID]) - ->Count(); + 'BlogPostsCount' => function ($value, CategorisationObject $item) { + return $item->getBlogCount(); }, 'BlogPostsAllCount' => function ($value, CategorisationObject $item) { return $item->BlogPosts()->Count(); diff --git a/src/Model/Blog.php b/src/Model/Blog.php index eaf33a3..9c4cda0 100644 --- a/src/Model/Blog.php +++ b/src/Model/Blog.php @@ -135,17 +135,15 @@ class Blog extends Page implements PermissionProvider */ public function Tags($hideEmpty = true) { - $tags = BlogTag::get(); - if ($this->ID) { - $tags->setDataQueryParam('BlogID', $this->ID); + $tags = BlogTag::get()->setDataQueryParam('BlogID', $this->ID); - // Conditionally hide empty tags - if ($hideEmpty) { - $tags = $tags->filter([ - 'BlogPosts.ParentID' => $this->ID, - ]); - } + // Conditionally hide empty tags + if ($this->ID && $hideEmpty) { + $tags = $tags->filter([ + 'BlogPosts.ParentID' => $this->ID, + ]); } + $this->extend('updateBlogTags', $tags); return $tags; } @@ -158,17 +156,15 @@ class Blog extends Page implements PermissionProvider */ public function Categories($hideEmpty = true) { - $tags = BlogCategory::get(); - if ($this->ID) { - $tags->setDataQueryParam('BlogID', $this->ID); + $tags = BlogCategory::get()->setDataQueryParam('BlogID', $this->ID); - // Conditionally hide empty categories - if ($hideEmpty) { - $tags = $tags->filter([ - 'BlogPosts.ParentID' => $this->ID, - ]); - } + // Conditionally hide empty categories + if ($this->ID && $hideEmpty) { + $tags = $tags->filter([ + 'BlogPosts.ParentID' => $this->ID, + ]); } + $this->extend('updateBlogCategories', $tags); return $tags; } @@ -184,7 +180,7 @@ class Blog extends Page implements PermissionProvider if (!$this->canEdit()) { return; } - + $categories = GridField::create( 'Categories', _t(__CLASS__ . '.Categories', 'Categories'), @@ -194,8 +190,7 @@ class Blog extends Page implements PermissionProvider $this->Categories(false)->sort('Title'), BlogCategory::class, 'Categories', - 'BlogPosts', - $this + 'BlogPosts' ) ); @@ -208,8 +203,7 @@ class Blog extends Page implements PermissionProvider $this->Tags(false)->sort('Title'), BlogTag::class, 'Tags', - 'BlogPosts', - $this + 'BlogPosts' ) ); diff --git a/src/Model/BlogObject.php b/src/Model/BlogObject.php index 866840e..30c9b2a 100644 --- a/src/Model/BlogObject.php +++ b/src/Model/BlogObject.php @@ -67,6 +67,24 @@ trait BlogObject return $fields; } + /** + * Number of times this object has blog posts in the current blog + * + * @return int + */ + public function getBlogCount() + { + $blog = $this->Blog(); + if (!$blog) { + return 0; + } + + return $this + ->BlogPosts() + ->filter(['ParentID' => $blog->ID]) + ->Count(); + } + /** * {@inheritdoc} * @return ValidationResult diff --git a/src/Model/CategorisationObject.php b/src/Model/CategorisationObject.php index 3e29247..ad817b0 100644 --- a/src/Model/CategorisationObject.php +++ b/src/Model/CategorisationObject.php @@ -9,5 +9,10 @@ use SilverStripe\ORM\ManyManyList; */ interface CategorisationObject { - + /** + * Number of times this object has blog posts in the current blog + * + * @return int + */ + public function getBlogCount(); } diff --git a/src/Widgets/BlogTagsCloudWidget.php b/src/Widgets/BlogTagsCloudWidget.php index 4730e7e..c17a580 100644 --- a/src/Widgets/BlogTagsCloudWidget.php +++ b/src/Widgets/BlogTagsCloudWidget.php @@ -3,11 +3,10 @@ namespace SilverStripe\Blog\Widgets; use SilverStripe\Blog\Model\Blog; -use SilverStripe\Core\Convert; +use SilverStripe\Blog\Model\BlogTag; use SilverStripe\Forms\DropdownField; use SilverStripe\ORM\ArrayList; -use SilverStripe\ORM\DataObject; -use SilverStripe\ORM\DB; +use SilverStripe\View\ArrayData; use SilverStripe\Widgets\Model\Widget; if (!class_exists(Widget::class)) { @@ -73,53 +72,42 @@ class BlogTagsCloudWidget extends Widget } /** - * @return array + * @return ArrayList */ public function getTags() { - if ($blog = $this->Blog()) { - $escapedID = Convert::raw2sql($blog->ID); - $sql = 'SELECT DISTINCT "BlogTag"."URLSegment","BlogTag"."Title",Count("BlogTagID") AS "TagCount" - from "BlogPost_Tags" - INNER JOIN "BlogPost" - ON "BlogPost"."ID" = "BlogPost_Tags"."BlogPostID" - INNER JOIN "BlogTag" - ON "BlogTag"."ID" = "BlogPost_Tags"."BlogTagID" - WHERE "BlogID" = ' . $escapedID - . ' GROUP By "BlogTag"."URLSegment","BlogTag"."Title" - ORDER BY "Title"'; - - $records = DB::query($sql); - $bloglink = $blog->Link(); - $maxTagCount = 0; - - // create DataObjects that can be used to render the tag cloud - $tags = ArrayList::create(); - foreach ($records as $record) { - $tag = DataObject::create(); - $tag->TagName = $record['Title']; - $link = $bloglink.'tag/'.$record['URLSegment']; - $tag->Link = $link; - if ($record['TagCount'] > $maxTagCount) { - $maxTagCount = $record['TagCount']; - } - $tag->TagCount = $record['TagCount']; - $tags->push($tag); - } - - // normalize the tag counts from 1 to 10 - if ($maxTagCount) { - $tagfactor = 10 / $maxTagCount; - foreach ($tags->getIterator() as $tag) { - $normalized = round($tagfactor * ($tag->TagCount)); - $tag->NormalizedTag = $normalized; - } - } - - - return $tags; + // Check blog exists + $blog = $this->Blog(); + if (!$blog) { + return ArrayList::create([]); } - return []; + // create ArrayData that can be used to render the tag cloud + $maxTagCount = 0; + $tags = ArrayList::create(); + /** @var BlogTag $record */ + foreach ($blog->Tags() as $record) { + // Remember max count found + $count = $record->getBlogCount(); + $maxTagCount = $maxTagCount > $count ? $maxTagCount : $count; + + // Save + $tags->push(ArrayData::create([ + 'TagName' => $record->Title, + 'Link' => $record->getLink(), + 'TagCount' => $count, + ])); + } + + // normalize the tag counts from 1 to 10 + if ($maxTagCount) { + $tagfactor = 10 / $maxTagCount; + foreach ($tags->getIterator() as $tag) { + $normalized = round($tagfactor * ($tag->TagCount)); + $tag->NormalizedTag = $normalized; + } + } + + return $tags; } } From 82b7dab5747985972accf39d77ba05bba1fad7b5 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 16 Oct 2019 15:39:24 +1300 Subject: [PATCH 07/11] Fix BlogCategoryTest --- src/Model/BlogObject.php | 5 --- tests/BlogCategoryTest.php | 82 ++++++++++++++++++++++---------------- tests/blog.yml | 10 ----- 3 files changed, 47 insertions(+), 50 deletions(-) diff --git a/src/Model/BlogObject.php b/src/Model/BlogObject.php index 30c9b2a..8b0738e 100644 --- a/src/Model/BlogObject.php +++ b/src/Model/BlogObject.php @@ -97,11 +97,6 @@ trait BlogObject return $validation; } - $blog = $this->Blog(); - if (!$blog || !$blog->exists()) { - return $validation; - } - if ($this->getDuplicatesByField('Title')->count() > 0) { $validation->addError($this->getDuplicateError(), self::DUPLICATE_EXCEPTION); } diff --git a/tests/BlogCategoryTest.php b/tests/BlogCategoryTest.php index 1cc4aa0..a730123 100755 --- a/tests/BlogCategoryTest.php +++ b/tests/BlogCategoryTest.php @@ -5,7 +5,6 @@ namespace SilverStripe\Blog\Tests; use SilverStripe\Blog\Model\Blog; use SilverStripe\Blog\Model\BlogCategory; use SilverStripe\Blog\Model\BlogPost; -use SilverStripe\Blog\Model\BlogTag; use SilverStripe\Control\Controller; use SilverStripe\Dev\FunctionalTest; use SilverStripe\ORM\FieldType\DBDatetime; @@ -70,15 +69,19 @@ class BlogCategoryTest extends FunctionalTest */ public function testAllowMultibyteUrlSegment() { + /** @var Blog $blog */ $blog = $this->objFromFixture(Blog::class, 'FirstBlog'); + $cat = new BlogCategory(); - $cat->BlogID = $blog->ID; $cat->Title = 'تست'; $cat->write(); + + // urlencoded $this->assertEquals('%D8%AA%D8%B3%D8%AA', $cat->URLSegment); - $link = Controller::join_links($cat->Blog()->Link(), 'category', '%D8%AA%D8%B3%D8%AA'); - $this->assertEquals($link, $cat->getLink()); + $expectedLink = Controller::join_links($blog->Link('category'), '%D8%AA%D8%B3%D8%AA'); + $actualLink = $blog->Categories(false)->byID($cat->ID)->getLink(); + $this->assertEquals($expectedLink, $actualLink); } public function testCanView() @@ -88,8 +91,9 @@ class BlogCategoryTest extends FunctionalTest $this->objFromFixture(Member::class, 'Admin'); $editor = $this->objFromFixture(Member::class, 'Editor'); - $category = $this->objFromFixture(BlogCategory::class, 'SecondCategory'); - + /** @var Blog $secondBlog */ + $secondBlog = $this->objFromFixture(Blog::class, 'SecondBlog'); + $category = $secondBlog->Categories(false)->find('URLSegment', 'second-category'); $this->assertFalse($category->canView($editor), 'Editor should not be able to view category.'); } @@ -103,20 +107,26 @@ class BlogCategoryTest extends FunctionalTest $admin = $this->objFromFixture(Member::class, 'Admin'); $editor = $this->objFromFixture(Member::class, 'Editor'); - $category = $this->objFromFixture(BlogCategory::class, 'FirstCategory'); + /** @var Blog $firstBlog */ + $firstBlog = $this->objFromFixture(Blog::class, 'FirstBlog'); + $firstCategory = $firstBlog->Categories(false)->find('URLSegment', 'first-category'); - $this->assertTrue($category->canEdit($admin), 'Admin should be able to edit category.'); - $this->assertTrue($category->canEdit($editor), 'Editor should be able to edit category.'); + $this->assertTrue($firstCategory->canEdit($admin), 'Admin should be able to edit category.'); + $this->assertTrue($firstCategory->canEdit($editor), 'Editor should be able to edit category.'); - $category = $this->objFromFixture(BlogCategory::class, 'SecondCategory'); + /** @var Blog $secondBlog */ + $secondBlog = $this->objFromFixture(Blog::class, 'SecondBlog'); + $secondCategory = $secondBlog->Categories(false)->find('URLSegment', 'second-category'); - $this->assertTrue($category->canEdit($admin), 'Admin should be able to edit category.'); - $this->assertFalse($category->canEdit($editor), 'Editor should not be able to edit category.'); + $this->assertTrue($secondCategory->canEdit($admin), 'Admin should be able to edit category.'); + $this->assertFalse($secondCategory->canEdit($editor), 'Editor should not be able to edit category.'); - $category = $this->objFromFixture(BlogCategory::class, 'ThirdCategory'); + /** @var Blog $secondBlog */ + $thirdBlog = $this->objFromFixture(Blog::class, 'ThirdBlog'); + $thirdCategory = $thirdBlog->Categories(false)->find('URLSegment', 'third-category'); - $this->assertTrue($category->canEdit($admin), 'Admin should always be able to edit category.'); - $this->assertTrue($category->canEdit($editor), 'Editor should be able to edit category.'); + $this->assertTrue($thirdCategory->canEdit($admin), 'Admin should always be able to edit category.'); + $this->assertTrue($thirdCategory->canEdit($editor), 'Editor should be able to edit category.'); } public function testCanCreate() @@ -126,7 +136,7 @@ class BlogCategoryTest extends FunctionalTest $admin = $this->objFromFixture(Member::class, 'Admin'); $editor = $this->objFromFixture(Member::class, 'Editor'); - $category = singleton(BlogCategory::class); + $category = BlogCategory::singleton(); $this->assertTrue($category->canCreate($admin), 'Admin should be able to create category.'); $this->assertTrue($category->canCreate($editor), 'Editor should be able to create category.'); @@ -139,43 +149,45 @@ class BlogCategoryTest extends FunctionalTest $admin = $this->objFromFixture(Member::class, 'Admin'); $editor = $this->objFromFixture(Member::class, 'Editor'); - $category = $this->objFromFixture(BlogCategory::class, 'FirstCategory'); + /** @var Blog $firstBlog */ + $firstBlog = $this->objFromFixture(Blog::class, 'FirstBlog'); + $firstCategory = $firstBlog->Categories(false)->find('URLSegment', 'first-category'); - $this->assertTrue($category->canDelete($admin), 'Admin should be able to delete category.'); - $this->assertTrue($category->canDelete($editor), 'Editor should be able to category category.'); + $this->assertTrue($firstCategory->canDelete($admin), 'Admin should be able to delete category.'); + $this->assertTrue($firstCategory->canDelete($editor), 'Editor should be able to category category.'); - $category = $this->objFromFixture(BlogCategory::class, 'SecondCategory'); - $this->assertTrue($category->canDelete($admin), 'Admin should be able to delete category.'); - $this->assertFalse($category->canDelete($editor), 'Editor should not be able to delete category.'); + /** @var Blog $secondBlog */ + $secondBlog = $this->objFromFixture(Blog::class, 'SecondBlog'); + $secondCategory = $secondBlog->Categories(false)->find('URLSegment', 'second-category'); - $category = $this->objFromFixture(BlogCategory::class, 'ThirdCategory'); - $this->assertTrue($category->canDelete($admin), 'Admin should always be able to delete category.'); - $this->assertTrue($category->canDelete($editor), 'Editor should be able to delete category.'); + $this->assertTrue($secondCategory->canDelete($admin), 'Admin should be able to delete category.'); + $this->assertFalse($secondCategory->canDelete($editor), 'Editor should not be able to delete category.'); + + /** @var Blog $secondBlog */ + $thirdBlog = $this->objFromFixture(Blog::class, 'ThirdBlog'); + $thirdCategory = $thirdBlog->Categories(false)->find('URLSegment', 'third-category'); + + $this->assertTrue($thirdCategory->canDelete($admin), 'Admin should always be able to delete category.'); + $this->assertTrue($thirdCategory->canDelete($editor), 'Editor should be able to delete category.'); } public function testDuplicateCategories() { + $this->expectException(ValidationException::class); + $this->expectExceptionMessage('A blog category already exists with that name.'); + $blog = new Blog(); $blog->Title = 'Testing for duplicate categories'; $blog->write(); $category = new BlogCategory(); $category->Title = 'Test'; - $category->BlogID = $blog->ID; $category->URLSegment = 'test'; $category->write(); $category = new BlogCategory(); $category->Title = 'Test'; $category->URLSegment = 'test'; - $category->BlogID = $blog->ID; - try { - $category->write(); - $this->fail('Duplicate BlogCategory written'); - } catch (ValidationException $e) { - $messages = $e->getResult()->getMessages(); - $this->assertCount(1, $messages); - $this->assertEquals(BlogTag::DUPLICATE_EXCEPTION, $messages[0]['messageType']); - } + $category->write(); } } diff --git a/tests/blog.yml b/tests/blog.yml index 699395e..c7cd2f6 100755 --- a/tests/blog.yml +++ b/tests/blog.yml @@ -93,47 +93,37 @@ SilverStripe\Blog\Model\BlogCategory: FirstCategory: Title: 'First Category' URLSegment: 'first-category' - BlogID: =>SilverStripe\Blog\Model\Blog.FirstBlog SecondCategory: Title: 'Second Category' URLSegment: 'second-category' - BlogID: =>SilverStripe\Blog\Model\Blog.SecondBlog ThirdCategory: Title: 'Third Category' URLSegment: 'third-category' - BlogID: =>SilverStripe\Blog\Model\Blog.ThirdBlog SilverStripe\Blog\Model\BlogTag: FirstTag: Title: 'First Tag' URLSegment: 'first-tag' - BlogID: =>SilverStripe\Blog\Model\Blog.FirstBlog SecondTag: Title: 'Second Tag' URLSegment: 'second-tag' - BlogID: =>SilverStripe\Blog\Model\Blog.SecondBlog ThirdTag: Title: 'Third Tag' URLSegment: 'third-tag' - BlogID: =>SilverStripe\Blog\Model\Blog.ThirdBlog #Tags for Tag Cloud widget PopularTag: Title: 'Popular' URLSegment: 'popular' - BlogID: =>SilverStripe\Blog\Model\Blog.FourthBlog CoolTag: Title: 'Cool' URLSegment: 'cool' - BlogID: =>SilverStripe\Blog\Model\Blog.FourthBlog CatTag: Title: 'Cat' URLSegment: 'cat' - BlogID: =>SilverStripe\Blog\Model\Blog.FourthBlog KiwiTag: Title: 'Kiwi' URLSegment: 'kiwi' - BlogID: =>SilverStripe\Blog\Model\Blog.FourthBlog SilverStripe\Blog\Model\BlogPost: FirstBlogPost: From 91f89637bb7a6cb36ccfa52fe837ad57519f093f Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 25 Oct 2019 10:49:05 +1300 Subject: [PATCH 08/11] Fix blog controller tests Add blog setter / getter to tags and categories Fix remaining tests --- src/Model/BlogController.php | 12 +++- src/Model/BlogObject.php | 22 ++++++- tests/BlogTagTest.php | 64 ++++++++++++-------- tests/Model/BlogControllerFunctionalTest.yml | 8 --- 4 files changed, 72 insertions(+), 34 deletions(-) diff --git a/src/Model/BlogController.php b/src/Model/BlogController.php index e27a195..e9c529d 100644 --- a/src/Model/BlogController.php +++ b/src/Model/BlogController.php @@ -177,7 +177,9 @@ class BlogController extends PageController return (int)$this->urlParams['Year']; } - if ($this->urlParams['Action'] === 'archive') { + if (empty($this->urlParams['Year']) && + $this->urlParams['Action'] === 'archive' + ) { return DBDatetime::now()->Year(); } @@ -197,6 +199,10 @@ class BlogController extends PageController ? $this->urlParams['Month'] : null; + if (!$month) { + return null; + } + if (preg_match('/^[0-9]{1,2}$/', $month) && $month > 0 && $month < 13 @@ -220,6 +226,10 @@ class BlogController extends PageController ? $this->urlParams['Day'] : null; + if (!$day) { + return null; + } + // Cannot calculate day without month and year $month = $this->getArchiveMonth(); $year = $this->getArchiveYear(); diff --git a/src/Model/BlogObject.php b/src/Model/BlogObject.php index 8b0738e..91265cd 100644 --- a/src/Model/BlogObject.php +++ b/src/Model/BlogObject.php @@ -39,7 +39,7 @@ trait BlogObject */ public function Blog() { - $blogID = $this->getSourceQueryParam('BlogID'); + $blogID = $this->getBlogID(); if ($blogID) { /** @var Blog $blog */ $blog = Blog::get()->byID($blogID); @@ -195,6 +195,26 @@ trait BlogObject return $blog && $blog->canEdit($member); } + /** + * @return mixed + */ + public function getBlogID() + { + return $this->getSourceQueryParam('BlogID'); + } + + /** + * Set a blog ID for this record + * + * @param int $id + * @return $this + */ + public function setBlogID($id) + { + $this->setSourceQueryParam('BlogID', $id); + return $this; + } + protected function onBeforeWrite() { parent::onBeforeWrite(); diff --git a/tests/BlogTagTest.php b/tests/BlogTagTest.php index 88a48ff..d7705d2 100755 --- a/tests/BlogTagTest.php +++ b/tests/BlogTagTest.php @@ -82,15 +82,19 @@ class BlogTagTest extends FunctionalTest $admin = $this->objFromFixture(Member::class, 'Admin'); $editor = $this->objFromFixture(Member::class, 'Editor'); - $tag = $this->objFromFixture(BlogTag::class, 'FirstTag'); + /** @var Blog $firstBlog */ + $firstBlog = $this->objFromFixture(Blog::class, 'FirstBlog'); + $firstTag = $firstBlog->Tags(false)->find('URLSegment', 'first-tag'); - $this->assertTrue($tag->canView($admin), 'Admin should be able to view tag.'); - $this->assertTrue($tag->canView($editor), 'Editor should be able to view tag.'); + $this->assertTrue($firstTag->canView($admin), 'Admin should be able to view tag.'); + $this->assertTrue($firstTag->canView($editor), 'Editor should be able to view tag.'); - $tag = $this->objFromFixture(BlogTag::class, 'SecondTag'); + /** @var Blog $secondBlog */ + $secondBlog = $this->objFromFixture(Blog::class, 'SecondBlog'); + $secondTag = $secondBlog->Tags(false)->find('URLSegment', 'second-tag'); - $this->assertTrue($tag->canView($admin), 'Admin should be able to view tag.'); - $this->assertFalse($tag->canView($editor), 'Editor should not be able to view tag.'); + $this->assertTrue($secondTag->canView($admin), 'Admin should be able to view tag.'); + $this->assertFalse($secondTag->canView($editor), 'Editor should not be able to view tag.'); } public function testCanEdit() @@ -100,20 +104,26 @@ class BlogTagTest extends FunctionalTest $admin = $this->objFromFixture(Member::class, 'Admin'); $editor = $this->objFromFixture(Member::class, 'Editor'); - $tag = $this->objFromFixture(BlogTag::class, 'FirstTag'); + /** @var Blog $firstBlog */ + $firstBlog = $this->objFromFixture(Blog::class, 'FirstBlog'); + $firstTag = $firstBlog->Tags(false)->find('URLSegment', 'first-tag'); - $this->assertTrue($tag->canEdit($admin), 'Admin should be able to edit tag.'); - $this->assertTrue($tag->canEdit($editor), 'Editor should be able to edit tag.'); + $this->assertTrue($firstTag->canEdit($admin), 'Admin should be able to edit tag.'); + $this->assertTrue($firstTag->canEdit($editor), 'Editor should be able to edit tag.'); - $tag = $this->objFromFixture(BlogTag::class, 'SecondTag'); + /** @var Blog $secondBlog */ + $secondBlog = $this->objFromFixture(Blog::class, 'SecondBlog'); + $secondTag = $secondBlog->Tags(false)->find('URLSegment', 'second-tag'); - $this->assertTrue($tag->canEdit($admin), 'Admin should be able to edit tag.'); - $this->assertFalse($tag->canEdit($editor), 'Editor should not be able to edit tag.'); + $this->assertTrue($secondTag->canEdit($admin), 'Admin should be able to edit tag.'); + $this->assertFalse($secondTag->canEdit($editor), 'Editor should not be able to edit tag.'); - $tag = $this->objFromFixture(BlogTag::class, 'ThirdTag'); + /** @var Blog $thirdBlog */ + $thirdBlog = $this->objFromFixture(Blog::class, 'ThirdBlog'); + $thirdTag = $thirdBlog->Tags(false)->find('URLSegment', 'third-tag'); - $this->assertTrue($tag->canEdit($admin), 'Admin should always be able to edit tags.'); - $this->assertTrue($tag->canEdit($editor), 'Editor should be able to edit tag.'); + $this->assertTrue($thirdTag->canEdit($admin), 'Admin should always be able to edit tags.'); + $this->assertTrue($thirdTag->canEdit($editor), 'Editor should be able to edit tag.'); } public function testCanCreate() @@ -136,20 +146,26 @@ class BlogTagTest extends FunctionalTest $admin = $this->objFromFixture(Member::class, 'Admin'); $editor = $this->objFromFixture(Member::class, 'Editor'); - $tag = $this->objFromFixture(BlogTag::class, 'FirstTag'); + /** @var Blog $firstBlog */ + $firstBlog = $this->objFromFixture(Blog::class, 'FirstBlog'); + $firstTag = $firstBlog->Tags(false)->find('URLSegment', 'first-tag'); - $this->assertTrue($tag->canDelete($admin), 'Admin should be able to delete tag.'); - $this->assertTrue($tag->canDelete($editor), 'Editor should be able to delete tag.'); + $this->assertTrue($firstTag->canDelete($admin), 'Admin should be able to delete tag.'); + $this->assertTrue($firstTag->canDelete($editor), 'Editor should be able to delete tag.'); - $tag = $this->objFromFixture(BlogTag::class, 'SecondTag'); + /** @var Blog $secondBlog */ + $secondBlog = $this->objFromFixture(Blog::class, 'SecondBlog'); + $secondTag = $secondBlog->Tags(false)->find('URLSegment', 'second-tag'); - $this->assertTrue($tag->canDelete($admin), 'Admin should be able to delete tag.'); - $this->assertFalse($tag->canDelete($editor), 'Editor should not be able to delete tag.'); + $this->assertTrue($secondTag->canDelete($admin), 'Admin should be able to delete tag.'); + $this->assertFalse($secondTag->canDelete($editor), 'Editor should not be able to delete tag.'); - $tag = $this->objFromFixture(BlogTag::class, 'ThirdTag'); + /** @var Blog $thirdBlog */ + $thirdBlog = $this->objFromFixture(Blog::class, 'ThirdBlog'); + $thirdTag = $thirdBlog->Tags(false)->find('URLSegment', 'third-tag'); - $this->assertTrue($tag->canDelete($admin), 'Admin should always be able to delete tags.'); - $this->assertTrue($tag->canDelete($editor), 'Editor should be able to delete tag.'); + $this->assertTrue($thirdTag->canDelete($admin), 'Admin should always be able to delete tags.'); + $this->assertTrue($thirdTag->canDelete($editor), 'Editor should be able to delete tag.'); } public function testDuplicateTagsForURLSegment() diff --git a/tests/Model/BlogControllerFunctionalTest.yml b/tests/Model/BlogControllerFunctionalTest.yml index c29c0e3..c91b913 100644 --- a/tests/Model/BlogControllerFunctionalTest.yml +++ b/tests/Model/BlogControllerFunctionalTest.yml @@ -12,10 +12,6 @@ SilverStripe\Blog\Model\Blog: blog_a: URLSegment: my-blog Title: My Blog - Categories: - - =>SilverStripe\Blog\Model\BlogCategory.category_a - Tags: - - =>SilverStripe\Blog\Model\BlogTag.tag_a SilverStripe\Blog\Model\BlogPost: blogpost_a: @@ -23,7 +19,3 @@ SilverStripe\Blog\Model\BlogPost: URLSegment: آبیدآبید PublishDate: 2017-08-01 00:00:00 Parent: =>SilverStripe\Blog\Model\Blog.blog_a - Categories: - - =>SilverStripe\Blog\Model\BlogCategory.category_a - Tags: - - =>SilverStripe\Blog\Model\BlogTag.tag_a From 5ef99a76a972c65cb9caac0a7f58558c6036d497 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 25 Oct 2019 11:24:54 +1300 Subject: [PATCH 09/11] Fix unit test --- tests/BlogTagTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/BlogTagTest.php b/tests/BlogTagTest.php index d7705d2..c0d5324 100755 --- a/tests/BlogTagTest.php +++ b/tests/BlogTagTest.php @@ -61,14 +61,15 @@ class BlogTagTest extends FunctionalTest */ public function testAllowMultibyteUrlSegment() { + /** @var Blog $blog */ $blog = $this->objFromFixture(Blog::class, 'FirstBlog'); $tag = new BlogTag(); - $tag->BlogID = $blog->ID; + $tag->setBlogID($blog->ID); $tag->Title = 'تست'; $tag->write(); // urlencoded $this->assertEquals('%D8%AA%D8%B3%D8%AA', $tag->URLSegment); - $link = Controller::join_links($tag->Blog()->Link(), 'tag', '%D8%AA%D8%B3%D8%AA'); + $link = Controller::join_links($blog->Link(), 'tag', '%D8%AA%D8%B3%D8%AA'); $this->assertEquals($link, $tag->getLink()); } From 1ba72c74cefae15c18cf9d47dd85e40d4e67edb8 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 25 Oct 2019 11:40:02 +1300 Subject: [PATCH 10/11] Default sort for tags / categories --- src/Model/BlogCategory.php | 5 +++++ src/Model/BlogTag.php | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/Model/BlogCategory.php b/src/Model/BlogCategory.php index 7bbfdf5..cbe9c4e 100644 --- a/src/Model/BlogCategory.php +++ b/src/Model/BlogCategory.php @@ -53,6 +53,11 @@ class BlogCategory extends DataObject implements CategorisationObject 'BlogPosts' => BlogPost::class, ]; + /** + * @var string + */ + private static $default_sort = '"BlogCategory"."Title" ASC'; + /** * {@inheritdoc} */ diff --git a/src/Model/BlogTag.php b/src/Model/BlogTag.php index 2399880..64eb804 100644 --- a/src/Model/BlogTag.php +++ b/src/Model/BlogTag.php @@ -56,6 +56,11 @@ class BlogTag extends DataObject implements CategorisationObject 'BlogPosts' => BlogPost::class ]; + /** + * @var string + */ + private static $default_sort = '"BlogTag"."Title" ASC'; + /** * {@inheritdoc} */ From 3fa3093d969ff2977eb6bb503f3d796b743da7bd Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 26 Mar 2020 15:36:09 +1300 Subject: [PATCH 11/11] Remove redundant code --- src/Model/BlogPost.php | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/src/Model/BlogPost.php b/src/Model/BlogPost.php index d03b0d5..77f36e4 100644 --- a/src/Model/BlogPost.php +++ b/src/Model/BlogPost.php @@ -490,32 +490,6 @@ class BlogPost extends Page } } - /** - * {@inheritdoc} - * - * Sets blog relationship on all categories and tags assigned to this post. - */ - public function onAfterWrite() - { - parent::onAfterWrite(); - - foreach ($this->Categories() as $category) { - /** - * @var BlogCategory $category - */ - $category->BlogID = $this->ParentID; - $category->write(); - } - - foreach ($this->Tags() as $tag) { - /** - * @var BlogTag $tag - */ - $tag->BlogID = $this->ParentID; - $tag->write(); - } - } - /** * {@inheritdoc} */