diff --git a/control/Session.php b/control/Session.php index 9725783b2..e40018a4f 100644 --- a/control/Session.php +++ b/control/Session.php @@ -359,6 +359,15 @@ class Session { $path = Config::inst()->get('Session', 'cookie_path'); if(!$path) $path = Director::baseURL(); $domain = Config::inst()->get('Session', 'cookie_domain'); + // Director::baseURL can return absolute domain names - this extracts the relevant parts + // for the session otherwise we can get broken session cookies + if (Director::is_absolute_url($path)) { + $urlParts = parse_url($path); + $path = $urlParts['path']; + if (!$domain) { + $domain = $urlParts['host']; + } + } $secure = Director::is_https() && Config::inst()->get('Session', 'cookie_secure'); $session_path = Config::inst()->get('Session', 'session_store_path'); $timeout = Config::inst()->get('Session', 'timeout'); diff --git a/core/Object.php b/core/Object.php index eb9ec9310..3607e4ac9 100755 --- a/core/Object.php +++ b/core/Object.php @@ -777,7 +777,7 @@ abstract class Object { } else { // Please do not change the exception code number below. $class = get_class($this); - throw new Exception("Object->__call(): the method '$method' does not exist on '$class'", 2175); + throw new Exception("Object->__call(): the method '$method' does not exist on '$class', or the method is not public.", 2175); } } diff --git a/forms/CheckboxSetField.php b/forms/CheckboxSetField.php index f1ef4bbaa..acd205809 100644 --- a/forms/CheckboxSetField.php +++ b/forms/CheckboxSetField.php @@ -333,21 +333,22 @@ class CheckboxSetField extends OptionsetField { return true; } $sourceArray = $this->getSourceAsArray(); + $validValues = array_keys($sourceArray); if (is_array($values)) { - if (!array_intersect_key($sourceArray, $values)) { + if (!array_intersect($validValues, $values)) { $validator->validationError( $this->name, _t( 'CheckboxSetField.SOURCE_VALIDATION', "Please select a value within the list provided. '{value}' is not a valid option", - array('value' => implode(' and ', array_diff($sourceArray, $values))) + array('value' => implode(' and ', $values)) ), "validation" ); return false; } } else { - if (!in_array($this->value, $sourceArray)) { + if (!in_array($this->value, $validValues)) { $validator->validationError( $this->name, _t( diff --git a/forms/DropdownField.php b/forms/DropdownField.php index 83fa88edf..1aa6c28f5 100644 --- a/forms/DropdownField.php +++ b/forms/DropdownField.php @@ -318,13 +318,22 @@ class DropdownField extends FormField { public function getSourceAsArray() { $source = $this->getSource(); + + // Simplify source if presented as dataobject list + if ($source instanceof SS_List) { + $source = $source->map(); + } + if ($source instanceof SS_Map) { + $source = $source->toArray(); + } + if (is_array($source)) { return $source; - } else { - $sourceArray = array(); - foreach ($source as $key => $value) { - $sourceArray[$key] = $value; - } + } + + $sourceArray = array(); + foreach ($source as $key => $value) { + $sourceArray[$key] = $value; } return $sourceArray; } diff --git a/forms/FieldList.php b/forms/FieldList.php index 83be3f75c..68a1f9922 100644 --- a/forms/FieldList.php +++ b/forms/FieldList.php @@ -550,7 +550,12 @@ class FieldList extends ArrayList { public function makeFieldReadonly($field) { $fieldName = ($field instanceof FormField) ? $field->getName() : $field; $srcField = $this->dataFieldByName($fieldName); - $this->replaceField($fieldName, $srcField->performReadonlyTransformation()); + if($srcField) { + $this->replaceField($fieldName, $srcField->performReadonlyTransformation()); + } + else { + user_error("Trying to make field '$fieldName' readonly, but it does not exist in the list",E_USER_WARNING); + } } /** diff --git a/model/ArrayList.php b/model/ArrayList.php index 903b04421..784fb55ce 100644 --- a/model/ArrayList.php +++ b/model/ArrayList.php @@ -448,7 +448,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta // First argument is the direction to be sorted, $multisortArgs[] = &$sortDirection[$column]; if ($firstRun) { - $multisortArgs[] = defined('SORT_NATURAL') ? SORT_NATURAL : SORT_STRING; + $multisortArgs[] = SORT_REGULAR; } $firstRun = false; } diff --git a/tests/forms/CheckboxSetFieldTest.php b/tests/forms/CheckboxSetFieldTest.php index 94cb4e171..e90a31d4c 100644 --- a/tests/forms/CheckboxSetFieldTest.php +++ b/tests/forms/CheckboxSetFieldTest.php @@ -177,28 +177,65 @@ class CheckboxSetFieldTest extends SapphireTest { $tag1 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag1'); $tag2 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag2'); $tag3 = $this->objFromFixture('CheckboxSetFieldTest_Tag', 'tag3'); - $field = CheckboxSetField::create('Test', 'Testing', $checkboxTestArticle->Tags() ->map()); + $field = CheckboxSetField::create('Test', 'Testing', $checkboxTestArticle->Tags()); $validator = new RequiredFields(); - $field->setValue(array( - $tag1->ID => $tag1->ID, - $tag2->ID => $tag2->ID - )); + $field->setValue(array( $tag1->ID, $tag2->ID )); + $isValid = $field->validate($validator); $this->assertTrue( - $field->validate($validator), + $isValid, 'Validates values in source map' ); - //invalid value should fail + + // Invalid value should fail + $validator = new RequiredFields(); $fakeID = CheckboxSetFieldTest_Tag::get()->max('ID') + 1; - $field->setValue(array($fakeID => $fakeID)); + $field->setValue(array($fakeID)); $this->assertFalse( $field->validate($validator), 'Field does not valid values outside of source map' ); - //non valid value included with valid options should succeed + $errors = $validator->getErrors(); + $error = reset($errors); + $this->assertEquals( + "Please select a value within the list provided. '$fakeID' is not a valid option", + $error['message'] + ); + + // Multiple invalid values should fail + $validator = new RequiredFields(); + $fakeID = CheckboxSetFieldTest_Tag::get()->max('ID') + 1; + $field->setValue(array($fakeID, $tag3->ID)); + $this->assertFalse( + $field->validate($validator), + 'Field does not valid values outside of source map' + ); + $errors = $validator->getErrors(); + $error = reset($errors); + $this->assertEquals( + "Please select a value within the list provided. '{$fakeID} and {$tag3->ID}' is not a valid option", + $error['message'] + ); + + // Invalid value with non-array value + $validator = new RequiredFields(); + $field->setValue($fakeID); + $this->assertFalse( + $field->validate($validator), + 'Field does not valid values outside of source map' + ); + $errors = $validator->getErrors(); + $error = reset($errors); + $this->assertEquals( + "Please select a value within the list provided. '{$fakeID}' is not a valid option", + $error['message'] + ); + + // non valid value included with valid options should succeed + $validator = new RequiredFields(); $field->setValue(array( - $tag1->ID => $tag1->ID, - $tag2->ID => $tag2->ID, - $tag3->ID => $tag3->ID + $tag1->ID, + $tag2->ID, + $tag3->ID )); $this->assertTrue( $field->validate($validator), diff --git a/tests/forms/DropdownFieldTest.php b/tests/forms/DropdownFieldTest.php index a43f8ee09..ce954566e 100644 --- a/tests/forms/DropdownFieldTest.php +++ b/tests/forms/DropdownFieldTest.php @@ -6,12 +6,37 @@ class DropdownFieldTest extends SapphireTest { public function testGetSource() { - $source = array(1=>'one'); + $source = array(1=>'one', 2 => 'two'); $field = new DropdownField('Field', null, $source); $this->assertEquals( - $field->getSource(), + $source, + $field->getSource() + ); + $this->assertEquals( + $source, + $field->getSourceAsArray() + ); + + $items = new ArrayList(array( + array( 'ID' => 1, 'Title' => 'ichi', 'OtherField' => 'notone' ), + array( 'ID' => 2, 'Title' => 'ni', 'OtherField' => 'nottwo' ), + )); + $field->setSource($items); + $this->assertEquals( + $field->getSourceAsArray(), array( - 1 => 'one' + 1 => 'ichi', + 2 => 'ni', + ) + ); + + $map = new SS_Map($items, 'ID', 'OtherField'); + $field->setSource($map); + $this->assertEquals( + $field->getSourceAsArray(), + array( + 1 => 'notone', + 2 => 'nottwo', ) ); } diff --git a/tests/model/ArrayListTest.php b/tests/model/ArrayListTest.php index 7e2e4630d..1f98fa086 100644 --- a/tests/model/ArrayListTest.php +++ b/tests/model/ArrayListTest.php @@ -314,67 +314,46 @@ class ArrayListTest extends SapphireTest { ), $list->toArray()); } - public function testNaturalSort() { - //natural sort is only available in 5.4+ - if (version_compare(phpversion(), '5.4.0', '<')) { - $this->markTestSkipped(); - } - $list = new ArrayList(array( - array('Name' => 'Steve'), + public function testMixedCaseSort() { + // Note: Natural sorting is not expected, so if 'bonny10' were included + // below we would expect it to appear between bonny1 and bonny2. That's + // undesirable though so we're not enforcing it in tests. + $original = array( + array('Name' => 'Steve'), (object) array('Name' => 'Bob'), array('Name' => 'John'), array('Name' => 'bonny'), array('Name' => 'bonny1'), - array('Name' => 'bonny10'), + //array('Name' => 'bonny10'), array('Name' => 'bonny2'), - )); + ); + + $list = new ArrayList($original); + + $expected = array( + (object) array('Name' => 'Bob'), + array('Name' => 'bonny'), + array('Name' => 'bonny1'), + //array('Name' => 'bonny10'), + array('Name' => 'bonny2'), + array('Name' => 'John'), + array('Name' => 'Steve'), + ); // Unquoted name $list1 = $list->sort('Name'); - $this->assertEquals(array( - (object) array('Name' => 'Bob'), - array('Name' => 'bonny'), - array('Name' => 'bonny1'), - array('Name' => 'bonny2'), - array('Name' => 'bonny10'), - array('Name' => 'John'), - array('Name' => 'Steve'), - ), $list1->toArray()); + $this->assertEquals($expected, $list1->toArray()); // Quoted name name $list2 = $list->sort('"Name"'); - $this->assertEquals(array( - (object) array('Name' => 'Bob'), - array('Name' => 'bonny'), - array('Name' => 'bonny1'), - array('Name' => 'bonny2'), - array('Name' => 'bonny10'), - array('Name' => 'John'), - array('Name' => 'Steve'), - ), $list2->toArray()); + $this->assertEquals($expected, $list2->toArray()); // Array (non-associative) $list3 = $list->sort(array('"Name"')); - $this->assertEquals(array( - (object) array('Name' => 'Bob'), - array('Name' => 'bonny'), - array('Name' => 'bonny1'), - array('Name' => 'bonny2'), - array('Name' => 'bonny10'), - array('Name' => 'John'), - array('Name' => 'Steve'), - ), $list3->toArray()); + $this->assertEquals($expected, $list3->toArray()); // Check original list isn't altered - $this->assertEquals(array( - array('Name' => 'Steve'), - (object) array('Name' => 'Bob'), - array('Name' => 'John'), - array('Name' => 'bonny'), - array('Name' => 'bonny1'), - array('Name' => 'bonny10'), - array('Name' => 'bonny2'), - ), $list->toArray()); + $this->assertEquals($original, $list->toArray()); } @@ -472,6 +451,42 @@ class ArrayListTest extends SapphireTest { )); } + public function testSortNumeric() { + $list = new ArrayList(array( + array('Sort' => 0), + array('Sort' => -1), + array('Sort' => 1), + array('Sort' => -2), + array('Sort' => 2), + array('Sort' => -10), + array('Sort' => 10) + )); + + // Sort descending + $list1 = $list->sort('Sort', 'DESC'); + $this->assertEquals(array( + array('Sort' => 10), + array('Sort' => 2), + array('Sort' => 1), + array('Sort' => 0), + array('Sort' => -1), + array('Sort' => -2), + array('Sort' => -10) + ), $list1->toArray()); + + // Sort ascending + $list1 = $list->sort('Sort', 'ASC'); + $this->assertEquals(array( + array('Sort' => -10), + array('Sort' => -2), + array('Sort' => -1), + array('Sort' => 0), + array('Sort' => 1), + array('Sort' => 2), + array('Sort' => 10) + ), $list1->toArray()); + } + public function testReverse() { $list = new ArrayList(array( array('Name' => 'John'), diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index 1a4353127..87a695f17 100755 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -23,6 +23,7 @@ class DataListTest extends SapphireTest { 'DataObjectTest_EquipmentCompany', 'DataObjectTest_SubEquipmentCompany', 'DataObjectTest\NamespacedClass', + 'DataObjectTest_Sortable', 'DataObjectTest_Company', 'DataObjectTest_Fan', 'ManyManyListTest_Product', @@ -533,6 +534,34 @@ class DataListTest extends SapphireTest { $this->assertEquals('Phil', $list->last()->Name, 'Last comment should be from Phil'); } + public function testSortNumeric() { + $list = DataObjectTest_Sortable::get(); + $list1 = $list->sort('Sort', 'ASC'); + $this->assertEquals(array( + -10, + -2, + -1, + 0, + 1, + 2, + 10 + ), $list1->column('Sort')); + } + + public function testSortMixedCase() { + $list = DataObjectTest_Sortable::get(); + $list1 = $list->sort('Name', 'ASC'); + $this->assertEquals(array( + 'Bob', + 'bonny', + 'jane', + 'John', + 'sam', + 'Steve', + 'steven' + ), $list1->column('Name')); + } + /** * Test DataList->canFilterBy() */ diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index add8c7ed2..21e1882cf 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -1709,6 +1709,13 @@ class DataObjectTest extends SapphireTest { } +class DataObjectTest_Sortable extends DataObject implements TestOnly { + private static $db = array( + 'Sort' => 'Int', + 'Name' => 'Varchar', + ); +} + class DataObjectTest_Player extends Member implements TestOnly { private static $db = array( 'IsRetired' => 'Boolean', diff --git a/tests/model/DataObjectTest.yml b/tests/model/DataObjectTest.yml index 19065154e..16c145971 100644 --- a/tests/model/DataObjectTest.yml +++ b/tests/model/DataObjectTest.yml @@ -1,3 +1,25 @@ +DataObjectTest_Sortable: + numeric1: + Sort: 0 + Name: steven + numeric2: + Sort: -1 + Name: bonny + numeric3: + Sort: 1 + Name: sam + numeric4: + Sort: -2 + Name: Bob + numeric5: + Sort: 2 + Name: jane + numeric6: + Sort: -10 + Name: Steve + numeric7: + Sort: 10 + Name: John DataObjectTest_EquipmentCompany: equipmentcompany1: Name: Company corp