ENH Standardise naming and improve code quality

silverstripe · Feb 12, 2024 · 28b833f · 28b833f
1 parent 51acf4f
commit 28b833f
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -1,2 +1,2 @@
 <input $AttributesHTML />
-<div data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProps"></div>
+<div data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProp"></div>
diff --git a/templates/SilverStripe/LinkField/Form/MultiLinkField.ss b/templates/SilverStripe/LinkField/Form/MultiLinkField.ss
@@ -1,2 +1,2 @@
 <input $AttributesHTML />
-<div data-is-multi="true" data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProps"></div>
+<div data-is-multi="true" data-field-id="$ID" data-schema-component="$SchemaComponent" class="entwine-linkfield" data-types="$TypesProp"></div>
diff --git 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']);