Complete CI setup (#3)

* Enable Github CI

* Update ci.yml

* Require "colymba/gridfield-bulk-editing-tools" for now to fix "class not found" issue

* Fix CI tests

* Deacticate phplinting for now

* Try phplinting again

* Work on PSR-12 compliance

* CI: Re-enable phplint

.github/workflows/ci.yml

@@ -9,5 +9,3 @@ jobs:
   ci:
     name: CI
     uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1
-    with:
-      phplinting: false

composer.json

@@ -21,7 +21,8 @@
         "colymba/gridfield-bulk-editing-tools": "^3.0 || ^4.0"
     },
     "require-dev": {
-        "phpunit/phpunit": "^9.5"
+        "phpunit/phpunit": "^9.5",
+        "squizlabs/php_codesniffer": "^3.7"
     },
     "suggest": {
         "bummzack/sortablefile": "Support for sortable UploadFields, e.g. for Download Package files",

phpcs.xml.dist

@@ -7,8 +7,8 @@
 
     <!-- base rules are PSR-12 -->
     <rule ref="PSR12" >
-        <!-- Current exclusions -->
-        <!--<exclude name="PSR1.Files.SideEffects.FoundWithSymbols" />-->
-        <!--<exclude name="PSR1.Methods.CamelCapsMethodName.NotCamelCaps" />-->
+        <!-- Allow non camel cased method names -->
+        <exclude name="PSR1.Methods.CamelCapsMethodName.NotCamelCaps"/>
+        <exclude name="Generic.Files.LineLength.TooLong" />
     </rule>
 </ruleset>

src/Controller/DLCodeAdmin.php

@@ -17,7 +17,6 @@
 
 class DLCodeAdmin extends ModelAdmin
 {
-
     private static $url_segment = 'dlcodes';
 
     private static $managed_models = [
@@ -65,9 +64,7 @@ protected function getGridFieldConfig(): GridFieldConfig
     {
         $config = parent::getGridFieldConfig();
         if ($this->modelTab == 'packages') {
-            $config->addComponent(
-                RowValidation::create(), GridFieldEditButton::class
-            );
+            $config->addComponent(RowValidation::create(), GridFieldEditButton::class);
         }
         if ($this->modelTab == 'codes') {
             if (singleton(DLCode::class)->canCreate()) {
@@ -82,11 +79,10 @@ protected function getGridFieldConfig(): GridFieldConfig
                     $config->addComponent(BulkManager::create([], false)
                         ->addBulkAction(DeleteHandler::class)
                         ->addBulkAction(MarkDistributedHandler::class)
-                        ->addBulkAction(UnmarkDistributedHandler::class)
-                    );
+                        ->addBulkAction(UnmarkDistributedHandler::class));
                 }
-
-            }        }
+            }
+        }
         return $config;
     }
 

src/Forms/DLRequestForm.php

@@ -22,7 +22,7 @@ public function __construct(RequestHandler $controller = null, $name = self::DEF
         $actions = new FieldList(
             FormAction::create('submitcode', _t(__CLASS__ . '.ACTION_submitcode', 'Submit'))
         );
-        $validator = new RequiredFields('Code');;
+        $validator = new RequiredFields('Code');
         parent::__construct($controller, $name, $fields, $actions, $validator);
     }
 

src/Forms/GenerateCodesForm.php

@@ -46,5 +46,4 @@ public function __construct(RequestHandler $controller, $name, DLCode $obj, $lin
             Controller::join_links($link, $name)
         );
     }
-
 }

src/Forms/GridField/GenerateCodesButton.php

@@ -2,7 +2,6 @@
 
 namespace Mhe\DownloadCodes\Forms\GridField;
 
-
 use SilverStripe\Forms\GridField\GridField;
 use SilverStripe\Forms\GridField\GridField_FormAction;
 use SilverStripe\Forms\GridField\GridFieldImportButton;
@@ -16,7 +15,6 @@
  */
 class GenerateCodesButton extends GridFieldImportButton
 {
-
     /**
      * @param GridField $gridField
      * @return array
@@ -64,8 +62,4 @@ public function getHTMLFragments($gridField)
             $this->targetFragment => $button->Field()
         ];
     }
-
-
-
-
 }

src/Forms/GridField/RowValidation.php

@@ -8,7 +8,6 @@
 
 class RowValidation extends AbstractGridFieldComponent implements GridField_ColumnProvider
 {
-
     public function augmentColumns($gridField, &$columns)
     {
         if (!in_array('Validation', $columns ?? [])) {

src/Mhe/DownloadCodes/Controller/MarkDistributedHandler.php

@@ -54,6 +54,4 @@ public function mark(HTTPRequest $request)
         }
         return $response;
     }
-
-
 }

src/Mhe/DownloadCodes/Controller/UnmarkDistributedHandler.php

@@ -54,6 +54,4 @@ public function unmark(HTTPRequest $request)
         }
         return $response;
     }
-
-
 }

src/Model/DLCode.php

@@ -25,11 +25,10 @@
  */
 class DLCode extends DataObject implements PermissionProvider
 {
-
     /**
      * Permission to edit DLCodes
      */
-    const EDIT_ALL = 'DLCode_EDIT_ALL';
+    public const EDIT_ALL = 'DLCode_EDIT_ALL';
 
     private static $table_name = 'DLCode';
 
@@ -133,7 +132,9 @@ public function getCMSFields()
         $fields->addFieldToTab('Root.Main', $usagecount);
         // simple readonly grid field for Redemptions
         $redemptions = $fields->fieldByName('Root.Redemptions.Redemptions');
-        if ($redemptions) $redemptions->setConfig(GridFieldConfig_Base::create());
+        if ($redemptions) {
+            $redemptions->setConfig(GridFieldConfig_Base::create());
+        }
 
         $this->extend('updateCMSFields', $fields);
         return $fields;
@@ -271,7 +272,9 @@ public function increaseUsageCount()
      */
     public function redeem()
     {
-        if (!$this->isRedeeamable()) return null;
+        if (!$this->isRedeeamable()) {
+            return null;
+        }
         $this->increaseUsageCount();
         if ($this->Limited) {
             $redemption = $this->Redemptions()->first();

src/Model/DLPackage.php

@@ -37,11 +37,10 @@
  */
 class DLPackage extends DataObject implements PermissionProvider, Flushable
 {
-
     /**
      * Permission to edit DLCodes
      */
-    const EDIT_ALL = 'DLPackage_EDIT_ALL';
+    public const EDIT_ALL = 'DLPackage_EDIT_ALL';
 
     private static $table_name = 'DLPackage';
 
@@ -176,12 +175,16 @@ public function canCreate($member = null, $context = [])
     public function filesAreProtected()
     {
         $protected = true;
-        $checker = Injector::inst()->get(PermissionChecker::class.'.file');
+        $checker = Injector::inst()->get(PermissionChecker::class . '.file');
         /* @var \SilverStripe\Assets\File $file */
         foreach ($this->Files() as $file) {
-            if ($file->CanViewType == InheritedPermissions::ANYONE) return false;
+            if ($file->CanViewType == InheritedPermissions::ANYONE) {
+                return false;
+            }
             if ($file->CanViewType === InheritedPermissions::INHERIT && $file->ParentID) {
-                if ($checker->canView($file->ParentID, null)) return false;
+                if ($checker->canView($file->ParentID, null)) {
+                    return false;
+                }
             }
         }
         return $protected;
@@ -192,7 +195,8 @@ public function gridFieldValidation()
         return $this->filesAreProtected();
     }
 
-    public function gridFieldValidationMessage() {
+    public function gridFieldValidationMessage()
+    {
         return _t(__CLASS__ . '.UnprotectedFiles', 'unprotected files');
     }
 
@@ -202,7 +206,8 @@ public function gridFieldValidationMessage() {
      * @param array|null $filter optionally filter files to Zip, @see \SilverStripe\ORM\DataList::filter()
      * @return DBFile|null
      */
-    public function getZippedFiles($filter = null) {
+    public function getZippedFiles($filter = null)
+    {
         $files = is_array($filter) ? $this->Files()->filter($filter) : $this->Files();
         if (!$this->EnableZip || $files->count() < 1) {
             return null;
@@ -225,7 +230,7 @@ public function getZippedFiles($filter = null) {
                 $tempPath = TEMP_PATH;
                 $zip = new ZipArchive();
                 $tempFile = tempnam($tempPath, 'dl');
-                if ($zip->open($tempFile, ZipArchive::OVERWRITE|ZipArchive::CREATE)!== TRUE) {
+                if ($zip->open($tempFile, ZipArchive::OVERWRITE | ZipArchive::CREATE) !== true) {
                     user_error("Could not open temp file for ZIP creation", E_USER_WARNING);
                     return null;
                 }
@@ -278,7 +283,8 @@ public function getCache()
      * get cache key, build from Title and package files
      * @return string
      */
-    public function getCacheKey() {
+    public function getCacheKey()
+    {
         $key = hash_init('sha1');
         hash_update($key, $this->ID . $this->Title);
         /* @var \SilverStripe\Assets\File $file */

src/Model/DLPageController.php

@@ -11,7 +11,6 @@
 
 class DLPageController extends PageController
 {
-
     private static $allowed_actions = [
         "RequestForm",
         "redeem"
@@ -75,5 +74,4 @@ public function submitcode($data, DLRequestForm $form)
         $url = Controller::join_links($this->owner->Link('redeem'), $redemption->getUrlParamString());
         return $this->redirect($url);
     }
-
 }

src/Model/DLRedemption.php

@@ -67,7 +67,9 @@ public function isValid()
      */
     public function getUrlParamString()
     {
-        if (!$this->isValid()) return '';
+        if (!$this->isValid()) {
+            return '';
+        }
         return "?" . http_build_query(['c' => $this->Code()->ID, 'r' => $this->ID, 's' => $this->URLSecret]);
     }
 
@@ -79,7 +81,9 @@ public function getUrlParamString()
      */
     public static function get_by_query_params($vars, $onlyvalid = true)
     {
-        if (!isset($vars['r']) || !isset($vars['c']) || !isset($vars['s'])) return null;
+        if (!isset($vars['r']) || !isset($vars['c']) || !isset($vars['s'])) {
+            return null;
+        }
         $redemption = self::get()->filter([
             'ID' => $vars['r'],
             'Code.ID' => $vars['c'],
@@ -105,5 +109,4 @@ public function canView($member = null)
         }
         return Permission::checkMember($member, 'CMS_ACCESS_DLCodeAdmin');
     }
-
 }

tests/Model/DLCodeTest.php

@@ -10,7 +10,6 @@
 
 class DLCodeTest extends SapphireTest
 {
-
     protected static $fixture_file = 'DLCodeTest.yml';
 
     public function setUp(): void
@@ -76,13 +75,14 @@ public function testRedeemUnlimited()
         $this->assertTrue($result->exists(), 'a new redemption is written');
         $this->assertEquals($code->ID, $result->Code()->ID, 'New DLRedemption object is related to the code');
         $this->assertNotEquals($redemption->URLSecret, $result->URLSecret, 'a new redemption is written instead of the existing one');
-        $this->assertGreaterThan($redemption->ID, $result->ID,'a new redemption is written instead of the existing one');
+        $this->assertGreaterThan($redemption->ID, $result->ID, 'a new redemption is written instead of the existing one');
     }
 
     /**
      * test validation (for creation of new DLCodes)
      */
-    public function testValidateUnique() {
+    public function testValidateUnique()
+    {
         $code = new DLCode([ 'Code' => 'abc']);
         $code->write();
         $code = new DLCode([ 'Code' => 'abc1234']);
@@ -97,7 +97,8 @@ public function testValidateUnique() {
     /**
      * test getting a redemption for code as entered by a user
      */
-    public function testGetRedeemable() {
+    public function testGetRedeemable()
+    {
         Config::modify()->set(DLCode::class, 'case_sensitive', true);
         $code = DLCode::get_redeemable_code('VALIDCODE');
         $this->assertNotEmpty($code);
@@ -114,7 +115,8 @@ public function testGetRedeemable() {
         $this->assertTrue($code->isRedeeamable());
     }
 
-    public function testAutoGenerate() {
+    public function testAutoGenerate()
+    {
         Config::modify()->set(DLCode::class, 'autogenerate_chars', 'abcde');
         Config::modify()->set(DLCode::class, 'autogenerate_length', 6);
         for ($i = 0; $i < 5; $i++) {

tests/Model/DLPackageTest.php

@@ -26,7 +26,8 @@ public function setUp(): void
                 $sourcePath = __DIR__ . '/../downloads/' . $file->Name;
                 $file->setFromLocalFile($sourcePath, $file->Filename);
                 $file->publishSingle();
-            } catch (\InvalidArgumentException $e) { }
+            } catch (\InvalidArgumentException $e) {
+            }
         }
     }
 
@@ -39,7 +40,8 @@ public function tearDown(): void
      * check if all files are protected
      * @return void
      */
-    public function testCheckFilesAccess() {
+    public function testCheckFilesAccess()
+    {
         $package = new DLPackage();
         $this->assertTrue($package->filesAreProtected());
         $package->Files()->add($this->objFromFixture(Image::class, 'protectedimage1'));
@@ -56,7 +58,8 @@ public function testCheckFilesAccess() {
         $this->assertFalse($package->filesAreProtected());
     }
 
-    public function testGetCacheKey() {
+    public function testGetCacheKey()
+    {
         $hashs = [];
         /* @var DLPackage $package */
         $package = $this->objFromFixture(DLPackage::class, 'package1');
@@ -78,7 +81,8 @@ public function testGetCacheKey() {
         $this->assertEquals(4, count(array_filter($hashs, fn($hash) => strlen($hash) == 40)));
     }
 
-    public function testGeneratedZip() {
+    public function testGeneratedZip()
+    {
         /* @var DLPackage $package */
         $package = $this->objFromFixture(DLPackage::class, 'package1');
         $this->assertEquals('Test Package', $package->Title);
@@ -88,7 +92,7 @@ public function testGeneratedZip() {
         $this->assertTrue($zip->exists());
         $this->assertEquals(40, strlen($zip->Hash));
         $this->assertEquals('application/zip', $zip->getMimeType());
-        $this->assertEquals('test-package.zip', $zip->Filename );
+        $this->assertEquals('test-package.zip', $zip->Filename);
 
         $package->EnableZip = false;
         $this->assertEmpty($package->getZippedFiles());