diff --git a/composer.json b/composer.json index 2711665..2d128b0 100644 --- a/composer.json +++ b/composer.json @@ -15,10 +15,11 @@ "slack": "https://silverstripe-users.slack.com" }, "require": { - "silverstripe/framework": "^4||^5" + "php": "^8.3", + "silverstripe/framework": "^6" }, "require-dev": { - "phpunit/phpunit": "^5.7", + "phpunit/phpunit": "^11", "squizlabs/php_codesniffer": "^3.0" }, "autoload": { diff --git a/src/Slug.php b/src/Slug.php index 9f19ebe..c19ca2d 100644 --- a/src/Slug.php +++ b/src/Slug.php @@ -3,101 +3,82 @@ namespace Nightjar\Slug; use InvalidArgumentException; -use UnexpectedValueException; -use SilverStripe\ORM\DataObject; -use SilverStripe\Forms\FieldList; -use SilverStripe\ORM\DataExtension; +use Override; use SilverStripe\Control\Controller; +use SilverStripe\Core\Extension; +use SilverStripe\Forms\FieldList; +use SilverStripe\ORM\DataObject; use SilverStripe\View\Parsers\URLSegmentFilter; +use UnexpectedValueException; /** * Adds a 'url slug' property to DataObject classes, in order for them to be able to be loaded via a URL - * through a{@see SilverStripe\Control\Controller}. The typical use case for this is to view records related - * to a {@see Page}, to be able to have a controller action that doesn't need to reference the ID ofthe record. + * through a{@see \SilverStripe\Control\Controller}. The typical use case for this is to view records related + * to a {@see Page}, to be able to have a controller action that doesn't need to reference the ID of the record. * E.g. /products/nice-jacket - where this is /pageType/relatedObject-notAPage * * To simplify the controller section of this purpose {@see SlugHandler} */ -class Slug extends DataExtension +class Slug extends Extension { /** * Used to set {@see $active} to inactive, or 'link' */ - const ACTIVE_NONE = null; + const null ACTIVE_NONE = null; /** * Used to set and check {@see $active} as a section (ancestor of 'current') */ - const ACTIVE_SECTION = false; + const false ACTIVE_SECTION = false; /** * Used to set and check {@see $active} as the current object active in a request */ - const ACTIVE_CURRENT = true; + const true ACTIVE_CURRENT = true; - private static $db = [ + private static array $db = [ 'URLSlug' => 'Varchar(255)', ]; - private static $indexes = [ + private static array $indexes = [ 'URLSlug' => true, ]; - /** - * The field on the owner that we should take the value from in order to generate a slug - * - * @var string - */ - protected $fieldToSlug; - - /** - * If we should restrict the slugging uniqueness to a certain subset of the owner class, - * this will be the name of the relation to filter by to detect uniqueness - * - * @var string|null - */ - protected $relationName; - - /** - * Whether or not we should update the URLSlug field when the field to slug changes - * - * @var boolean - */ - protected $enforceParity; - /** * The owner has been accessed via a route involving the URLSlug * Tri state; current, section, none * * @var null|boolean {@see setSlugActive} */ - protected $active = null; + protected ?bool $active = null; /** * Apply extension with configurable defaults * - * @param string $fieldToSlug The field on the owner to base the URL Slug from - defaults to 'Title' - * @param string $relationName Optional name of the has_one relationship to the owner's parent class/Page + * @param string $fieldToSlug The field on the owner that we should take the value from to generate a slug - defaults to 'Title' + * @param string $relationName Optional name of the has_one relationship to the owner's parent class/Page * @param boolean $enforceParity true to alter the URLSlug whenever the $fieldToSlug changes value (default: false) */ - public function __construct($fieldToSlug = 'Title', $relationName = null, $enforceParity = false) - { + public function __construct( + protected $fieldToSlug = 'Title', + /** + * If we should restrict the slugging uniqueness to a certain subset of the owner class, + * this will be the name of the relation to filter by to detect uniqueness + */ + protected $relationName = null, + protected $enforceParity = false + ) { parent::__construct(); - $this->fieldToSlug = $fieldToSlug; - $this->relationName = $relationName; - $this->enforceParity = $enforceParity; } + #[Override] public function setOwner($owner) { - // parent method is set in a try, with a finally to clearOwner - so it is important we set it first, - // otherwise our InvalidArgumentException will be swallowed by a BadMethodCallException! parent::setOwner($owner); - // throw an exception if the $relationName is invalid or not has_one - if ($this->relationName) { - $ownerClass = get_class($owner); - $valid = DataObject::getSchema()->hasOneComponent($ownerClass, $this->relationName); + if ($this->relationName && $owner) { + $ownerClass = $owner::class; + $valid = DataObject::getSchema()?->hasOneComponent($ownerClass, $this->relationName); if (!$valid) { throw new InvalidArgumentException("$this->relationName is an invalid has_one on $ownerClass"); } @@ -134,7 +115,7 @@ public function setSlugActive($active) /** * Generate a url slug segment * - * @param boolean $forceRegeneration + * @param boolean $forceRegeneration * @return string */ public function getSlug($forceRegeneration = false) @@ -166,13 +147,13 @@ public function onBeforeWrite() if ($updateSlug) { $owner->URLSlug = $this->getSlug($this->enforceParity); - $collisionList = DataObject::get(get_class($owner))->exclude('ID', $owner->ID); + $collisionList = DataObject::get($owner::class)->exclude(['ID' => $owner->ID]); $filter = ['URLSlug' => $owner->URLSlug]; if ($this->relationName) { $parentIDField = $this->relationName . 'ID'; $filter[$parentIDField] = $owner->$parentIDField; // Also handle polymorphic relationships - $parentClassName = DataObject::getSchema()->hasOneComponent(get_class($owner), $this->relationName); + $parentClassName = DataObject::getSchema()->hasOneComponent($owner::class, $this->relationName); if ($parentClassName === DataObject::class) { $parentClassField = $this->relationName . 'Class'; $filter[$parentClassField] = $owner->$parentClassField; @@ -181,7 +162,7 @@ public function onBeforeWrite() $count = 1; while ($collisionList->filter($filter)->exists()) { - $owner->URLSlug = $owner->URLSlug . $count++; + $owner->URLSlug .= $count++; $filter['URLSlug'] = $owner->URLSlug; } } elseif ($slugHasChanged) { @@ -218,11 +199,11 @@ public function Link($action = null) $link = null; $owner = $this->getOwner(); $action = ($action) ? Controller::join_links($owner->URLSlug, $action) : $owner->URLSlug; - + $relationName = $this->relationName; if ($relationName && ($parent = $owner->$relationName()) && $parent->hasMethod('Link')) { $link = $parent->Link($action); - } elseif (Controller::has_curr()) { + } elseif (Controller::curr() === null) { // Quite the assumption, but sufficient in most cases. $link = Controller::curr()->Link($action); } @@ -282,10 +263,12 @@ public function LinkingMode() { if ($this->isCurrent()) { return 'current'; - } elseif ($this->isSection()) { + } + + if ($this->isSection()) { return 'section'; - } else { - return 'link'; } + + return 'link'; } } diff --git a/src/SlugHandler.php b/src/SlugHandler.php index 43c6d07..5daad85 100644 --- a/src/SlugHandler.php +++ b/src/SlugHandler.php @@ -3,18 +3,18 @@ namespace Nightjar\Slug; use LogicException; -use SilverStripe\Core\Extension; -use SilverStripe\ORM\DataObject; +use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse_Exception; +use SilverStripe\Core\Extension; /** * This is for handling a request for a slugged DataObject, and should be applied to a Controller. * * One can either supply the method name (to get slugs from) as a string to the constructor, - * or define a slug_trails {@see SilverStripe\Core\Config} property on the Controller. + * or define a slug_trails {@see \SilverStripe\Core\Config} property on the Controller. * The latter allows support for multiple slugged relationships to reside on the same - * {@see SilverStripe\ORM\DataObject}, and the `slug_trails` property is an array map + * {@see \SilverStripe\ORM\DataObject}, and the `slug_trails` property is an array map * in the format of * [ $Trail => $RelationshipName, ... ] * where $Trail is the URL segment {@see $url_handlers} it is loaded over, @@ -51,14 +51,7 @@ class SlugHandler extends Extension protected $slugs; /** - * Which function on the Controller will get us the initial DataObject? - * - * @var array - */ - protected $dataSource; - - /** - * Apply extension to {@see SilverStripe\Control\Controller}. + * Apply extension to {@see Controller}. * Supply a parameter as a shorthand if handling multiple slugs on the same object is unneeded. * Also takes the name of the initial getter function used to move from Controller to Model, * the default is set to 'getFailover' as this is nicely generic - so if this property is not set @@ -66,13 +59,12 @@ class SlugHandler extends Extension * 'bare' and getter methods exist directly on it for each slugged object type (with no parent relation). * * @param string $relationship Relationship name (optional) - * @param string $dataSource getter function name + * @param string $dataSource getter function name; Which function on the Controller will get us the initial DataObject? */ - public function __construct($relationship = null, $dataSource = 'getFailover') + public function __construct($relationship = null, protected $dataSource = 'getFailover') { parent::__construct(); $this->slugs = $relationship ? ['' => $relationship] : null; - $this->dataSource = $dataSource; } /** @@ -84,15 +76,15 @@ public function __construct($relationship = null, $dataSource = 'getFailover') */ protected function findSlug() { - /** @var SilverStripe\Control\Controller */ + /** @var Controller */ $owner = $this->getOwner(); $request = $owner->getRequest(); - + // You're probably wondering about these variable names... $plot = $this->dataSource; $garden = $owner->$plot(); if (!$garden) { - $mrMcGregors = get_class($owner); + $mrMcGregors = $owner::class; throw new LogicException("There is no garden in $mrMcGregors::$plot() to find Slugs in!"); } $slime = $this->slugs; diff --git a/tests/SlugHandlerTest.php b/tests/SlugHandlerTest.php index 3eae4e9..a46e154 100644 --- a/tests/SlugHandlerTest.php +++ b/tests/SlugHandlerTest.php @@ -2,8 +2,8 @@ namespace Nightjar\Slug\Tests; +use Override; use Nightjar\Slug\Slug; -use InvalidArgumentException; use SilverStripe\Control\Director; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\FunctionalTest; @@ -33,20 +33,18 @@ class SlugHandlerTest extends FunctionalTest * fixture classes, and here we are also testing defining a service to apply the extension works. * This is important for e.g. backwards compatiblity aliases */ + #[Override] public static function getExtraDataObjects() { $config = Config::modify(); - $config->set(Injector::class, 'JournalistSlug', [ - 'class' => Slug::class, - 'constructor' => ['Name', 'NewsPages'], + $config->set(Journalist::class, 'extensions', [ + Slug::class . '("Name")' ]); - $config->merge(Journalist::class, 'extensions', ['JournalistSlug']); - // Do what we actually came here for return parent::getExtraDataObjects(); } - protected function setUp(): void + protected function setUp(): void { parent::setUp(); $config = Config::modify(); @@ -73,21 +71,21 @@ public function testRequestingSlugs() $news->Journalists()->add($jim); $output = $this->get('news/'); - $this->assertEquals('Index ok', $output->getBody()); + $this->assertEquals('Index ok', trim((string) $output->getBody())); $output = $this->get('news/nonsense/first-news'); $this->assertEquals(404, $output->getStatusCode()); $output = $this->get('news/stories/'); $this->assertEquals(404, $output->getStatusCode()); $output = $this->get('news/stories/first-news'); - $this->assertEquals('First News', $output->getBody()); + $this->assertEquals('First News', trim((string) $output->getBody())); $output = $this->get('news/first-news'); $this->assertEquals(404, $output->getStatusCode()); $output = $this->get('news/contributors/'); $this->assertEquals(404, $output->getStatusCode()); $output = $this->get('news/contributors/jimbo-the-journo'); - $this->assertEquals('Jimbo the Journo', $output->getBody()); + $this->assertEquals('Jimbo the Journo', trim((string) $output->getBody())); $output = $this->get('news/jimbo-the-journo'); $this->assertEquals(404, $output->getStatusCode()); diff --git a/tests/SlugTest.php b/tests/SlugTest.php index f623967..fe20d0d 100644 --- a/tests/SlugTest.php +++ b/tests/SlugTest.php @@ -2,16 +2,15 @@ namespace Nightjar\Slug\Tests; -use Nightjar\Slug\Slug; use InvalidArgumentException; -use UnexpectedValueException; -use SilverStripe\Dev\SapphireTest; -use SilverStripe\Core\Config\Config; +use Nightjar\Slug\Slug; use Nightjar\Slug\Tests\Stubs\Article; use Nightjar\Slug\Tests\Stubs\Blitzem; -use Nightjar\Slug\Tests\Stubs\NewsPage; -use SilverStripe\Core\Injector\Injector; use Nightjar\Slug\Tests\Stubs\Journalist; +use Nightjar\Slug\Tests\Stubs\NewsPage; +use SilverStripe\Core\Config\Config; +use SilverStripe\Dev\SapphireTest; +use UnexpectedValueException; class SlugTest extends SapphireTest { @@ -53,18 +52,16 @@ public function testCannotAssociateToInvalidRelationshipType() $this->expectException(InvalidArgumentException::class); // Try to go through the Blitzem to get to the tasty Lettuce! - Blitzem::create(); + //we need to extend onBeforeWrite to ensure setOwner() is called + Blitzem::create()->extend('onBeforeWrite'); } public function testSlugsWillSetAndSanitiseOnSave() { $config = Config::modify(); - $config->set(Injector::class, 'JournalistSlug', [ - 'class' => Slug::class, - 'constructor' => ['Name', 'NewsPages'], + $config->set(Journalist::class, 'extensions', [ + Slug::class . '("Name", "NewsPages")' ]); - $config->merge(Journalist::class, 'extensions', ['JournalistSlug']); - $journo = Journalist::create(); $journo->update(['Name' => 'Ash Katchum!'])->extend('onBeforeWrite'); $this->assertEquals('ash-katchum', $journo->URLSlug, 'initial write should sanitise'); @@ -76,7 +73,7 @@ public function testSlugsWillSetAndSanitiseOnSave() public function testSlugKeepsParity() { $newArticle = Article::create(); - + $newArticle->update(['Title' => 'Second News'])->fakeWrite(); $this->assertEquals('second-news', $newArticle->URLSlug); diff --git a/tests/SlugTest.yml b/tests/SlugTest.yml index a4e2edf..698d6ac 100644 --- a/tests/SlugTest.yml +++ b/tests/SlugTest.yml @@ -1,5 +1,6 @@ Nightjar\Slug\Tests\Stubs\NewsPage: holder: + Title: News Holder Nightjar\Slug\Tests\Stubs\Article: one: Title: 'First News' diff --git a/tests/Stubs/Article.php b/tests/Stubs/Article.php index f921ca6..d2c8c4e 100644 --- a/tests/Stubs/Article.php +++ b/tests/Stubs/Article.php @@ -5,10 +5,11 @@ use ReflectionProperty; use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; -use SilverStripe\Control\Controller; class Article extends DataObject implements TestOnly { + private static $table_name = 'Article'; + private static $extensions = [ 'Nightjar\Slug\Slug("Title", "Parent", true)' ]; @@ -30,7 +31,6 @@ public function fakeWrite() { if (!$this->reflectedChanges) { $this->reflectedChanges = new ReflectionProperty(DataObject::class, 'changed'); - $this->reflectedChanges->setAccessible(true); } $this->onBeforeWrite(); $this->reflectedChanges->setValue($this, []); diff --git a/tests/Stubs/Blitzem.php b/tests/Stubs/Blitzem.php index acd6c0f..967de86 100644 --- a/tests/Stubs/Blitzem.php +++ b/tests/Stubs/Blitzem.php @@ -2,12 +2,15 @@ namespace Nightjar\Slug\Tests\Stubs; +use Override; use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; use Nightjar\Slug\Tests\Stubs\Lettuce; class Blitzem extends DataObject implements TestOnly { + private static $table_name = 'Blitzem'; + // extension applied in test set with params ("Title", "Protects") // as it errors on boot otherwise. @@ -15,6 +18,7 @@ class Blitzem extends DataObject implements TestOnly 'Protects' => Lettuce::class ]; + #[Override] public function getTitle() { return 'No slugs!'; diff --git a/tests/Stubs/Journalist.php b/tests/Stubs/Journalist.php index 5a83081..ed0838a 100644 --- a/tests/Stubs/Journalist.php +++ b/tests/Stubs/Journalist.php @@ -7,6 +7,8 @@ class Journalist extends DataObject implements TestOnly { + private static $table_name = 'Journalist'; + // Slug extension applied in test setUp, so we can test different instantiation methods. private static $db = [ diff --git a/tests/Stubs/Lettuce.php b/tests/Stubs/Lettuce.php index b6e79af..0a4275f 100644 --- a/tests/Stubs/Lettuce.php +++ b/tests/Stubs/Lettuce.php @@ -7,6 +7,8 @@ class Lettuce extends DataObject implements TestOnly { + private static $table_name = 'Lettuce'; + private static $many_many = [ 'Protection' => Blitzem::class ]; diff --git a/tests/Stubs/NewsController.php b/tests/Stubs/NewsController.php index c3b5d10..e39c441 100644 --- a/tests/Stubs/NewsController.php +++ b/tests/Stubs/NewsController.php @@ -2,9 +2,9 @@ namespace Nightjar\Slug\Tests\Stubs; +use Override; use Nightjar\Slug\SlugHandler; use SilverStripe\Dev\TestOnly; -use SilverStripe\View\SSViewer; use SilverStripe\Control\Controller; class NewsController extends Controller implements TestOnly @@ -20,18 +20,13 @@ class NewsController extends Controller implements TestOnly 'contributors' => 'Journalists', ]; + #[Override] public function getViewer($action) { - $testTemplate = '' . - '<% if $ActiveSlug %>' . - // using Name as Title is a default fallback in framework - '$ActiveSlug.Title' . - '<% else %>' . - 'Index ok' . - '<% end_if %>'; - return SSViewer::fromString($testTemplate); + return parent::getViewer($action); } + #[Override] public function Link($action = null) { return 'news/'; diff --git a/tests/Stubs/NewsPage.php b/tests/Stubs/NewsPage.php index 4ca40c0..12e9270 100644 --- a/tests/Stubs/NewsPage.php +++ b/tests/Stubs/NewsPage.php @@ -8,9 +8,11 @@ class NewsPage extends DataObject implements TestOnly { + private static $table_name = 'NewsPage'; + private static $has_many = [ 'Articles' => Article::class, - 'Journalists' => Journalist::class, + 'Journalists' => Journalist::class . '.NewsPages', ]; public function Link($action = null) diff --git a/tests/templates/Nightjar/Slug/Tests/Stubs/NewsController.ss b/tests/templates/Nightjar/Slug/Tests/Stubs/NewsController.ss new file mode 100644 index 0000000..9f8d65d --- /dev/null +++ b/tests/templates/Nightjar/Slug/Tests/Stubs/NewsController.ss @@ -0,0 +1 @@ +<% if $ActiveSlug %>$ActiveSlug.Title<% else %>Index ok<% end_if %>