From c308416afaa2646d4b4bd3a4a4534a7dbc3285ac Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 23 May 2018 11:22:22 +1200 Subject: [PATCH 01/13] =?UTF-8?q?FIX=20Add=20macron=20to=20M=C4=81ori=20la?= =?UTF-8?q?nguage=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Dev/Install/InstallConfig.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dev/Install/InstallConfig.php b/src/Dev/Install/InstallConfig.php index 21465392b..1143ff1c0 100644 --- a/src/Dev/Install/InstallConfig.php +++ b/src/Dev/Install/InstallConfig.php @@ -228,7 +228,7 @@ class InstallConfig 'lv_LV' => 'Latvian (Latvia)', 'lt_LT' => 'Lithuanian (Lithuania)', 'ms_MY' => 'Malay (Malaysia)', - 'mi_NZ' => 'Maori (New Zealand)', + 'mi_NZ' => 'Māori (New Zealand)', 'ne_NP' => 'Nepali (Nepal)', 'nb_NO' => 'Norwegian', 'fa_IR' => 'Persian (Iran)', From 063d765e9445331c41d18ccd4fa2df11137461c3 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 29 May 2018 17:26:18 +1200 Subject: [PATCH 02/13] Add test assertion for response instance This prevents middlewares that return null (like the example delegate in this test) from killing a testsuite --- tests/php/Control/DirectorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index 0070f2a50..ac45e6351 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -593,6 +593,7 @@ class DirectorTest extends SapphireTest }, 'http://www.mysite.com:9090/some-url'); // Middleware returns non-exception redirect + $this->assertInstanceOf(HTTPResponse::class, $response); $this->assertEquals('https://www.mysite.com:9090/some-url', $response->getHeader('Location')); $this->assertEquals(301, $response->getStatusCode()); } From 7a8a24d1755249eb2ee3624aa11bb112c4bd896f Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 29 May 2018 17:34:22 +1200 Subject: [PATCH 03/13] Reset force SSL domain/patterns in setup to prevent global state pollution --- tests/php/Control/DirectorTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index ac45e6351..5e34f2cdd 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -31,8 +31,13 @@ class DirectorTest extends SapphireTest Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/'); // Ensure redirects enabled on all environments - CanonicalURLMiddleware::singleton()->setEnabledEnvs(true); + $middleware = CanonicalURLMiddleware::singleton()->setEnabledEnvs(true); $this->expectedRedirect = null; + + // Ensure global state doesn't affect this test + $middleware + ->setForceSSLDomain(null) + ->setForceSSLPatterns([]); } protected function getExtraRoutes() From 8064ed8220fae35d35ac9dd9d717a7283bf303fc Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 1 Jun 2018 17:47:03 +1200 Subject: [PATCH 04/13] FIX Minor updates to unit tests to pass with HTML5 parser and various themes --- tests/php/Forms/EmailFieldTest.php | 17 +-- tests/php/Forms/FormTest.php | 36 +++--- .../GridField/GridFieldDetailFormTest.php | 13 +- tests/php/Forms/LookupFieldTest.php | 113 ++++++++---------- tests/php/Forms/SelectionGroupTest.php | 1 + tests/php/Forms/TreeDropdownFieldTest.php | 18 +-- 6 files changed, 92 insertions(+), 106 deletions(-) diff --git a/tests/php/Forms/EmailFieldTest.php b/tests/php/Forms/EmailFieldTest.php index 9eeaa4915..85733d8a6 100644 --- a/tests/php/Forms/EmailFieldTest.php +++ b/tests/php/Forms/EmailFieldTest.php @@ -67,23 +67,16 @@ class EmailFieldTest extends FunctionalTest * * @see SimpleTagBuilder::_createInputTag() */ - function testEmailFieldPopulation() + public function testEmailFieldPopulation() { - $this->get('EmailFieldTest_Controller'); - $this->submitForm( + + $response = $this->submitForm( 'Form_Form', null, - array( - 'Email' => 'test@test.com' - ) + ['Email' => 'test@test.com'] ); - $this->assertPartialMatchBySelector( - 'p.good', - array( - 'Test save was successful' - ) - ); + $this->assertContains('Test save was successful', $response->getBody()); } } diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index ab7a91367..ae7e1d7bd 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -2,33 +2,31 @@ namespace SilverStripe\Forms\Tests; +use SilverStripe\Control\Controller; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Session; -use SilverStripe\Core\Config\Config; +use SilverStripe\Dev\CSSContentParser; +use SilverStripe\Dev\FunctionalTest; +use SilverStripe\Forms\DateField; +use SilverStripe\Forms\FieldList; +use SilverStripe\Forms\FileField; +use SilverStripe\Forms\Form; +use SilverStripe\Forms\FormAction; +use SilverStripe\Forms\HeaderField; +use SilverStripe\Forms\LookupField; +use SilverStripe\Forms\NumericField; use SilverStripe\Forms\PasswordField; -use SilverStripe\Forms\Tests\FormTest\TestController; use SilverStripe\Forms\Tests\FormTest\ControllerWithSecurityToken; use SilverStripe\Forms\Tests\FormTest\ControllerWithStrictPostCheck; use SilverStripe\Forms\Tests\FormTest\Player; use SilverStripe\Forms\Tests\FormTest\Team; +use SilverStripe\Forms\Tests\FormTest\TestController; +use SilverStripe\Forms\TextareaField; +use SilverStripe\Forms\TextField; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\NullSecurityToken; -use SilverStripe\Security\Security; -use SilverStripe\Security\SecurityToken; use SilverStripe\Security\RandomGenerator; -use SilverStripe\Dev\CSSContentParser; -use SilverStripe\Dev\FunctionalTest; -use SilverStripe\Control\Controller; -use SilverStripe\Control\HTTPRequest; -use SilverStripe\Forms\TextField; -use SilverStripe\Forms\FieldList; -use SilverStripe\Forms\Form; -use SilverStripe\Forms\HeaderField; -use SilverStripe\Forms\TextareaField; -use SilverStripe\Forms\DateField; -use SilverStripe\Forms\NumericField; -use SilverStripe\Forms\LookupField; -use SilverStripe\Forms\FileField; -use SilverStripe\Forms\FormAction; +use SilverStripe\Security\SecurityToken; use SilverStripe\View\SSViewer; /** @@ -50,6 +48,8 @@ class FormTest extends FunctionalTest ControllerWithStrictPostCheck::class, ]; + protected static $disable_themes = true; + protected function setUp() { parent::setUp(); diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest.php b/tests/php/Forms/GridField/GridFieldDetailFormTest.php index 81d425867..9bc0226d8 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest.php @@ -2,20 +2,19 @@ namespace SilverStripe\Forms\Tests\GridField; -use SilverStripe\Dev\CSSContentParser; -use SilverStripe\Dev\Debug; -use SilverStripe\Dev\FunctionalTest; use SilverStripe\Control\Controller; -use SilverStripe\Forms\HiddenField; -use SilverStripe\Forms\GridField\GridFieldDetailForm; +use SilverStripe\Dev\CSSContentParser; +use SilverStripe\Dev\FunctionalTest; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\Forms\GridField\GridFieldDetailForm; use SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest; +use SilverStripe\Forms\HiddenField; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Category; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\CategoryController; -use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\TestController; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\GroupController; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\PeopleGroup; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Person; +use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\TestController; /** * @skipUpgrade @@ -36,6 +35,8 @@ class GridFieldDetailFormTest extends FunctionalTest GroupController::class, ]; + protected static $disable_themes = true; + public function testValidator() { $this->logInWithPermission('ADMIN'); diff --git a/tests/php/Forms/LookupFieldTest.php b/tests/php/Forms/LookupFieldTest.php index c41b1b141..1f53a8234 100644 --- a/tests/php/Forms/LookupFieldTest.php +++ b/tests/php/Forms/LookupFieldTest.php @@ -5,90 +5,85 @@ namespace SilverStripe\Forms\Tests; use SilverStripe\ORM\DataObject; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\LookupField; +use SilverStripe\Security\Member; class LookupFieldTest extends SapphireTest { - protected static $fixture_file = 'LookupFieldTest.yml'; public function testNullValueWithNumericArraySource() { $source = array(1 => 'one', 2 => 'two', 3 => 'three'); - $f = new LookupField('test', 'test', $source); - $f->setValue(null); + $field = new LookupField('test', 'test', $source); + $field->setValue(null); + $result = trim($field->Field()->getValue()); - $this->assertEquals( - '(none)', - trim($f->Field()->getValue()) - ); + $this->assertContains('(none)', $result); + $this->assertContains('', $result); } public function testStringValueWithNumericArraySource() { $source = array(1 => 'one', 2 => 'two', 3 => 'three'); - $f = new LookupField('test', 'test', $source); - $f->setValue(1); - $this->assertEquals( - 'one', - trim($f->Field()->getValue()) - ); + $field = new LookupField('test', 'test', $source); + $field->setValue(1); + $result = trim($field->Field()->getValue()); + $this->assertContains('one', $result); + $this->assertContains('', $result); } public function testUnknownStringValueWithNumericArraySource() { $source = array(1 => 'one', 2 => 'two', 3 => 'three'); - $f = new LookupField('test', 'test', $source); - $f->setValue('w00t'); - $this->assertEquals( - 'w00t', - trim($f->Field()->getValue()) - ); + $field = new LookupField('test', 'test', $source); + $field->setValue('w00t'); + $result = trim($field->Field()->getValue()); + + $this->assertContains('w00t', $result); + $this->assertContains('', $result); } public function testArrayValueWithAssociativeArraySource() { // Array values (= multiple selections) might be set e.g. from ListboxField $source = array('one' => 'one val', 'two' => 'two val', 'three' => 'three val'); - $f = new LookupField('test', 'test', $source); - $f->setValue(array('one','two')); - $this->assertEquals( - 'one val, two val' - . '', - trim($f->Field()->getValue()) - ); + $field = new LookupField('test', 'test', $source); + $field->setValue(array('one','two')); + $result = trim($field->Field()->getValue()); + + $this->assertContains('one val, two val', $result); + $this->assertContains('', $result); } public function testArrayValueWithNumericArraySource() { // Array values (= multiple selections) might be set e.g. from ListboxField $source = array(1 => 'one', 2 => 'two', 3 => 'three'); - $f = new LookupField('test', 'test', $source); - $f->setValue(array(1,2)); - $this->assertEquals( - 'one, two', - trim($f->Field()->getValue()) - ); + $field = new LookupField('test', 'test', $source); + $field->setValue(array(1,2)); + $result = trim($field->Field()->getValue()); + + $this->assertContains('one, two', $result); + $this->assertContains('', $result); } public function testArrayValueWithSqlMapSource() { - $member1 = $this->objFromFixture('SilverStripe\\Security\\Member', 'member1'); - $member2 = $this->objFromFixture('SilverStripe\\Security\\Member', 'member2'); - $member3 = $this->objFromFixture('SilverStripe\\Security\\Member', 'member3'); + $member1 = $this->objFromFixture(Member::class, 'member1'); + $member2 = $this->objFromFixture(Member::class, 'member2'); + $member3 = $this->objFromFixture(Member::class, 'member3'); - $source = DataObject::get('SilverStripe\\Security\\Member'); - $f = new LookupField('test', 'test', $source->map('ID', 'FirstName')); - $f->setValue(array($member1->ID, $member2->ID)); + $source = DataObject::get(Member::class); + $field = new LookupField('test', 'test', $source->map('ID', 'FirstName')); + $field->setValue(array($member1->ID, $member2->ID)); + $result = trim($field->Field()->getValue()); - $this->assertEquals( - sprintf( - 'member1, member2' - . '', - $member1->ID, - $member2->ID - ), - trim($f->Field()->getValue()) - ); + $this->assertContains('member1, member2', $result); + $this->assertContains(sprintf( + '', + $member1->ID, + $member2->ID + ), $result); } public function testWithMultiDimensionalSource() @@ -105,23 +100,17 @@ class LookupFieldTest extends SapphireTest ) ); - $f = new LookupField('test', 'test', $choices); - $f->setValue(3); + $field = new LookupField('test', 'test', $choices); + $field->setValue(3); + $result = trim($field->Field()->getValue()); - $this->assertEquals( - 'Carrots', - trim($f->Field()->getValue()) - ); + $this->assertContains('Carrots', $result); + $this->assertContains('', $result); - $f->setValue( - array( - 3, 9 - ) - ); + $field->setValue([3, 9]); + $result = trim($field->Field()->getValue()); - $this->assertEquals( - 'Carrots, Vegan', - trim($f->Field()->getValue()) - ); + $this->assertContains('Carrots, Vegan', $result); + $this->assertContains('', $result); } } diff --git a/tests/php/Forms/SelectionGroupTest.php b/tests/php/Forms/SelectionGroupTest.php index 3c1d3e441..82ba88520 100644 --- a/tests/php/Forms/SelectionGroupTest.php +++ b/tests/php/Forms/SelectionGroupTest.php @@ -58,6 +58,7 @@ class SelectionGroupTest extends SapphireTest $field->setValue('two'); $parser = new CSSContentParser($field->FieldHolder()); + $listEls = $parser->getBySelector('li'); $listElOne = $listEls[0]; $listElTwo = $listEls[1]; diff --git a/tests/php/Forms/TreeDropdownFieldTest.php b/tests/php/Forms/TreeDropdownFieldTest.php index dfc49bbce..5b5e22fd1 100644 --- a/tests/php/Forms/TreeDropdownFieldTest.php +++ b/tests/php/Forms/TreeDropdownFieldTest.php @@ -196,15 +196,17 @@ class TreeDropdownFieldTest extends SapphireTest public function testReadonly() { $field = new TreeDropdownField('TestTree', 'Test tree', File::class); - $asdf = $this->objFromFixture(File::class, 'asdf'); - $field->setValue($asdf->ID); + $fileMock = $this->objFromFixture(File::class, 'asdf'); + $field->setValue($fileMock->ID); $readonlyField = $field->performReadonlyTransformation(); - $this->assertEquals( - <<<"HTML" -<Special & characters> -HTML - , - (string)$readonlyField->Field() + $result = (string) $readonlyField->Field(); + $this->assertContains( + '<Special & characters>', + $result + ); + $this->assertContains( + '', + $result ); } } From 5a5ba1e5c001de161fbeb19d6d662391dccc4c1e Mon Sep 17 00:00:00 2001 From: Jonathon Menz Date: Fri, 1 Jun 2018 12:52:22 -0700 Subject: [PATCH 05/13] Fix: negative values in read only currency field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don’t strip out ‘-‘ character as this makes negative values appear to be positive (Fixes #8126) --- src/Forms/CurrencyField_Disabled.php | 2 +- src/Forms/CurrencyField_Readonly.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Forms/CurrencyField_Disabled.php b/src/Forms/CurrencyField_Disabled.php index 26a5106f8..7e743755e 100644 --- a/src/Forms/CurrencyField_Disabled.php +++ b/src/Forms/CurrencyField_Disabled.php @@ -22,7 +22,7 @@ class CurrencyField_Disabled extends CurrencyField { if ($this->value) { $val = Convert::raw2xml($this->value); - $val = _t('SilverStripe\\Forms\\CurrencyField.CURRENCYSYMBOL', '$') . number_format(preg_replace('/[^0-9.]/', "", $val), 2); + $val = _t('SilverStripe\\Forms\\CurrencyField.CURRENCYSYMBOL', '$') . number_format(preg_replace('/[^0-9.-]/', "", $val), 2); $valforInput = Convert::raw2att($val); } else { $valforInput = ''; diff --git a/src/Forms/CurrencyField_Readonly.php b/src/Forms/CurrencyField_Readonly.php index 866c09c61..775b9eb6b 100644 --- a/src/Forms/CurrencyField_Readonly.php +++ b/src/Forms/CurrencyField_Readonly.php @@ -20,7 +20,7 @@ class CurrencyField_Readonly extends ReadonlyField { if ($this->value) { $val = Convert::raw2xml($this->value); - $val = _t('SilverStripe\\Forms\\CurrencyField.CURRENCYSYMBOL', '$') . number_format(preg_replace('/[^0-9.]/', "", $val), 2); + $val = _t('SilverStripe\\Forms\\CurrencyField.CURRENCYSYMBOL', '$') . number_format(preg_replace('/[^0-9.-]/', "", $val), 2); $valforInput = Convert::raw2att($val); } else { $val = '' . _t('SilverStripe\\Forms\\CurrencyField.CURRENCYSYMBOL', '$') . '0.00'; From 1cbf27e0f47c3547914b03193d0f5f77c87ff8d5 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 29 May 2018 14:16:10 +1200 Subject: [PATCH 06/13] FIX PHP 5.3 compat for referencing $this in closure, and make method public for same reason sdf --- security/Member.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/Member.php b/security/Member.php index cd1d22518..ea06793df 100644 --- a/security/Member.php +++ b/security/Member.php @@ -1052,7 +1052,7 @@ class Member extends DataObject implements TemplateGlobalProvider { * * @return int[] List of group IDs */ - protected function disallowedGroups() { + public function disallowedGroups() { // unless the current user is an admin already OR the logged in user is an admin if (Permission::check('ADMIN') || Permission::checkMember($this, 'ADMIN')) { return array(); @@ -1476,7 +1476,7 @@ class Member extends DataObject implements TemplateGlobalProvider { if(Permission::check('EDIT_PERMISSIONS')) { // Filter allowed groups $groups = Group::get(); - $disallowedGroupIDs = $this->disallowedGroups(); + $disallowedGroupIDs = $self->disallowedGroups(); if ($disallowedGroupIDs) { $groups = $groups->exclude('ID', $disallowedGroupIDs); } From c1b0c56788a3ca230cbc76a2f38ee4300a678730 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Sat, 2 Jun 2018 20:44:33 +1200 Subject: [PATCH 07/13] Increase memory limit to 2G in Travis builds --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 3cb849f4a..77a77322e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,7 @@ before_script: - composer self-update || true - phpenv rehash - phpenv config-rm xdebug.ini + - echo 'memory_limit = 2G' >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini - git clone git://github.com/silverstripe-labs/silverstripe-travis-support.git ~/travis-support - "if [ \"$BEHAT_TEST\" = \"\" ] && [ \"$CMS_TEST\" = \"\" ]; then php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss; fi" - "if [ \"$BEHAT_TEST\" = \"1\" ] && [ \"$CMS_TEST\" = \"\" ]; then php ~/travis-support/travis_setup.php --source `pwd` --target ~/builds/ss --require silverstripe/behat-extension; fi" From 41e601a036307065d9ea2ba8862f67be738d402f Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Mon, 4 Jun 2018 15:17:05 +0100 Subject: [PATCH 08/13] FIX Regression from #8009 --- tests/model/DataObjectDuplicationTest.php | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/tests/model/DataObjectDuplicationTest.php b/tests/model/DataObjectDuplicationTest.php index 2fcab1c4d..7f73bf6a2 100644 --- a/tests/model/DataObjectDuplicationTest.php +++ b/tests/model/DataObjectDuplicationTest.php @@ -83,6 +83,13 @@ class DataObjectDuplicationTest extends SapphireTest { $two = DataObject::get_by_id("DataObjectDuplicateTestClass2", $two->ID); $three = DataObject::get_by_id("DataObjectDuplicateTestClass3", $three->ID); + $this->assertCount(1, $one->twos(), + "Many-to-one relation not copied (has_many)"); + $this->assertCount(1, $one->threes(), + "Object has the correct number of relations"); + $this->assertCount(1, $three->ones(), + "Object has the correct number of relations"); + //test duplication $oneCopy = $one->duplicate(); $twoCopy = $two->duplicate(); @@ -100,16 +107,16 @@ class DataObjectDuplicationTest extends SapphireTest { $this->assertEquals($text2, $twoCopy->text); $this->assertEquals($text3, $threeCopy->text); - $this->assertNotEquals($one->twos()->Count(), $oneCopy->twos()->Count(), + $this->assertCount(0, $oneCopy->twos(), "Many-to-one relation not copied (has_many)"); - $this->assertEquals($one->threes()->Count(), $oneCopy->threes()->Count(), + $this->assertCount(2, $oneCopy->threes(), "Object has the correct number of relations"); - $this->assertEquals($three->ones()->Count(), $threeCopy->ones()->Count(), + $this->assertCount(2, $threeCopy->ones(), "Object has the correct number of relations"); $this->assertEquals($one->ID, $twoCopy->one()->ID, "Match between relation of copy and the original"); - $this->assertEquals(0, $oneCopy->twos()->Count(), + $this->assertCount(0, $oneCopy->twos(), "Many-to-one relation not copied (has_many)"); $this->assertEquals($three->ID, $oneCopy->threes()->First()->ID, "Match between relation of copy and the original"); @@ -142,6 +149,8 @@ class DataObjectDuplicateTestClass1 extends DataObject implements TestOnly { 'TestExtra' => 'Varchar' ) ); + + private static $default_sort = '"ID" ASC'; } class DataObjectDuplicateTestClass2 extends DataObject implements TestOnly { @@ -154,6 +163,8 @@ class DataObjectDuplicateTestClass2 extends DataObject implements TestOnly { 'one' => 'DataObjectDuplicateTestClass1' ); + private static $default_sort = '"ID" ASC'; + } class DataObjectDuplicateTestClass3 extends DataObject implements TestOnly { @@ -165,6 +176,8 @@ class DataObjectDuplicateTestClass3 extends DataObject implements TestOnly { private static $belongs_many_many = array( 'ones' => 'DataObjectDuplicateTestClass1' ); + + private static $default_sort = '"ID" ASC'; } From 0aa13da0d99a94a0a6dc4cc8cf42e37abcf5374a Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 6 Jun 2018 09:43:04 +1200 Subject: [PATCH 09/13] BUG Backport bugfix for belongs_many_many with many_many through. Partial backport of #7928 Fixes #8131 --- src/ORM/DataObjectSchema.php | 14 +++++++++++--- tests/php/ORM/ManyManyThroughListTest/Item.php | 3 ++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/ORM/DataObjectSchema.php b/src/ORM/DataObjectSchema.php index e309a1660..8251c8477 100644 --- a/src/ORM/DataObjectSchema.php +++ b/src/ORM/DataObjectSchema.php @@ -982,9 +982,17 @@ class DataObjectSchema if (!$otherManyMany) { return null; } - foreach ($otherManyMany as $inverseComponentName => $nextClass) { - if ($nextClass === $parentClass) { - return $inverseComponentName; + foreach ($otherManyMany as $inverseComponentName => $manyManySpec) { + // Normal many-many + if ($manyManySpec === $parentClass) { + return $inverseComponentName; + } + // many-many through, inspect 'to' for the many_many + if (is_array($manyManySpec)) { + $toClass = $this->hasOneComponent($manyManySpec['through'], $manyManySpec['to']); + if ($toClass === $parentClass) { + return $inverseComponentName; + } } } return null; diff --git a/tests/php/ORM/ManyManyThroughListTest/Item.php b/tests/php/ORM/ManyManyThroughListTest/Item.php index e717b3c84..9f96e7d14 100644 --- a/tests/php/ORM/ManyManyThroughListTest/Item.php +++ b/tests/php/ORM/ManyManyThroughListTest/Item.php @@ -19,6 +19,7 @@ class Item extends DataObject implements TestOnly ]; private static $belongs_many_many = [ - 'Objects' => 'SilverStripe\\ORM\\Tests\\ManyManyThroughListTest\\TestObject.Items' + // Intentionally omit parent `.Items` specifier to ensure it's not mandatory + 'Objects' => TestObject::class, ]; } From 66f57bd4dac0bd4c8106f8071ddc45103c2643f2 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Thu, 7 Jun 2018 10:24:27 +0100 Subject: [PATCH 10/13] FIX: Only set MYSQL_ATTR_INIT_COMMAND when using mysql driver (fixes #8103) --- src/ORM/Connect/PDOConnector.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ORM/Connect/PDOConnector.php b/src/ORM/Connect/PDOConnector.php index ca4cf309f..f3fbae164 100644 --- a/src/ORM/Connect/PDOConnector.php +++ b/src/ORM/Connect/PDOConnector.php @@ -176,9 +176,11 @@ class PDOConnector extends DBConnector if (!isset($charset)) { $charset = $connCharset; } - $options = array( - PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES ' . $charset . ' COLLATE ' . $connCollation - ); + + $options = []; + if ($parameters['driver'] === 'mysql') { + $options[PDO::MYSQL_ATTR_INIT_COMMAND] = 'SET NAMES ' . $charset . ' COLLATE ' . $connCollation; + } // Set SSL options if they are defined if (array_key_exists('ssl_key', $parameters) && From e37e3e1746e56c866ee875f41a7fddf61c926d9f Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 8 Jun 2018 11:23:24 +1200 Subject: [PATCH 11/13] BUG Fix test that relies on implicit ID order breaking postgres --- tests/php/ORM/DataObjectDuplicationTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/php/ORM/DataObjectDuplicationTest.php b/tests/php/ORM/DataObjectDuplicationTest.php index 09c88d821..57e5b8bb7 100644 --- a/tests/php/ORM/DataObjectDuplicationTest.php +++ b/tests/php/ORM/DataObjectDuplicationTest.php @@ -151,14 +151,14 @@ class DataObjectDuplicationTest extends SapphireTest $oneCopy->twos()->Count(), "Many-to-one relation not copied (has_many)" ); - $this->assertEquals( + $this->assertContains( $three->ID, - $oneCopy->threes()->First()->ID, + $oneCopy->threes()->column('ID'), "Match between relation of copy and the original" ); - $this->assertEquals( + $this->assertContains( $one->ID, - $threeCopy->ones()->First()->ID, + $threeCopy->ones()->column('ID'), "Match between relation of copy and the original" ); From 29f9b1c18fb38dab912a0b9dcae63eacae19335d Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 8 Jun 2018 11:38:36 +1200 Subject: [PATCH 12/13] Fix linting issues --- src/Forms/HTMLEditor/HTMLEditorSanitiser.php | 19 ++++++------------- src/ORM/Queries/SQLSelect.php | 3 +-- src/View/Parsers/ShortcodeParser.php | 15 +++++---------- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/Forms/HTMLEditor/HTMLEditorSanitiser.php b/src/Forms/HTMLEditor/HTMLEditorSanitiser.php index ca4e5f407..343e8253b 100644 --- a/src/Forms/HTMLEditor/HTMLEditorSanitiser.php +++ b/src/Forms/HTMLEditor/HTMLEditorSanitiser.php @@ -4,7 +4,6 @@ namespace SilverStripe\Forms\HTMLEditor; use DOMAttr; use DOMElement; -use DOMNode; use SilverStripe\Core\Injector\Injectable; use SilverStripe\View\Parsers\HTMLValue; use stdClass; @@ -114,24 +113,20 @@ class HTMLEditorSanitiser if ($attrType === '!') { $element->attributesRequired[] = $attrName; $attr->required = true; - } // Denied from global - elseif ($attrType === '-') { + } elseif ($attrType === '-') { // Denied from global unset($element->attributes[$attrName]); continue; } // Default value if ($prefix) { - // Default value - if ($prefix === '=') { + if ($prefix === '=') { // Default value $element->attributesDefault[$attrName] = $value; $attr->defaultValue = $value; - } // Forced value - elseif ($prefix === ':') { + } elseif ($prefix === ':') { // Forced value $element->attributesForced[$attrName] = $value; $attr->forcedValue = $value; - } // Required values - elseif ($prefix === '<') { + } elseif ($prefix === '<') { // Required values $attr->validValues = explode('?', $value); } } @@ -290,8 +285,7 @@ class HTMLEditorSanitiser // If it's a script or style, we don't keep contents if ($el->tagName === 'script' || $el->tagName === 'style') { $el->parentNode->removeChild($el); - } // Otherwise we replace this node with all it's children - else { + } else { // Otherwise we replace this node with all it's children // First, create a new fragment with all of $el's children moved into it $frag = $doc->createDocumentFragment(); while ($el->firstChild) { @@ -301,8 +295,7 @@ class HTMLEditorSanitiser // Then replace $el with the frags contents (which used to be it's children) $el->parentNode->replaceChild($frag, $el); } - } // Otherwise tidy the element - else { + } else { // Otherwise tidy the element // First, if we're supposed to pad & this element is empty, fix that if ($elementRule->paddEmpty && !$el->firstChild) { $el->nodeValue = ' '; diff --git a/src/ORM/Queries/SQLSelect.php b/src/ORM/Queries/SQLSelect.php index 8eef054e4..50f67c3f3 100644 --- a/src/ORM/Queries/SQLSelect.php +++ b/src/ORM/Queries/SQLSelect.php @@ -622,8 +622,7 @@ class SQLSelect extends SQLConditionalExpression if (!empty($this->having)) { $records = $this->execute(); return $records->numRecords(); - } // Choose a default column - elseif ($column == null) { + } elseif ($column == null) { // Choose a default column if ($this->groupby) { $column = 'DISTINCT ' . implode(", ", $this->groupby); } else { diff --git a/src/View/Parsers/ShortcodeParser.php b/src/View/Parsers/ShortcodeParser.php index 37be5c631..0a70ea089 100644 --- a/src/View/Parsers/ShortcodeParser.php +++ b/src/View/Parsers/ShortcodeParser.php @@ -573,8 +573,7 @@ class ShortcodeParser } elseif ($location == self::AFTER) { // Move after block parent $this->insertAfter($node, $parent); - } // Split parent at node - elseif ($location == self::SPLIT) { + } elseif ($location == self::SPLIT) { // Split parent at node $at = $node; $splitee = $node->parentNode; @@ -593,8 +592,7 @@ class ShortcodeParser } $this->insertAfter($node, $parent); - } // Do nothing - elseif ($location == self::INLINE) { + } elseif ($location == self::INLINE) { // Do nothing if (in_array(strtolower($node->tagName), self::$block_level_elements)) { user_error( 'Requested to insert block tag ' . $node->tagName . ' inline - probably this will break HTML compliance', @@ -638,7 +636,6 @@ class ShortcodeParser */ public function parse($content) { - $this->extend('onBeforeParse', $content); $continue = true; @@ -646,11 +643,9 @@ class ShortcodeParser // If no shortcodes defined, don't try and parse any if (!$this->shortcodes) { $continue = false; - } // If no content, don't try and parse it - elseif (!trim($content)) { + } elseif (!trim($content)) { // If no content, don't try and parse it $continue = false; - } // If no shortcode tag, don't try and parse it - elseif (strpos($content, '[') === false) { + } elseif (strpos($content, '[') === false) { // If no shortcode tag, don't try and parse it $continue = false; } @@ -659,7 +654,7 @@ class ShortcodeParser // use a proper DOM list($content, $tags) = $this->replaceElementTagsWithMarkers($content); - /** @var HTMLValue $htmlvalue */ + /** @var HTMLValue $htmlvalue */ $htmlvalue = Injector::inst()->create('HTMLValue', $content); // Now parse the result into a DOM From 1d6d601050aef6a1ff2def631f34564f3448c771 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Fri, 8 Jun 2018 14:00:59 +0100 Subject: [PATCH 13/13] Use chaining syntax for setting up middleware --- tests/php/Control/DirectorTest.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php index 5e34f2cdd..590dfc067 100644 --- a/tests/php/Control/DirectorTest.php +++ b/tests/php/Control/DirectorTest.php @@ -30,14 +30,12 @@ class DirectorTest extends SapphireTest parent::setUp(); Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/'); - // Ensure redirects enabled on all environments - $middleware = CanonicalURLMiddleware::singleton()->setEnabledEnvs(true); - $this->expectedRedirect = null; - - // Ensure global state doesn't affect this test - $middleware + // Ensure redirects enabled on all environments and global state doesn't affect the tests + CanonicalURLMiddleware::singleton() ->setForceSSLDomain(null) - ->setForceSSLPatterns([]); + ->setForceSSLPatterns([]) + ->setEnabledEnvs(true); + $this->expectedRedirect = null; } protected function getExtraRoutes()