Merge branch '4.13' into 4

src/Core/Environment.php

@@ -187,16 +187,22 @@ class Environment
      */
     public static function getEnv($name)
     {
-        switch (true) {
+        if (array_key_exists($name, static::$env)) {
-            case  is_array(static::$env) && array_key_exists($name, static::$env):
             return static::$env[$name];
-            case  is_array($_ENV) && array_key_exists($name, $_ENV):
-                return $_ENV[$name];
-            case  is_array($_SERVER) && array_key_exists($name, $_SERVER):
-                return $_SERVER[$name];
-            default:
-                return getenv($name);
         }
+        // isset() is used for $_ENV and $_SERVER instead of array_key_exists() to fix a very strange issue that
+        // occured in CI running silverstripe/recipe-kitchen-sink where PHP would timeout due apparently due to an
+        // excessively high number of array method calls. isset() is not used for static::$env above because
+        // values there may be null, and isset() will return false for null values
+        // Symfony also uses isset() for reading $_ENV and $_SERVER values
+        // https://github.com/symfony/dependency-injection/blob/6.2/EnvVarProcessor.php#L148
+        if (isset($_ENV[$name])) {
+            return $_ENV[$name];
+        }
+        if (isset($_SERVER[$name])) {
+            return $_SERVER[$name];
+        }
+        return getenv($name);
     }
 
     /**
@@ -231,9 +237,10 @@ class Environment
      */
     public static function hasEnv(string $name): bool
     {
+        // See getEnv() for an explanation of why isset() is used for $_ENV and $_SERVER
         return array_key_exists($name, static::$env)
-            || is_array($_ENV) && array_key_exists($name, $_ENV)
+            || isset($_ENV[$name])
-            || is_array($_SERVER) && array_key_exists($name, $_SERVER)
+            || isset($_SERVER[$name])
             || getenv($name) !== false;
     }
 

src/Dev/Deprecation.php

@@ -237,8 +237,8 @@ class Deprecation
         if (!Director::isDev()) {
             return false;
         }
-        $envVar = Environment::getEnv('SS_DEPRECATION_ENABLED');
         if (Environment::hasEnv('SS_DEPRECATION_ENABLED')) {
+            $envVar = Environment::getEnv('SS_DEPRECATION_ENABLED');
             return self::varAsBoolean($envVar);
         }
         return static::$currentlyEnabled;
@@ -293,8 +293,8 @@ class Deprecation
      */
     public static function shouldShowForHttp(): bool
     {
-        $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_HTTP');
         if (Environment::hasEnv('SS_DEPRECATION_SHOW_HTTP')) {
+            $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_HTTP');
             return self::varAsBoolean($envVar);
         }
         return self::$shouldShowForHttp;
@@ -306,8 +306,8 @@ class Deprecation
      */
     public static function shouldShowForCli(): bool
     {
-        $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_CLI');
         if (Environment::hasEnv('SS_DEPRECATION_SHOW_CLI')) {
+            $envVar = Environment::getEnv('SS_DEPRECATION_SHOW_CLI');
             return self::varAsBoolean($envVar);
         }
         return self::$shouldShowForCli;

src/Forms/GridField/GridFieldFilterHeader.php

@@ -254,21 +254,25 @@ class GridFieldFilterHeader extends AbstractGridFieldComponent implements GridFi
     public function canFilterAnyColumns($gridField)
     {
         $list = $gridField->getList();
-
+        if (!($list instanceof Filterable) || !$this->checkDataType($list)) {
-        if (!$this->checkDataType($list)) {
             return false;
         }
-
+        $modelClass = $gridField->getModelClass();
-        $columns = $gridField->getColumns();
+        // note: searchableFields() will return summary_fields if there are no searchable_fields on the model
-        foreach ($columns as $columnField) {
+        $searchableFields = array_keys($modelClass::singleton()->searchableFields());
-            $metadata = $gridField->getColumnMetadata($columnField);
+        $summaryFields = array_keys($modelClass::singleton()->summaryFields());
-            $title = $metadata['title'];
+        sort($searchableFields);
-
+        sort($summaryFields);
-            if ($title && $list->canFilterBy($columnField)) {
+        // searchable_fields has been explictily defined i.e. searchableFields() is not falling back to summary_fields
+        if ($searchableFields !== $summaryFields) {
+            return true;
+        }
+        // we have fallen back to summary_fields, check they are filterable
+        foreach ($searchableFields as $searchableField) {
+            if ($list->canFilterBy($searchableField)) {
                 return true;
             }
         }
-
         return false;
     }
 

src/ORM/ArrayList.php

@@ -604,7 +604,7 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
 
         $firstRecord = $this->first();
 
-        return is_array($firstRecord) ? array_key_exists($by, $firstRecord) : property_exists($by, $firstRecord ?? '');
+        return is_array($firstRecord) ? array_key_exists($by, $firstRecord) : property_exists($firstRecord, $by ?? '');
     }
 
     /**

tests/php/Core/EnvironmentTest.php

@@ -79,7 +79,6 @@ class EnvironmentTest extends SapphireTest
         $valueOptions = [
             true,
             false,
-            null,
             0,
             1,
             1.75,
@@ -97,6 +96,12 @@ class EnvironmentTest extends SapphireTest
                 ];
             }
         }
+        // `null` isn't a supported value outside of using the `.env` option.
+        $scenarios[] = [
+            'setAs' => '.env',
+            'value' => null,
+            'expected' => true,
+        ];
         $scenarios[] = [
             'setAs' => null,
             'value' => null,

tests/php/Forms/GridField/GridFieldFilterHeaderTest.php

@@ -182,4 +182,30 @@ class GridFieldFilterHeaderTest extends SapphireTest
 
         $this->assertEquals('ReallyCustomSearch', $this->component->getSearchField());
     }
+
+    public function testCanFilterAnyColumns()
+    {
+        $gridField = $this->gridField;
+        $filterHeader = $gridField->getConfig()->getComponentByType(GridFieldFilterHeader::class);
+        
+        // test that you can filter by something if searchable_fields is not defined
+        // silverstripe will scaffold db columns that are in the gridfield to be
+        // searchable by default
+        Config::modify()->remove(Team::class, 'searchable_fields');
+        $this->assertTrue($filterHeader->canFilterAnyColumns($gridField));
+
+        // test that you can filterBy if searchable_fields is defined
+        Config::modify()->set(Team::class, 'searchable_fields', ['Name']);
+        $this->assertTrue($filterHeader->canFilterAnyColumns($gridField));
+
+        // test that you can filterBy if searchable_fields even if it is not a legit field
+        // this is because we're making a blind assumption it will be filterable later in a SearchContext
+        Config::modify()->set(Team::class, 'searchable_fields', ['WhatIsThis']);
+        $this->assertTrue($filterHeader->canFilterAnyColumns($gridField));
+
+        // test that you cannot filter by non-db field when it falls back to summary_fields
+        Config::modify()->remove(Team::class, 'searchable_fields');
+        Config::modify()->set(Team::class, 'summary_fields', ['MySummaryField']);
+        $this->assertFalse($filterHeader->canFilterAnyColumns($gridField));
+    }
 }

tests/php/Forms/GridField/GridFieldFilterHeaderTest/Team.php

@@ -24,4 +24,9 @@ class Team extends DataObject implements TestOnly
         'Cheerleader' => Cheerleader::class,
         'CheerleadersMom' => Mom::class
     ];
+
+    public function getMySummaryField()
+    {
+        return 'MY SUMMARY FIELD';
+    }
 }