From 2e73b5eeca692f015d63539322dccf62e25cf72a Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Thu, 22 Jun 2023 11:04:26 +0200 Subject: [PATCH 01/13] Use cached query Fixes https://github.com/silverstripe/silverstripe-framework/issues/10833 --- .../MemberAuthenticator/SessionAuthenticationHandler.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index c2fbd399c..ef5008ee6 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -4,6 +4,7 @@ namespace SilverStripe\Security\MemberAuthenticator; use SilverStripe\Control\Controller; use SilverStripe\Control\Cookie; +use SilverStripe\ORM\DataObject; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Security\AuthenticationHandler; @@ -59,7 +60,7 @@ class SessionAuthenticationHandler implements AuthenticationHandler return null; } /** @var Member $member */ - $member = Member::get()->byID($id); + $member = DataObject::get_by_id("SilverStripe\\Security\\Member", $id); return $member; } From 9391e696bbb4e12f58c52f3335b15b0e157cc2c1 Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Fri, 23 Jun 2023 09:35:34 +0200 Subject: [PATCH 02/13] use Member::class --- .../MemberAuthenticator/SessionAuthenticationHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php index ef5008ee6..a192f7a71 100644 --- a/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php +++ b/src/Security/MemberAuthenticator/SessionAuthenticationHandler.php @@ -60,7 +60,7 @@ class SessionAuthenticationHandler implements AuthenticationHandler return null; } /** @var Member $member */ - $member = DataObject::get_by_id("SilverStripe\\Security\\Member", $id); + $member = DataObject::get_by_id(Member::class, $id); return $member; } From 909bee810194ec630f440fb610a45999acebd921 Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Sat, 29 Jul 2023 08:56:24 +0200 Subject: [PATCH 03/13] fix cache key --- src/Core/Manifest/VersionProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Core/Manifest/VersionProvider.php b/src/Core/Manifest/VersionProvider.php index 503ed4486..f70629101 100644 --- a/src/Core/Manifest/VersionProvider.php +++ b/src/Core/Manifest/VersionProvider.php @@ -45,6 +45,7 @@ class VersionProvider public function getVersion() { $key = sprintf('%s-%s', $this->getComposerLockPath(), 'all'); + $key = preg_replace("/[^A-Za-z0-9]/", '', $key); $version = $this->getCachedValue($key); if ($version) { return $version; @@ -81,6 +82,7 @@ class VersionProvider public function getModuleVersion(string $module): string { $key = sprintf('%s-%s', $this->getComposerLockPath(), $module); + $key = preg_replace("/[^A-Za-z0-9]/", '', $key); $version = $this->getCachedValue($key); if ($version) { return $version; From d3c2fa897c06b4c8379b514a7773c74e1c3ee4ea Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Tue, 1 Aug 2023 14:27:00 +0200 Subject: [PATCH 04/13] nicer key --- src/Core/Manifest/VersionProvider.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Core/Manifest/VersionProvider.php b/src/Core/Manifest/VersionProvider.php index f70629101..7901223d8 100644 --- a/src/Core/Manifest/VersionProvider.php +++ b/src/Core/Manifest/VersionProvider.php @@ -44,8 +44,7 @@ class VersionProvider */ public function getVersion() { - $key = sprintf('%s-%s', $this->getComposerLockPath(), 'all'); - $key = preg_replace("/[^A-Za-z0-9]/", '', $key); + $key = sprintf('%s_%s', preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath()), 'all'); $version = $this->getCachedValue($key); if ($version) { return $version; @@ -81,8 +80,7 @@ class VersionProvider */ public function getModuleVersion(string $module): string { - $key = sprintf('%s-%s', $this->getComposerLockPath(), $module); - $key = preg_replace("/[^A-Za-z0-9]/", '', $key); + $key = sprintf('%s_%s', preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath()), $module); $version = $this->getCachedValue($key); if ($version) { return $version; From c0cc129f1249ee979c2052700b4d818c30c94965 Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Tue, 1 Aug 2023 19:40:34 +0200 Subject: [PATCH 05/13] remove sprintf --- src/Core/Manifest/VersionProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Manifest/VersionProvider.php b/src/Core/Manifest/VersionProvider.php index 7901223d8..cf8f94cd3 100644 --- a/src/Core/Manifest/VersionProvider.php +++ b/src/Core/Manifest/VersionProvider.php @@ -44,7 +44,7 @@ class VersionProvider */ public function getVersion() { - $key = sprintf('%s_%s', preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath()), 'all'); + $key = preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath() . 'all'); $version = $this->getCachedValue($key); if ($version) { return $version; @@ -80,7 +80,7 @@ class VersionProvider */ public function getModuleVersion(string $module): string { - $key = sprintf('%s_%s', preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath()), $module); + $key = preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath() . $module); $version = $this->getCachedValue($key); if ($version) { return $version; From 359cb1427afee373ea9ff00f8aea148fdd6ec9ed Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Sat, 29 Jul 2023 08:13:45 +0200 Subject: [PATCH 06/13] BUG include silverstripe core files into roots --- src/Core/Manifest/ClassManifest.php | 5 +++++ tests/php/Core/ClassInfoTest.php | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php index 9d406620d..0e2887635 100644 --- a/src/Core/Manifest/ClassManifest.php +++ b/src/Core/Manifest/ClassManifest.php @@ -650,6 +650,11 @@ class ClassManifest } $this->children[$lowerAncestor][$lowerClassName] = $className; } + + // If the class extends a core class, add class to roots + if (strpos($ancestor, 'SilverStripe\\Control') === 0) { + $this->roots[$lowerAncestor] = $ancestor; + } } else { $this->roots[$lowerClassName] = $className; } diff --git a/tests/php/Core/ClassInfoTest.php b/tests/php/Core/ClassInfoTest.php index a1d353483..7528c3f36 100644 --- a/tests/php/Core/ClassInfoTest.php +++ b/tests/php/Core/ClassInfoTest.php @@ -87,6 +87,14 @@ class ClassInfoTest extends SapphireTest $subclassesWithoutBase, ClassInfo::subclassesFor('silverstripe\\core\\tests\\classinfotest\\baseclass', false) ); + + // Check that core classes are present (eg: Email subclasses) + $emailClasses = ClassInfo::subclassesFor(\SilverStripe\Control\Email\Email::class); + $this->assertArrayHasKey( + 'silverstripe\\control\\tests\\email\\emailtest\\emailsubclass', + $emailClasses, + 'It contains : ' . json_encode($emailClasses) + ); } public function testClassName() From b9f63001c5c6a63d89461d448fe8753f88326bdd Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Wed, 2 Aug 2023 06:55:16 +0200 Subject: [PATCH 07/13] Update src/Core/Manifest/VersionProvider.php Co-authored-by: Michal Kleiner --- src/Core/Manifest/VersionProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Manifest/VersionProvider.php b/src/Core/Manifest/VersionProvider.php index cf8f94cd3..1aaef5a38 100644 --- a/src/Core/Manifest/VersionProvider.php +++ b/src/Core/Manifest/VersionProvider.php @@ -44,7 +44,7 @@ class VersionProvider */ public function getVersion() { - $key = preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath() . 'all'); + $key = preg_replace("/[^A-Za-z0-9]/", '_', $this->getComposerLockPath() . '_all'); $version = $this->getCachedValue($key); if ($version) { return $version; From 0e839e12cc4af42801b5290fc8704a5ba3fc61be Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Wed, 2 Aug 2023 06:55:32 +0200 Subject: [PATCH 08/13] Update src/Core/Manifest/VersionProvider.php Co-authored-by: Michal Kleiner --- src/Core/Manifest/VersionProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Manifest/VersionProvider.php b/src/Core/Manifest/VersionProvider.php index 1aaef5a38..75339b4ba 100644 --- a/src/Core/Manifest/VersionProvider.php +++ b/src/Core/Manifest/VersionProvider.php @@ -80,7 +80,7 @@ class VersionProvider */ public function getModuleVersion(string $module): string { - $key = preg_replace("/[^A-Za-z0-9]/", '', $this->getComposerLockPath() . $module); + $key = preg_replace("/[^A-Za-z0-9]/", '_', $this->getComposerLockPath() . '_' . $module); $version = $this->getCachedValue($key); if ($version) { return $version; From d24095aba8de6e3328859953f00e1c6a4ac00db3 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Mon, 24 Jul 2023 16:10:47 +1200 Subject: [PATCH 09/13] FIX Image in summaryfields breaks search --- src/Forms/GridField/GridFieldFilterHeader.php | 2 +- src/ORM/DataObject.php | 63 ++++++++++++++----- tests/php/ORM/DataObjectTest.php | 45 +++++++++++++ tests/php/ORM/DataObjectTest/Player.php | 5 ++ tests/php/ORM/Search/SearchContextTest.php | 11 +++- .../SearchContextTest/NoSearchableFields.php | 23 +++++++ 6 files changed, 133 insertions(+), 16 deletions(-) diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index a0fff8038..e16e870a2 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -269,7 +269,7 @@ class GridFieldFilterHeader extends AbstractGridFieldComponent implements GridFi sort($searchableFields); sort($summaryFields); // searchable_fields has been explictily defined i.e. searchableFields() is not falling back to summary_fields - if ($searchableFields !== $summaryFields) { + if (!empty($searchableFields) && ($searchableFields !== $summaryFields)) { return true; } // we have fallen back to summary_fields, check they are filterable diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index cdb804206..f17c5d487 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -3724,6 +3724,52 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $this->extend('onAfterBuild'); } + private function getDatabaseBackedField(string $fieldPath): ?string + { + $component = $this; + $fieldParts = []; + $parts = explode('.', $fieldPath ?? ''); + + foreach ($parts as $nextPart) { + if (!$component) { + return null; + } + $fieldParts[] = $nextPart; + + if ($component instanceof Relation || $component instanceof DataList) { + if ($component->hasMethod($nextPart)) { + // If the next part is a method, we don't have a database-backed field. + return null; + } + // The next part could either be a field, or another relation + $singleton = DataObject::singleton($component->dataClass()); + if ($singleton->dbObject($nextPart) instanceof DBField) { + // If the next part is a DBField, we've found the database-backed field. + break; + } + $component = $component->relation($nextPart); + array_shift($parts); + } elseif ($component instanceof DataObject && ($component->dbObject($nextPart) instanceof DBField)) { + // If the next part is a DBField, we've found the database-backed field. + break; + } elseif ($component instanceof DataObject && $component->getRelationType($nextPart) !== null) { + // If it's a last part or only one elemnt of a relation, we don't have a database-backed field. + if (count($parts) === 1) { + return null; + } + $component = $component->$nextPart(); + array_shift($parts); + } elseif (ClassInfo::hasMethod($component, $nextPart)) { + // If the next part is a method, we don't have a database-backed field. + return null; + } else { + return null; + } + } + + return implode('.', $fieldParts) ?: null; + } + /** * Get the default searchable fields for this object, as defined in the * $searchable_fields list. If searchable fields are not defined on the @@ -3742,21 +3788,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $summaryFields = array_keys($this->summaryFields() ?? []); $fields = []; - // remove the custom getters as the search should not include them - $schema = static::getSchema(); if ($summaryFields) { - foreach ($summaryFields as $key => $name) { - $spec = $name; - - // Extract field name in case this is a method called on a field (e.g. "Date.Nice") - if (($fieldPos = strpos($name ?? '', '.')) !== false) { - $name = substr($name ?? '', 0, $fieldPos); - } - - if ($schema->fieldSpec($this, $name)) { - $fields[] = $name; - } elseif ($this->relObject($spec)) { - $fields[] = $spec; + foreach ($summaryFields as $name) { + if ($field = $this->getDatabaseBackedField($name)) { + $fields[] = $field; } } } diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php index 9ec9e1629..ed6c6a199 100644 --- a/tests/php/ORM/DataObjectTest.php +++ b/tests/php/ORM/DataObjectTest.php @@ -25,6 +25,7 @@ use SilverStripe\ORM\Tests\DataObjectTest\TreeNode; use SilverStripe\ORM\ValidationException; use SilverStripe\Security\Member; use SilverStripe\View\ViewableData; +use ReflectionMethod; use stdClass; class DataObjectTest extends SapphireTest @@ -2671,4 +2672,48 @@ class DataObjectTest extends SapphireTest $vals = ['25.25', '50.00', '75.00', '100.50']; $this->assertSame(array_combine($vals ?? [], $vals ?? []), $obj->dbObject('MyEnumWithDots')->enumValues()); } + + public function provideTestGetDatabaseBackedField() + { + return [ + ['Captain.IsRetired', 'Captain.IsRetired'], + ['Captain.ShirtNumber', 'Captain.ShirtNumber'], + ['Captain.FavouriteTeam', null], + ['Captain.FavouriteTeam.Fans', null], + ['Captain.FavouriteTeam.Fans.Count', null], + ['Captain.FavouriteTeam.Title', 'Captain.FavouriteTeam.Title'], + ['Captain.FavouriteTeam.Title.Plain', 'Captain.FavouriteTeam.Title'], + ['Captain.FavouriteTeam.ReturnsNull', null], + ['Captain.FavouriteTeam.MethodDoesNotExist', null], + ['Captain.ReturnsNull', null], + ['Founder.FavouriteTeam.Captain.ShirtNumber', 'Founder.FavouriteTeam.Captain.ShirtNumber'], + ['Founder.FavouriteTeam.Captain.Fans', null], + ['Founder.FavouriteTeam.Captain.Fans.Name.Plain', 'Founder.FavouriteTeam.Captain.Fans.Name'], + ['Founder.FavouriteTeam.Captain.ReturnsNull', null], + ['HasOneRelationship.FavouriteTeam.MyTitle', null], + ['SubTeams.Comments.Name.Plain', 'SubTeams.Comments.Name'], + ['Title', 'Title'], + ['Title.Plain', 'Title'], + ['DatabaseField', 'DatabaseField'], + ['DatabaseField.MethodDoesNotExist', 'DatabaseField'], + ['ReturnsNull', null], + ['DynamicField', null], + ['SubTeams.ParentTeam.Fans', null], + ['SubTeams.ParentTeam.Founder.FoundingTeams', null], + ]; + } + + /** + * @dataProvider provideTestGetDatabaseBackedField + */ + public function testGetDatabaseBackedField(string $fieldPath, $expected) + { + $dataObjectClass = new DataObject(); + $method = new ReflectionMethod($dataObjectClass, 'getDatabaseBackedField'); + $method->setAccessible(true); + $class = new Team([]); + + $databaseBackedField = $method->invokeArgs($class, [$fieldPath]); + $this->assertSame($expected, $databaseBackedField); + } } diff --git a/tests/php/ORM/DataObjectTest/Player.php b/tests/php/ORM/DataObjectTest/Player.php index 0dc5a795f..4aa2f69c7 100644 --- a/tests/php/ORM/DataObjectTest/Player.php +++ b/tests/php/ORM/DataObjectTest/Player.php @@ -37,4 +37,9 @@ class Player extends Member implements TestOnly 'IsRetired', 'ShirtNumber' ]; + + public function ReturnsNull() + { + return null; + } } diff --git a/tests/php/ORM/Search/SearchContextTest.php b/tests/php/ORM/Search/SearchContextTest.php index 8c80a4492..7236709cb 100644 --- a/tests/php/ORM/Search/SearchContextTest.php +++ b/tests/php/ORM/Search/SearchContextTest.php @@ -58,13 +58,22 @@ class SearchContextTest extends SapphireTest $obj = new SearchContextTest\NoSearchableFields(); $summaryFields = $obj->summaryFields(); $expected = []; - foreach ($summaryFields as $field => $label) { + + $expectedSearchableFields = [ + 'Name', + 'Customer.FirstName', + 'HairColor', + 'EyeColor', + ]; + + foreach ($expectedSearchableFields as $field) { $expected[$field] = [ 'title' => $obj->fieldLabel($field), 'filter' => 'PartialMatchFilter', ]; } $this->assertEquals($expected, $obj->searchableFields()); + $this->assertNotEquals($summaryFields, $obj->searchableFields()); } public function testSummaryIncludesDefaultFieldsIfNotDefined() diff --git a/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php b/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php index 39ead8537..515300895 100644 --- a/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php +++ b/tests/php/ORM/Search/SearchContextTest/NoSearchableFields.php @@ -4,6 +4,7 @@ namespace SilverStripe\ORM\Tests\Search\SearchContextTest; use SilverStripe\Dev\TestOnly; use SilverStripe\ORM\DataObject; +use SilverStripe\Assets\Image; class NoSearchableFields extends DataObject implements TestOnly { @@ -18,12 +19,34 @@ class NoSearchableFields extends DataObject implements TestOnly private static $has_one = [ 'Customer' => Customer::class, + 'Image' => Image::class, ]; private static $summary_fields = [ 'Name' => 'Custom Label', + 'Customer' => 'Customer', 'Customer.FirstName' => 'Customer', + 'Image.CMSThumbnail' => 'Image', + 'Image.BackLinks' => 'Backlinks', + 'Image.BackLinks.Count' => 'Backlinks', 'HairColor', 'EyeColor', + 'ReturnsNull', + 'DynamicField' ]; + + public function MyName() + { + return 'Class ' . $this->Name; + } + + public function getDynamicField() + { + return 'dynamicfield'; + } + + public function ReturnsNull() + { + return null; + } } From 2a56cc3ea4f75579299cd8039eca33e29e6ee047 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli <36352093+GuySartorelli@users.noreply.github.com> Date: Mon, 7 Aug 2023 11:57:23 +1200 Subject: [PATCH 10/13] MNT Run ORM tests in parallel to other tests (#10901) --- phpunit.xml.dist | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index dbb77b368..f435f9a45 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -8,8 +8,15 @@ Requires PHPUnit ^9 tests/php - + + tests/php + + tests/php/ORM + + + + tests/php/ORM vendor/silverstripe/cms/tests From 5a52484d881e2fedeb344f1461c3208086afc9e2 Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Tue, 8 Aug 2023 11:48:12 +1200 Subject: [PATCH 11/13] NEW Add FieldsValidator to ensure fields get validated --- src/Forms/FieldsValidator.php | 26 ++++++++ src/ORM/DataObject.php | 5 +- tests/behat/src/CmsUiContext.php | 36 +++++++---- tests/php/Forms/FieldsValidatorTest.php | 79 +++++++++++++++++++++++++ tests/php/Security/GroupTest.php | 7 ++- 5 files changed, 136 insertions(+), 17 deletions(-) create mode 100644 src/Forms/FieldsValidator.php create mode 100644 tests/php/Forms/FieldsValidatorTest.php diff --git a/src/Forms/FieldsValidator.php b/src/Forms/FieldsValidator.php new file mode 100644 index 000000000..0de96225f --- /dev/null +++ b/src/Forms/FieldsValidator.php @@ -0,0 +1,26 @@ +form->Fields(); + + foreach ($fields as $field) { + $valid = ($field->validate($this) && $valid); + } + + return $valid; + } + + public function canBeCached(): bool + { + return true; + } +} diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index f17c5d487..1adaf9f14 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -16,6 +16,7 @@ use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormScaffolder; use SilverStripe\Forms\CompositeValidator; +use SilverStripe\Forms\FieldsValidator; use SilverStripe\Forms\HiddenField; use SilverStripe\i18n\i18n; use SilverStripe\i18n\i18nEntityProvider; @@ -2600,7 +2601,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public function getCMSCompositeValidator(): CompositeValidator { - $compositeValidator = CompositeValidator::create(); + $compositeValidator = CompositeValidator::create([FieldsValidator::create()]); // Support for the old method during the deprecation period if ($this->hasMethod('getCMSValidator')) { @@ -3735,7 +3736,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity return null; } $fieldParts[] = $nextPart; - + if ($component instanceof Relation || $component instanceof DataList) { if ($component->hasMethod($nextPart)) { // If the next part is a method, we don't have a database-backed field. diff --git a/tests/behat/src/CmsUiContext.php b/tests/behat/src/CmsUiContext.php index 62504e63b..d80db9bfc 100644 --- a/tests/behat/src/CmsUiContext.php +++ b/tests/behat/src/CmsUiContext.php @@ -6,6 +6,7 @@ use Behat\Behat\Context\Context; use Behat\Behat\Hook\Scope\AfterStepScope; use Behat\Mink\Element\Element; use Behat\Mink\Element\NodeElement; +use Behat\Mink\Exception\ElementNotFoundException; use Behat\Mink\Selector\Xpath\Escaper; use Behat\Mink\Session; use PHPUnit\Framework\Assert; @@ -86,26 +87,37 @@ class CmsUiContext implements Context } /** - * @Then /^I should see a "(.+)" (\w+) toast$/ + * @Then /^I should (not |)see a "(.+)" (\w+) toast$/ */ - public function iShouldSeeAToast($notice, $type) + public function iShouldSeeAToast($not, $notice, $type) { - $this->getMainContext()->assertElementContains('.toast--' . $type, $notice); + if ($not) { + try { + // If there is a toast of that type, ensure it doesn't contain the notice. + $this->getMainContext()->assertElementNotContains('.toast--' . $type, $notice); + } catch (ElementNotFoundException $e) { + // no-op - if the element doesn't exist at all, then that passes the test. + } + } else { + $this->getMainContext()->assertElementContains('.toast--' . $type, $notice); + } } /** - * @Then /^I should see a "(.+)" (\w+) toast with these actions: (.+)$/ + * @Then /^I should (not |)see a "(.+)" (\w+) toast with these actions: (.+)$/ */ - public function iShouldSeeAToastWithAction($notice, $type, $actions) + public function iShouldSeeAToastWithAction($not, $notice, $type, $actions) { - $this->iShouldSeeAToast($notice, $type); + $this->iShouldSeeAToast($not, $notice, $type); - $actions = explode(',', $actions ?? ''); - foreach ($actions as $order => $action) { - $this->getMainContext()->assertElementContains( - sprintf('.toast--%s .toast__action:nth-child(%s)', $type, $order+1), - trim($action ?? '') - ); + if (!$not) { + $actions = explode(',', $actions ?? ''); + foreach ($actions as $order => $action) { + $this->getMainContext()->assertElementContains( + sprintf('.toast--%s .toast__action:nth-child(%s)', $type, $order+1), + trim($action ?? '') + ); + } } } diff --git a/tests/php/Forms/FieldsValidatorTest.php b/tests/php/Forms/FieldsValidatorTest.php new file mode 100644 index 000000000..8c958bcd3 --- /dev/null +++ b/tests/php/Forms/FieldsValidatorTest.php @@ -0,0 +1,79 @@ + [ + 'values' => [], + 'isValid' => true, + ], + 'empty values arent invalid' => [ + 'values' => [ + 'EmailField1' => '', + 'EmailField2' => null, + ], + 'isValid' => true, + ], + 'any invalid is invalid' => [ + 'values' => [ + 'EmailField1' => 'email@example.com', + 'EmailField2' => 'not email', + ], + 'isValid' => false, + ], + 'all invalid is invalid' => [ + 'values' => [ + 'EmailField1' => 'not email', + 'EmailField2' => 'not email', + ], + 'isValid' => false, + ], + 'all valid is valid' => [ + 'values' => [ + 'EmailField1' => 'email@example.com', + 'EmailField2' => 'email@example.com', + ], + 'isValid' => true, + ], + ]; + } + + /** + * @dataProvider provideValidation + */ + public function testValidation(array $values, bool $isValid) + { + $fieldList = new FieldList([ + $field1 = new EmailField('EmailField1'), + $field2 = new EmailField('EmailField2'), + ]); + if (array_key_exists('EmailField1', $values)) { + $field1->setValue($values['EmailField1']); + } + if (array_key_exists('EmailField2', $values)) { + $field2->setValue($values['EmailField2']); + } + $form = new Form(null, 'testForm', $fieldList, new FieldList([/* no actions */]), new FieldsValidator()); + + $result = $form->validationResult(); + $this->assertSame($isValid, $result->isValid()); + $messages = $result->getMessages(); + if ($isValid) { + $this->assertEmpty($messages); + } else { + $this->assertNotEmpty($messages); + } + } +} diff --git a/tests/php/Security/GroupTest.php b/tests/php/Security/GroupTest.php index ae4f8f34e..87112bbd8 100644 --- a/tests/php/Security/GroupTest.php +++ b/tests/php/Security/GroupTest.php @@ -290,10 +290,11 @@ class GroupTest extends FunctionalTest $newGroup = new Group(); - $validators = $newGroup->getCMSCompositeValidator()->getValidators(); + $validators = $newGroup->getCMSCompositeValidator()->getValidatorsByType(RequiredFields::class); $this->assertCount(1, $validators); - $this->assertInstanceOf(RequiredFields::class, $validators[0]); - $this->assertTrue(in_array('Title', $validators[0]->getRequired() ?? [])); + $validator = array_shift($validators); + $this->assertInstanceOf(RequiredFields::class, $validator); + $this->assertTrue(in_array('Title', $validator->getRequired() ?? [])); $newGroup->Title = $group1->Title; $result = $newGroup->validate(); From 037168a4fe9759b7f464ee8403dab612a570bab6 Mon Sep 17 00:00:00 2001 From: Sabina Talipova Date: Mon, 7 Aug 2023 16:23:46 +1200 Subject: [PATCH 12/13] FIX Multi HTML entities in shortcodes --- src/Forms/HTMLEditor/HTMLEditorField.php | 10 +++++++++ .../Forms/HTMLEditor/HTMLEditorFieldTest.php | 21 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/Forms/HTMLEditor/HTMLEditorField.php b/src/Forms/HTMLEditor/HTMLEditorField.php index 9075a5895..63ef950c2 100644 --- a/src/Forms/HTMLEditor/HTMLEditorField.php +++ b/src/Forms/HTMLEditor/HTMLEditorField.php @@ -190,4 +190,14 @@ class HTMLEditorField extends TextareaField $stateDefaults['data'] = $config->getConfigSchemaData(); return $stateDefaults; } + + /** + * Return value with all values encoded in html entities + * + * @return string Raw HTML + */ + public function ValueEntities() + { + return htmlentities($this->Value() ?? '', ENT_COMPAT, 'UTF-8', false); + } } diff --git a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php index 8fadaa8c6..68ff44b51 100644 --- a/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php +++ b/tests/php/Forms/HTMLEditor/HTMLEditorFieldTest.php @@ -208,4 +208,25 @@ EOS $readonlyContent->getValue() ); } + + public function testValueEntities() + { + $inputText = "The company & partners"; + $field = new HTMLEditorField("Content"); + $field->setValue($inputText); + + $this->assertEquals( + "The company & partners", + $field->obj('ValueEntities')->forTemplate() + ); + + $inputText = "The company && partners"; + $field = new HTMLEditorField("Content"); + $field->setValue($inputText); + + $this->assertEquals( + "The company && partners", + $field->obj('ValueEntities')->forTemplate() + ); + } } From 8da4aa86379a94cffe51e3dc5f7eb0af8549234c Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Wed, 9 Aug 2023 15:20:54 +0100 Subject: [PATCH 13/13] FIX: Regression with include argument (fixes #10911) --- src/View/SSViewer_DataPresenter.php | 2 +- tests/php/View/SSViewerTest.php | 27 ++++++++++++++----- .../SSViewerTestIncludeWithArguments.ss | 2 +- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/View/SSViewer_DataPresenter.php b/src/View/SSViewer_DataPresenter.php index b808773a3..9a65d01fa 100644 --- a/src/View/SSViewer_DataPresenter.php +++ b/src/View/SSViewer_DataPresenter.php @@ -377,7 +377,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope // Check if the method to-be-called exists on the target object - if so, don't check any further // injection locations $on = $this->itemIterator ? $this->itemIterator->current() : $this->item; - if ($on !== null && (isset($on->$property) || method_exists($on, $property ?? ''))) { + if (is_object($on) && (isset($on->$property) || method_exists($on, $property ?? ''))) { return []; } diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index 457c64b98..f7314c8a1 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -1035,22 +1035,22 @@ after' { $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments %>'), - '

[out:Arg1]

[out:Arg2]

' + '

[out:Arg1]

[out:Arg2]

[out:Arg2.Count]

' ); $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments Arg1=A %>'), - '

A

[out:Arg2]

' + '

A

[out:Arg2]

[out:Arg2.Count]

' ); $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments Arg1=A, Arg2=B %>'), - '

A

B

' + '

A

B

' ); $this->assertEquals( $this->render('<% include SSViewerTestIncludeWithArguments Arg1=A Bare String, Arg2=B Bare String %>'), - '

A Bare String

B Bare String

' + '

A Bare String

B Bare String

' ); $this->assertEquals( @@ -1058,7 +1058,7 @@ after' '<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=$B %>', new ArrayData(['B' => 'Bar']) ), - '

A

Bar

' + '

A

Bar

' ); $this->assertEquals( @@ -1066,7 +1066,22 @@ after' '<% include SSViewerTestIncludeWithArguments Arg1="A" %>', new ArrayData(['Arg1' => 'Foo', 'Arg2' => 'Bar']) ), - '

A

Bar

' + '

A

Bar

' + ); + + $this->assertEquals( + $this->render('<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=0 %>'), + '

A

0

' + ); + + $this->assertEquals( + $this->render('<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=false %>'), + '

A

' + ); + + $this->assertEquals( + $this->render('<% include SSViewerTestIncludeWithArguments Arg1="A", Arg2=null %>'), + '

A

' ); $this->assertEquals( diff --git a/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss b/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss index 385eb7abf..73004e1b7 100644 --- a/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss +++ b/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestIncludeWithArguments.ss @@ -1 +1 @@ -

$Arg1

$Arg2

+

$Arg1

$Arg2

{$Arg2.Count}