From 28b833fc4e009718a44ae3e4f154ee059357820f Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Mon, 12 Feb 2024 17:28:48 +1300 Subject: [PATCH] ENH Standardise naming and improve code quality --- src/Form/AbstractLinkField.php | 23 +++++++------------ src/Models/ExternalLink.php | 3 +-- src/Models/PhoneLink.php | 1 - .../SilverStripe/LinkField/Form/LinkField.ss | 2 +- .../LinkField/Form/MultiLinkField.ss | 2 +- .../Traits/AllowedLinkClassesTraitTest.php | 16 ++++++------- 6 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/Form/AbstractLinkField.php b/src/Form/AbstractLinkField.php index 7d44cf7f..3de5fa3a 100644 --- a/src/Form/AbstractLinkField.php +++ b/src/Form/AbstractLinkField.php @@ -24,7 +24,7 @@ abstract class AbstractLinkField extends FormField protected $inputType = 'hidden'; - private array $allowed_types = []; + private array $allowedTypes = []; private bool $excludeLinkTextField = false; @@ -45,10 +45,8 @@ public function getExcludeLinkTextField(): bool */ public function setAllowedTypes(array $types): static { - if ($this->validateTypes($types)) { - $this->allowed_types = $types; - } - + $this->validateTypes($types); + $this->allowedTypes = $types; return $this; } @@ -57,7 +55,7 @@ public function setAllowedTypes(array $types): static */ public function getAllowedTypes(): array { - return $this->allowed_types; + return $this->allowedTypes; } /** @@ -66,7 +64,7 @@ public function getAllowedTypes(): array * for full-fledged work on the client side. * @throws InvalidArgumentException */ - public function getTypesProps(): string + public function getTypesProp(): string { $typesList = []; $typeDefinitions = $this->generateAllowedTypes(); @@ -108,7 +106,7 @@ public function performDisabledTransformation(): FormField public function getSchemaDataDefaults(): array { $data = parent::getSchemaDataDefaults(); - $data['types'] = json_decode($this->getTypesProps()); + $data['types'] = json_decode($this->getTypesProp()); $data['excludeLinkTextField'] = $this->getExcludeLinkTextField(); $ownerFields = $this->getOwnerFields(); $data['ownerID'] = $ownerFields['ID']; @@ -200,7 +198,7 @@ private function generateAllowedTypes(): array * @param string[] $types * @throws InvalidArgumentException */ - private function validateTypes(array $types): bool + private function validateTypes(array $types): void { if (empty($types)) { throw new InvalidArgumentException( @@ -212,11 +210,8 @@ private function validateTypes(array $types): bool ); } - $validClasses = []; foreach ($types as $type) { - if (is_subclass_of($type, Link::class)) { - $validClasses[] = $type; - } else { + if (!is_subclass_of($type, Link::class)) { throw new InvalidArgumentException( _t( __TRAIT__ . '.INVALID_TYPECLASS', @@ -231,7 +226,5 @@ private function validateTypes(array $types): bool ); } } - - return count($validClasses) > 0; } } diff --git a/src/Models/ExternalLink.php b/src/Models/ExternalLink.php index 151fcc11..a382981f 100644 --- a/src/Models/ExternalLink.php +++ b/src/Models/ExternalLink.php @@ -30,8 +30,7 @@ class ExternalLink extends Link public function getCMSFields(): FieldList { $this->beforeUpdateCMSFields(function (FieldList $fields) { - $field = UrlField::create('ExternalUrl'); - $field->setTitle(_t(__CLASS__ . '.EXTERNAL_URL_FIELD', 'External URL')); + $field = UrlField::create('ExternalUrl', _t(__CLASS__ . '.EXTERNAL_URL_FIELD', 'External URL')); $field->setDescription(_t( __CLASS__ . '.EXTERNAL_URL_FIELD_DESCRIPTION', 'The URL must start with either http:// or https://' diff --git a/src/Models/PhoneLink.php b/src/Models/PhoneLink.php index a509c4ab..7ce981b6 100644 --- a/src/Models/PhoneLink.php +++ b/src/Models/PhoneLink.php @@ -5,7 +5,6 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\CompositeValidator; use SilverStripe\Forms\RequiredFields; -use SilverStripe\LinkField\Form\PhoneField; /** * A link to a phone number diff --git a/templates/SilverStripe/LinkField/Form/LinkField.ss b/templates/SilverStripe/LinkField/Form/LinkField.ss index 1710aab3..9c2e10e4 100644 --- a/templates/SilverStripe/LinkField/Form/LinkField.ss +++ b/templates/SilverStripe/LinkField/Form/LinkField.ss @@ -1,2 +1,2 @@ -
\ No newline at end of file +
diff --git a/templates/SilverStripe/LinkField/Form/MultiLinkField.ss b/templates/SilverStripe/LinkField/Form/MultiLinkField.ss index 2103cefe..2ae3ffcd 100644 --- a/templates/SilverStripe/LinkField/Form/MultiLinkField.ss +++ b/templates/SilverStripe/LinkField/Form/MultiLinkField.ss @@ -1,2 +1,2 @@ -
+
diff --git a/tests/php/Traits/AllowedLinkClassesTraitTest.php b/tests/php/Traits/AllowedLinkClassesTraitTest.php index d0293d74..056aa0b8 100644 --- a/tests/php/Traits/AllowedLinkClassesTraitTest.php +++ b/tests/php/Traits/AllowedLinkClassesTraitTest.php @@ -177,29 +177,29 @@ public function testGetSortedTypeProps(array $enabled, array $expected, bool $re $linkField = LinkField::create('LinkField'); $linkField->setAllowedTypes($enabled); - $json = json_decode($linkField->getTypesProps(), true); + $json = json_decode($linkField->getTypesProp(), true); $json = $this->removeCustomLinkTypes($json); $this->assertEquals(array_keys($json), $expected); } - public function testGetTypesPropsCanCreate(): void + public function testGetTypesPropCanCreate(): void { $linkField = LinkField::create('LinkField'); $linkField->setAllowedTypes([SiteTreeLink::class, TestPhoneLink::class]); - $json = json_decode($linkField->getTypesProps(), true); + $json = json_decode($linkField->getTypesProp(), true); $this->assertTrue(array_key_exists('sitetree', $json)); $this->assertTrue(array_key_exists('testphone', $json)); $this->assertTrue($json['sitetree']['allowed']); $this->assertTrue($json['testphone']['allowed']); TestPhoneLink::$fail = 'can-create'; - $json = json_decode($linkField->getTypesProps(), true); + $json = json_decode($linkField->getTypesProp(), true); $this->assertTrue(array_key_exists('sitetree', $json)); $this->assertTrue(array_key_exists('testphone', $json)); $this->assertTrue($json['sitetree']['allowed']); $this->assertFalse($json['testphone']['allowed']); } - public function provideGetTypesProps() : array + public function provideGetTypesProp() : array { return [ 'SiteTreeLink props' => [ @@ -254,9 +254,9 @@ public function provideGetTypesProps() : array } /** - * @dataProvider provideGetTypesProps + * @dataProvider provideGetTypesProp */ - public function testGetTypesProps( + public function testGetTypesProp( string $class, string $key, string $title, @@ -271,7 +271,7 @@ public function testGetTypesProps( $diff = array_diff($this->link_types, [$class]); $linkField->setAllowedTypes($diff); } - $json = json_decode($linkField->getTypesProps(), true); + $json = json_decode($linkField->getTypesProp(), true); $this->assertEquals($key, $json[$key]['key']); $this->assertEquals($title, $json[$key]['title']); $this->assertEquals($priority, $json[$key]['priority']);