Merge pull request #4053 from tractorcow/pulls/3.1/fix-stringfield-exists

BUG Fix default casted (boolean)false evaluating to true in templates

model/fieldtypes/StringField.php

@@ -83,7 +83,9 @@ abstract class StringField extends DBField {
 	 * @see core/model/fieldtypes/DBField#exists()
 	 */
 	public function exists() {
-		return ($this->value || $this->value == '0') || ( !$this->nullifyEmpty && $this->value === '');
+		return $this->getValue() // All truthy values exist
+			|| (is_string($this->getValue()) && strlen($this->getValue())) // non-empty strings exist ('0' but not (int)0)
+			|| (!$this->getNullifyEmpty() && $this->getValue() === ''); // Remove this stupid exemption in 4.0
 	}
 
 	/**

tests/model/StringFieldTest.php

@@ -36,6 +36,22 @@ class StringFieldTest extends SapphireTest {
 		);
 	}
 
+	public function testExists() {
+		// True exists
+		$this->assertTrue(DBField::create_field('StringFieldTest_MyStringField', true)->exists());
+		$this->assertTrue(DBField::create_field('StringFieldTest_MyStringField', '0')->exists());
+		$this->assertTrue(DBField::create_field('StringFieldTest_MyStringField', '1')->exists());
+		$this->assertTrue(DBField::create_field('StringFieldTest_MyStringField', 1)->exists());
+		$this->assertTrue(DBField::create_field('StringFieldTest_MyStringField', 1.1)->exists());
+
+		// false exists
+		$this->assertFalse(DBField::create_field('StringFieldTest_MyStringField', false)->exists());
+		$this->assertFalse(DBField::create_field('StringFieldTest_MyStringField', '')->exists());
+		$this->assertFalse(DBField::create_field('StringFieldTest_MyStringField', null)->exists());
+		$this->assertFalse(DBField::create_field('StringFieldTest_MyStringField', 0)->exists());
+		$this->assertFalse(DBField::create_field('StringFieldTest_MyStringField', 0.0)->exists());
+	}
+
 }
 
 class StringFieldTest_MyStringField extends StringField implements TestOnly {

tests/templates/SSViewerTestIncludeScopeInheritanceInclude.ss

@@ -1 +1 @@
-$Title <% if ArgA %>- $ArgA <% end_if %>- <%if First %>First-<% end_if %><% if Last %>Last-<% end_if %><%if MultipleOf(2) %>EVEN<% else %>ODD<% end_if %> top:$Top.Title
+<% if $Title %>$Title<% else %>Untitled<% end_if %> <% if $ArgA %>_ $ArgA <% end_if %>- <% if $First %>First-<% end_if %><% if $Last %>Last-<% end_if %><%if $MultipleOf(2) %>EVEN<% else %>ODD<% end_if %> top:$Top.Title

tests/view/SSViewerTest.php

@@ -46,18 +46,46 @@ class SSViewerTest extends SapphireTest {
 
 		// reset results for the tests that include arguments (the title is passed as an arg)
 		$expected = array(
-			'Item 1 - Item 1 - First-ODD top:Item 1',
+			'Item 1 _ Item 1 - First-ODD top:Item 1',
-			'Item 2 - Item 2 - EVEN top:Item 2',
+			'Item 2 _ Item 2 - EVEN top:Item 2',
-			'Item 3 - Item 3 - ODD top:Item 3',
+			'Item 3 _ Item 3 - ODD top:Item 3',
-			'Item 4 - Item 4 - EVEN top:Item 4',
+			'Item 4 _ Item 4 - EVEN top:Item 4',
-			'Item 5 - Item 5 - ODD top:Item 5',
+			'Item 5 _ Item 5 - ODD top:Item 5',
-			'Item 6 - Item 6 - Last-EVEN top:Item 6',
+			'Item 6 _ Item 6 - Last-EVEN top:Item 6',
 		);
 
 		$result = $data->renderWith('SSViewerTestIncludeScopeInheritanceWithArgs');
 		$this->assertExpectedStrings($result, $expected);
 	}
 	
+	public function testIncludeTruthyness() {
+		$data = new ArrayData(array(
+			'Title' => 'TruthyTest',
+			'Items' => new ArrayList(array(
+				new ArrayData(array('Title' => 'Item 1')),
+				new ArrayData(array('Title' => '')),
+				new ArrayData(array('Title' => true)),
+				new ArrayData(array('Title' => false)),
+				new ArrayData(array('Title' => null)),
+				new ArrayData(array('Title' => 0)),
+				new ArrayData(array('Title' => 7))
+			))
+		));
+		$result = $data->renderWith('SSViewerTestIncludeScopeInheritanceWithArgs');
+		
+		// We should not end up with empty values appearing as empty
+		$expected = array(
+			'Item 1 _ Item 1 - First-ODD top:Item 1',
+			'Untitled - EVEN top:',
+			'1 _ 1 - ODD top:1',
+			'Untitled - EVEN top:',
+			'Untitled - ODD top:',
+			'Untitled - EVEN top:0',
+			'7 _ 7 - Last-ODD top:7'
+		);
+		$this->assertExpectedStrings($result, $expected);
+	}
+
 	private function getScopeInheritanceTestData() {
 		return new ArrayData(array(
 			'Title' => 'TopTitleValue',

view/SSViewer.php

@@ -429,6 +429,15 @@ class SSViewer_DataPresenter extends SSViewer_Scope {
 		}
 	}
 
+	/**
+	 * Get the injected value
+	 * 
+	 * @param string $property Name of property
+	 * @param array $params
+	 * @param bool $cast If true, an object is always returned even if not an object.
+	 * @return array Result array with the keys 'value' for raw value, or 'obj' if contained in an object
+	 * @throws InvalidArgumentException
+	 */
 	public function getInjectedValue($property, $params, $cast = true) {
 		$on = $this->itemIterator ? $this->itemIterator->current() : $this->item;
 
@@ -512,32 +521,25 @@ class SSViewer_DataPresenter extends SSViewer_Scope {
 		if (isset($arguments[1]) && $arguments[1] != null) $params = $arguments[1]; 
 		else $params = array();
 
-		$hasInjected = $res = null;
+		$val = $this->getInjectedValue($property, $params);
-
+		if ($val) {
-		if ($name == 'hasValue') {
+			$obj = $val['obj'];
-			if ($val = $this->getInjectedValue($property, $params, false)) {
+			if ($name === 'hasValue') {
-				$hasInjected = true; $res = (bool)$val['value'];
+				$res = $obj instanceof Object
-			}
+					? $obj->exists()
-		}
+					: (bool)$obj;
-		else { // XML_val
+			} else {
-			if ($val = $this->getInjectedValue($property, $params)) {
+				// XML_val
-				$hasInjected = true;
-				$obj = $val['obj'];
 				$res = $obj->forTemplate();
 			}
-		}
-
-		if ($hasInjected) {
 			$this->resetLocalScope();
 			return $res;
-		}
+		} else {
-		else {
 			return parent::__call($name, $arguments);
 		}
 	}
 }
 
-
 /**
  * Parses a template file with an *.ss file extension.
  *