Merge pull request #7701 from open-sausages/pulls/4.0/better-summary-merging

BUG Fix incorrect merge of associative / non-associative summary fields
This commit is contained in:
Daniel Hensby 2017-12-14 13:25:51 +00:00 committed by GitHub
commit 7733e9caed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 115 additions and 112 deletions

View File

@ -426,13 +426,13 @@ class Injector implements ContainerInterface
// to ensure we get cached
$spec['id'] = $id;
// We've removed this check because new functionality means that the 'class' field doesn't need to refer
// specifically to a class anymore - it could be a compound statement, ala SilverStripe's old Object::create
// functionality
// We've removed this check because new functionality means that the 'class' field doesn't need to refer
// specifically to a class anymore - it could be a compound statement, ala SilverStripe's old Object::create
// functionality
//
// if (!class_exists($class)) {
// throw new Exception("Failed to load '$class' from $file");
// }
// if (!class_exists($class)) {
// throw new Exception("Failed to load '$class' from $file");
// }
// store the specs for now - we lazy load on demand later on.
$this->specs[$id] = $spec;

View File

@ -136,11 +136,11 @@ abstract class BulkLoader extends ViewableData
}
/*
* Load the given file via {@link self::processAll()} and {@link self::processRecord()}.
* Optionally truncates (clear) the table before it imports.
*
* @return BulkLoader_Result See {@link self::processAll()}
*/
* Load the given file via {@link self::processAll()} and {@link self::processRecord()}.
* Optionally truncates (clear) the table before it imports.
*
* @return BulkLoader_Result See {@link self::processAll()}
*/
public function load($filepath)
{
Environment::increaseTimeLimitTo(3600);

View File

@ -140,8 +140,8 @@ class DevelopmentAdmin extends Controller
/*
* Internal methods
*/
* Internal methods
*/
/**
* @return array of url => description
@ -175,8 +175,8 @@ class DevelopmentAdmin extends Controller
/*
* Unregistered (hidden) actions
*/
* Unregistered (hidden) actions
*/
/**
* Build the default data, calling requireDefaultRecords on all

View File

@ -688,7 +688,7 @@ class FieldList extends ArrayList
$fieldMap[$field->getName()] = $field;
}
// Iterate through the ordered list of names, building a new array to be put into $this->items.
// Iterate through the ordered list of names, building a new array to be put into $this->items.
// While we're doing this, empty out $fieldMap so that we can keep track of leftovers.
// Unrecognised field names are okay; just ignore them
$fields = array();

View File

@ -890,13 +890,13 @@ MESSAGE
/*
* This is a lookup table for data types.
* For instance, Postgres uses 'INT', while MySQL uses 'UNSIGNED'
* So this is a DB-specific list of equivilents.
*
* @param string $type
* @return string
*/
* This is a lookup table for data types.
* For instance, Postgres uses 'INT', while MySQL uses 'UNSIGNED'
* So this is a DB-specific list of equivilents.
*
* @param string $type
* @return string
*/
abstract public function dbDataType($type);
/**
@ -1151,10 +1151,10 @@ MESSAGE
abstract public function varchar($values);
/*
* Returns data type for 'year' column
*
* @param array $values Contains a tokenised list of info about this data type
* @return string
*/
* Returns data type for 'year' column
*
* @param array $values Contains a tokenised list of info about this data type
* @return string
*/
abstract public function year($values);
}

View File

@ -617,14 +617,14 @@ abstract class Database
}
/*
* Determines if the current database connection supports a given list of extensions
*
* @param array $extensions List of extensions to check for support of. The key of this array
* will be an extension name, and the value the configuration for that extension. This
* could be one of partitions, tablespaces, or clustering
* @return boolean Flag indicating support for all of the above
* @todo Write test cases
*/
* Determines if the current database connection supports a given list of extensions
*
* @param array $extensions List of extensions to check for support of. The key of this array
* will be an extension name, and the value the configuration for that extension. This
* could be one of partitions, tablespaces, or clustering
* @return boolean Flag indicating support for all of the above
* @todo Write test cases
*/
public function supportsExtensions($extensions)
{
return false;

View File

@ -614,11 +614,11 @@ class MySQLSchemaManager extends DBSchemaManager
}
/*
* Return the MySQL-proprietary 'Year' datatype
*
* @param array $values Contains a tokenised list of info about this data type
* @return string
*/
* Return the MySQL-proprietary 'Year' datatype
*
* @param array $values Contains a tokenised list of info about this data type
* @return string
*/
public function year($values)
{
return 'year(4)';

View File

@ -3337,12 +3337,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
*/
public function summaryFields()
{
$fields = $this->config()->get('summary_fields');
$rawFields = $this->config()->get('summary_fields');
// if fields were passed in numeric array,
// convert to an associative array
if ($fields && array_key_exists(0, $fields)) {
$fields = array_combine(array_values($fields), array_values($fields));
// Merge associative / numeric keys
$fields = [];
foreach ($rawFields as $key => $value) {
if (is_int($key)) {
$key = $value;
}
$fields[$key] = $value;
}
if (!$fields) {
@ -3418,8 +3421,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
}
/*
* @ignore
*/
* @ignore
*/
private static $subclass_access = true;
/**

View File

@ -241,11 +241,11 @@ JS;
}
/*
* @example Given the CMS settings has the following data
* | Title | My site title |
* | Theme | My site theme |
* @Given /^the CMS settings have the following data$/
*/
* @example Given the CMS settings has the following data
* | Title | My site title |
* | Theme | My site theme |
* @Given /^the CMS settings have the following data$/
*/
public function theCmsSettingsHasData(TableNode $fieldsTable)
{
$fields = $fieldsTable->getRowsHash();

View File

@ -390,8 +390,8 @@ class ControllerTest extends FunctionalTest
'Numeric actions do not slip through.'
);
//$this->assertFalse(
// $controller->hasAction('lowercase_permission'),
// 'Lowercase permission does not slip through.'
// $controller->hasAction('lowercase_permission'),
// 'Lowercase permission does not slip through.'
//);
$this->assertFalse(
$controller->hasAction('undefined'),

View File

@ -230,7 +230,7 @@ class CsvBulkLoaderTest extends SapphireTest
// null values are valid imported
// $this->assertEquals($player->Biography, 'He\'s a good guy',
// 'Test retaining of previous information on duplicate when overwriting with blank field');
// 'Test retaining of previous information on duplicate when overwriting with blank field');
}
public function testLoadWithCustomImportMethods()

View File

@ -226,11 +226,11 @@ class ListboxFieldTest extends SapphireTest
* @todo re-enable these tests when field validation is removed from {@link ListboxField::setValue()} and moved
* to the {@link ListboxField::validate()} function
*/
// $field->setValue(4);
// $this->assertFalse(
// $field->validate($validator),
// 'Field does not validate values outside of source map'
// );
// $field->setValue(4);
// $this->assertFalse(
// $field->validate($validator),
// 'Field does not validate values outside of source map'
// );
$field->setValue(
false,
new ArrayData(

View File

@ -2,25 +2,23 @@
namespace SilverStripe\ORM\Tests;
use InvalidArgumentException;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\i18n\i18n;
use SilverStripe\ORM\Connect\MySQLDatabase;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DataObjectSchema;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\FieldType\DBBoolean;
use SilverStripe\ORM\FieldType\DBDatetime;
use SilverStripe\ORM\FieldType\DBField;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\Connect\MySQLDatabase;
use SilverStripe\ORM\FieldType\DBPolymorphicForeignKey;
use SilverStripe\ORM\FieldType\DBVarchar;
use SilverStripe\ORM\ManyManyList;
use SilverStripe\ORM\Tests\DataObjectTest\Player;
use SilverStripe\ORM\ValidationException;
use SilverStripe\View\ViewableData;
use stdClass;
use ReflectionException;
use InvalidArgumentException;
class DataObjectTest extends SapphireTest
{
@ -87,11 +85,11 @@ class DataObjectTest extends SapphireTest
// Test with table required
$this->assertEquals(
DataObjectTest\TeamComment::class.'.Varchar',
DataObjectTest\TeamComment::class . '.Varchar',
$schema->fieldSpec(DataObjectTest\TeamComment::class, 'Name', DataObjectSchema::INCLUDE_CLASS)
);
$this->assertEquals(
DataObjectTest\TeamComment::class.'.Text',
DataObjectTest\TeamComment::class . '.Text',
$schema->fieldSpec(DataObjectTest\TeamComment::class, 'Comment', DataObjectSchema::INCLUDE_CLASS)
);
$dbFields = $schema->fieldSpecs(DataObjectTest\ExtendedTeamComment::class);
@ -127,8 +125,8 @@ class DataObjectTest extends SapphireTest
// Values can be an array...
$player = new DataObjectTest\Player(
array(
'FirstName' => 'James',
'Surname' => 'Smith'
'FirstName' => 'James',
'Surname' => 'Smith'
)
);
@ -278,7 +276,7 @@ class DataObjectTest extends SapphireTest
$comment1 = DataObject::get_one(
DataObjectTest\TeamComment::class,
array(
'"DataObjectTest_TeamComment"."Name"' => 'Joe'
'"DataObjectTest_TeamComment"."Name"' => 'Joe'
),
false
);
@ -287,7 +285,7 @@ class DataObjectTest extends SapphireTest
$comment2 = DataObject::get_one(
DataObjectTest\TeamComment::class,
array(
'"DataObjectTest_TeamComment"."Name"' => 'Joe'
'"DataObjectTest_TeamComment"."Name"' => 'Joe'
),
false
);
@ -297,7 +295,7 @@ class DataObjectTest extends SapphireTest
$comment1 = DataObject::get_one(
DataObjectTest\TeamComment::class,
array(
'"DataObjectTest_TeamComment"."Name"' => 'Bob'
'"DataObjectTest_TeamComment"."Name"' => 'Bob'
),
true
);
@ -306,7 +304,7 @@ class DataObjectTest extends SapphireTest
$comment2 = DataObject::get_one(
DataObjectTest\TeamComment::class,
array(
'"DataObjectTest_TeamComment"."Name"' => 'Bob'
'"DataObjectTest_TeamComment"."Name"' => 'Bob'
),
true
);
@ -338,7 +336,7 @@ class DataObjectTest extends SapphireTest
$subteam1 = DataObject::get_one(
strtolower(DataObjectTest\SubTeam::class),
array(
'"DataObjectTest_Team"."Title"' => 'Subteam 1'
'"DataObjectTest_Team"."Title"' => 'Subteam 1'
),
true
);
@ -705,8 +703,8 @@ class DataObjectTest extends SapphireTest
$obj->getChangedFields(true, DataObject::CHANGE_VALUE),
array(
'FirstName' => array(
'before'=>'Captain',
'after'=>'Captain-changed',
'before' => 'Captain',
'after' => 'Captain-changed',
'level' => DataObject::CHANGE_VALUE
)
),
@ -1190,15 +1188,13 @@ class DataObjectTest extends SapphireTest
$summaryFields = $team->summaryFields();
$this->assertEquals(
'Custom Title',
$summaryFields['Title'],
'Custom title is preserved'
);
$this->assertEquals(
'Captain\'s shirt number',
$summaryFields['Captain.ShirtNumber'],
'Custom title on relation is preserved'
[
'Title' => 'Custom Title',
'Title.UpperCase' => 'Title',
'Captain.ShirtNumber' => 'Captain\'s shirt number',
'Captain.FavouriteTeam.Title' => 'Captain\'s favourite team',
],
$summaryFields
);
}
@ -1211,10 +1207,10 @@ class DataObjectTest extends SapphireTest
$team1->update(
array(
'DatabaseField' => 'Something',
'Captain.FirstName' => 'Jim',
'Captain.Email' => 'jim@example.com',
'Captain.FavouriteTeam.Title' => 'New and improved team 1',
'DatabaseField' => 'Something',
'Captain.FirstName' => 'Jim',
'Captain.Email' => 'jim@example.com',
'Captain.FavouriteTeam.Title' => 'New and improved team 1',
)
);
@ -1242,8 +1238,8 @@ class DataObjectTest extends SapphireTest
$team1->update(
array(
'Captain.FirstName' => 'Jim',
'Captain.FavouriteTeam.Title' => 'New and improved team 1',
'Captain.FirstName' => 'Jim',
'Captain.FavouriteTeam.Title' => 'New and improved team 1',
)
);
/* Test that the captain ID has been updated */
@ -1459,8 +1455,8 @@ class DataObjectTest extends SapphireTest
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Controller is not a valid subclass of DataObject');
/**
* @skipUpgrade
*/
* @skipUpgrade
*/
$dataObject->newClassInstance('Controller');
}
@ -1577,7 +1573,7 @@ class DataObjectTest extends SapphireTest
$teamExtraFields = $team->manyManyExtraFields();
$this->assertEquals(
array(
'Players' => array('Position' => 'Varchar(100)')
'Players' => array('Position' => 'Varchar(100)')
),
$teamExtraFields
);
@ -1587,8 +1583,8 @@ class DataObjectTest extends SapphireTest
$teamExtraFields = $subTeam->manyManyExtraFields();
$this->assertEquals(
array(
'Players' => array('Position' => 'Varchar(100)'),
'FormerPlayers' => array('Position' => 'Varchar(100)')
'Players' => array('Position' => 'Varchar(100)'),
'FormerPlayers' => array('Position' => 'Varchar(100)')
),
$teamExtraFields
);
@ -1598,7 +1594,7 @@ class DataObjectTest extends SapphireTest
$this->assertEquals(
$teamExtraFields,
array(
'Position' => 'Varchar(100)'
'Position' => 'Varchar(100)'
)
);
@ -1607,7 +1603,7 @@ class DataObjectTest extends SapphireTest
$this->assertEquals(
$playerExtraFields,
array(
'Position' => 'Varchar(100)'
'Position' => 'Varchar(100)'
)
);
@ -1797,9 +1793,9 @@ class DataObjectTest extends SapphireTest
$company = new DataObjectTest\Company();
$this->assertEquals(
array (
'CurrentStaff' => DataObjectTest\Staff::class,
'PreviousStaff' => DataObjectTest\Staff::class
array(
'CurrentStaff' => DataObjectTest\Staff::class,
'PreviousStaff' => DataObjectTest\Staff::class
),
$company->hasMany(),
'has_many strips field name data by default.'
@ -1812,16 +1808,16 @@ class DataObjectTest extends SapphireTest
);
$this->assertEquals(
array (
'CurrentStaff' => DataObjectTest\Staff::class.'.CurrentCompany',
'PreviousStaff' => DataObjectTest\Staff::class.'.PreviousCompany'
array(
'CurrentStaff' => DataObjectTest\Staff::class . '.CurrentCompany',
'PreviousStaff' => DataObjectTest\Staff::class . '.PreviousCompany'
),
$company->hasMany(false),
'has_many returns field name data when $classOnly is false.'
);
$this->assertEquals(
DataObjectTest\Staff::class.'.CurrentCompany',
DataObjectTest\Staff::class . '.CurrentCompany',
DataObject::getSchema()->hasManyComponent(DataObjectTest\Company::class, 'CurrentStaff', false),
'has_many returns field name data on single records when $classOnly is false.'
);
@ -1897,7 +1893,7 @@ class DataObjectTest extends SapphireTest
public function testBelongsTo()
{
$company = new DataObjectTest\Company();
$ceo = new DataObjectTest\CEO();
$ceo = new DataObjectTest\CEO();
$company->Name = 'New Company';
$company->write();
@ -1948,7 +1944,7 @@ class DataObjectTest extends SapphireTest
public function testBelongsToPolymorphic()
{
$company = new DataObjectTest\Company();
$ceo = new DataObjectTest\CEO();
$ceo = new DataObjectTest\CEO();
$company->write();
$ceo->write();

View File

@ -59,7 +59,7 @@ class Team extends DataObject implements TestOnly
);
private static $summary_fields = array(
'Title' => 'Custom Title',
'Title', // Overridden by Team_Extension
'Title.UpperCase' => 'Title',
'Captain.ShirtNumber' => 'Captain\'s shirt number',
'Captain.FavouriteTeam.Title' => 'Captain\'s favourite team'

View File

@ -7,6 +7,10 @@ use SilverStripe\ORM\DataExtension;
class Team_Extension extends DataExtension implements TestOnly
{
private static $summary_fields = [
'Title' => 'Custom Title', // override non-associative 'Title'
];
private static $db = array(
'ExtendedDatabaseField' => 'Varchar'
);

View File

@ -1775,8 +1775,8 @@ EOC;
$this->assertContains($code, $result);
// TODO Fix inline links in PHP mode
// $this->assertContains(
// '<a class="inline" href="<?php echo str_replace(',
// $result
// '<a class="inline" href="<?php echo str_replace(',
// $result
// );
$this->assertContains(
'<svg><use xlink:href="#sprite"></use></svg>',