From 3fcf1e2c98629dcd0048ff9447bad4cd30b4bf95 Mon Sep 17 00:00:00 2001 From: Mark Stephens Date: Fri, 29 Jan 2016 10:22:18 +1300 Subject: [PATCH] BUG edge case on many many extra fields (fixes 4991) Fixes an edge case where extraFields are not returned if one side of a many many is added via extension (although this may not be the only failure case). Fixes a downstream issue with dms breaking the CMS on framework 3.2. The bug is where a many many relationship exists on a class, and a sub-class attempts to get the extra fields of the relationship. The change fixes the test for exact matching of the relationship class to the instance class, to checking if the instance is the class or a subclass of the relationship. The unit tests check the dms failure case, which is a more complex failure case. --- model/DataObject.php | 2 +- tests/model/ManyManyListExtensionTest.php | 119 ++++++++++++++++++++++ tests/model/ManyManyListExtensionTest.yml | 6 ++ 3 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 tests/model/ManyManyListExtensionTest.php create mode 100644 tests/model/ManyManyListExtensionTest.yml diff --git a/model/DataObject.php b/model/DataObject.php index 618ce6648..8381fbf3b 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -2077,7 +2077,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $candidateManyManys = (array)Config::inst()->get($candidate, 'many_many', Config::UNINHERITED); foreach($candidateManyManys as $relation => $relatedClass) { - if($relatedClass === $this->class) { + if (is_a($this, $relatedClass)) { $relationName = $relation; } } diff --git a/tests/model/ManyManyListExtensionTest.php b/tests/model/ManyManyListExtensionTest.php new file mode 100644 index 000000000..4b37e7848 --- /dev/null +++ b/tests/model/ManyManyListExtensionTest.php @@ -0,0 +1,119 @@ +objFromFixture('ManyManyListTest_IndirectPrimary', 'manymany_extra_primary'); + $secondaries = $primary->Secondary(); + $extraFields = $secondaries->getExtraFields(); + + $this->assertTrue(count($extraFields) > 0, 'has extra fields'); + $this->assertTrue(isset($extraFields['DocumentSort']), 'has DocumentSort'); + + // Test from the secondary (which is extended) to the primary (not extended) + $secondary = $this->objFromFixture('ManyManyListTest_SecondarySub', 'manymany_extra_secondary'); + + $primaries = $secondary->Primary(); + $extraFields = $primaries->getExtraFields(); + + $this->assertTrue(count($extraFields) > 0, 'has extra fields'); + $this->assertTrue(isset($extraFields['DocumentSort']), 'has DocumentSort'); + } +} + +/** + * @package framework + * @subpackage tests + * + * A data object that implements the primary side of a many_many (where the extra fields are + * defined.) The many-many refers to ManyManyListTest_Secondary rather than ManyManyListTest_SecondarySub + * by design, because we're trying to test that a subclass instance picks up the extra fields of it's parent. + */ +class ManyManyListTest_IndirectPrimary extends DataObject implements TestOnly { + + private static $db = array( + 'Title' => 'Varchar(255)' + ); + + private static $many_many = array( + 'Secondary' => 'ManyManyListTest_Secondary' + ); + + private static $many_many_extraFields = array( + 'Secondary' => array( + 'DocumentSort' => 'Int' + ) + ); +} + +/** + * @package framework + * @subpackage tests + * + * A data object that implements the secondary side of a many_many when extended by + * ManyManyListTest_IndirectSecondaryExtension. + */ +class ManyManyListTest_Secondary extends DataObject implements TestOnly { + + // Possibly not required, but want to simulate a real test failure case where + // database tables are present. + private static $db = array( + 'Title' => 'Varchar(255)' + ); + +} + +/** + * @package framework + * @subpackage tests + * + * A data object that is a subclass of the secondary side. The test will create an instance of this, + * and ensure that the extra fields are available on the instance even though the many many is + * defined at the parent level. + */ +class ManyManyListTest_SecondarySub extends ManyManyListTest_Secondary { + + // private static $db = array( + // 'Other' => 'Varchar(255)' + // ); + +} + +/** + * @package framework + * @subpackage tests + * + * An extension that is applied to ManyManyListTest_Secondary that implements the other side of the many-many + * relationship. + */ +class ManyManyListTest_IndirectSecondaryExtension extends DataExtension implements TestOnly { + + private static $db = array( + 'Title' => 'Varchar(255)' + ); + + private static $belongs_many_many = array( + 'Primary' => 'ManyManyListTest_IndirectPrimary' + ); + +} diff --git a/tests/model/ManyManyListExtensionTest.yml b/tests/model/ManyManyListExtensionTest.yml new file mode 100644 index 000000000..a28c7be68 --- /dev/null +++ b/tests/model/ManyManyListExtensionTest.yml @@ -0,0 +1,6 @@ +ManyManyListTest_IndirectPrimary: + manymany_extra_primary: + Title: 'primary' +ManyManyListTest_SecondarySub: + manymany_extra_secondary: + Title: 'secondary'