ENHANCEMENT Ability to add title constructor arguments to Tab and TabSet classes to resolve i18n issues with untranslated or unreferencable Tabs (see #2359)

ENHANCEMENT Support for $field_labels in relation tabs created by DataObject->addScaffoldRelationFields()
ENHANCEMENT Type checking and user errors for Tab and TabSet constructor arguments
ENHANCEMENT Supporting titles in FieldSet->findOrMakeTab()

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@64073 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/model/DataObject.php

@@ -1214,6 +1214,7 @@ class DataObject extends ViewableData implements DataObjectInterface {
 		if($this->has_many()) {
 			// Add each relation as a separate tab
 			foreach($this->has_many() as $relationship => $component) {
+				$relationTab = $fieldSet->findOrMakeTab("Root.$relationship", $this->fieldLabel($relationship));
 				$relationshipFields = singleton($component)->summaryFields();
 				$foreignKey = $this->getComponentJoinField($relationship);
 				$ctf = new ComplexTableField(
@@ -1230,6 +1231,7 @@ class DataObject extends ViewableData implements DataObjectInterface {
 		}
 		if ($this->many_many()) {
 			foreach($this->many_many() as $relationship => $component) {
+				$relationTab = $fieldSet->findOrMakeTab("Root.$relationship", $this->fieldLabel($relationship));
 				$relationshipFields = singleton($component)->summaryFields();
 				$filterWhere = $this->getManyManyFilter($relationship, $component);
 				$filterJoin = $this->getManyManyJoin($relationship, $component);

forms/FieldSet.php

@@ -214,24 +214,37 @@ class FieldSet extends DataObjectSet {
 	/**
 	 * Returns the specified tab object, creating it if necessary.
 	 * 
-	 * @param tabName The tab to return, in the form "Tab.Subtab.Subsubtab"
+	 * @todo Support recursive creation of TabSets
+	 * 
+	 * @param string $tabName The tab to return, in the form "Tab.Subtab.Subsubtab".
+	 *   Caution: Does not recursively create TabSet instances, you need to make sure everything
+	 *   up until the last tab in the chain exists.
+	 * @param string $title Natural language title of the tab. If {@link $tabName} is passed in dot notation,
+	 *   the title parameter will only apply to the innermost referenced tab.
+	 *   The title is only changed if the tab doesn't exist already.
+	 * @return Tab The found or newly created Tab instance
 	 */
-	protected function findOrMakeTab($tabName) {
+	public function findOrMakeTab($tabName, $title = null) {
 		$parts = explode('.',$tabName);
 		
 		// We could have made this recursive, but I've chosen to keep all the logic code within FieldSet rather than add it to TabSet and Tab too.
 		$currentPointer = $this;
-		foreach($parts as $part) {
+		foreach($parts as $k => $part) {
 			$parentPointer = $currentPointer;
 			$currentPointer = $currentPointer->fieldByName($part);
 			// Create any missing tabs
 			if(!$currentPointer) {
 				if(is_a($parentPointer, 'TabSet')) {
-					$currentPointer = new Tab($part);
+					// use $title on the innermost tab only
+					if($title && $k == count($parts)-1) {
+						$currentPointer = new Tab($part, $title);
+					} else {
+						$currentPointer = new Tab($part);
+					}
 					$parentPointer->push($currentPointer);
 				} else {
-					$withName = ($parentPointer->hasMethod('Name')) ? " named {$parentPointer->Name()}" : null;
-					user_error("FieldSet::addFieldToTab() Tried to add a tab to object '{$parentPointer->class}{$withName}' - '$part' didn't exist.", E_USER_ERROR);
+					$withName = ($parentPointer->hasMethod('Name')) ? " named '{$parentPointer->Name()}'" : null;
+					user_error("FieldSet::addFieldToTab() Tried to add a tab to object '{$parentPointer->class}'{$withName} - '$part' didn't exist.", E_USER_ERROR);
 				}
 			}
 		}

forms/Tab.php

@@ -7,13 +7,29 @@
 class Tab extends CompositeField {
 	protected $tabSet;
 	
+	/**
+	 * @uses FormField::name_to_label()
+	 * 
+	 * @param string $name Identifier of the tab, without characters like dots or spaces
+	 * @param string $title Natural language title of the tab. If its left out,
+	 *  the class uses {@link FormField::name_to_label()} to produce a title from the {@link $name} parameter.
+	 * @param FormField All following parameters are inserted as children to this tab
+	 */
 	public function __construct($name) {
 		$args = func_get_args();
+		
 		$name = array_shift($args);
+		if(!is_string($name)) user_error('TabSet::__construct(): $name parameter to a valid string', E_USER_ERROR);
+		$this->name = $name;
 		
 		$this->id = preg_replace('/[^0-9A-Za-z]+/', '', $name);
-		$this->title = preg_replace('/([a-z0-9])([A-Z])/', '\\1 \\2', $name);
-		$this->name = $name;
+		
+		// Legacy handling: only assume second parameter as title if its a string,
+		// otherwise it might be a formfield instance
+		if(isset($args[0]) && is_string($args[0])) {
+			$title = array_shift($args);
+		}
+		$this->title = (isset($title)) ? $title : FormField::name_to_label($name);
 		
 		parent::__construct($args);
 	}

forms/TabSet.php

@@ -7,15 +7,36 @@
  * @subpackage fields-structural
  */
 class TabSet extends CompositeField {
-	public function __construct($id) {
-		$tabs = func_get_args();
-		$this->id = array_shift($tabs);
-		$this->name = $this->id;
-		$this->title = $this->id;
 	
-		foreach($tabs as $tab) $tab->setTabSet($this);
+	/**
+	 * @param string $name Identifier
+	 * @param string $title (Optional) Natural language title of the tabset
+	 * @param Tab|TabSet $unknown All further parameters are inserted as children into the TabSet
+	 */
+	public function __construct($name) {
+		$args = func_get_args();
 		
-		parent::__construct($tabs);
+		$name = array_shift($args);
+		if(!is_string($name)) user_error('TabSet::__construct(): $name parameter to a valid string', E_USER_ERROR);
+		$this->name = $name;
+		
+		$this->id = $name;
+		
+		// Legacy handling: only assume second parameter as title if its a string,
+		// otherwise it might be a formfield instance
+		if(isset($args[0]) && is_string($args[0])) {
+			$title = array_shift($args);
+		}
+		$this->title = (isset($title)) ? $title : FormField::name_to_label($name);
+		
+		if($args) foreach($args as $tab) {
+			$isValidArg = (is_object($tab) && (!($tab instanceof Tab) || !($tab instanceof TabSet)));
+			if(!$isValidArg) user_error('TabSet::__construct(): Parameter not a valid Tab instance', E_USER_ERROR);
+			
+			$tab->setTabSet($this);
+		}
+		
+		parent::__construct($args);
 	}
 	
 	public function id() {

tests/forms/FieldSetTest.php

@@ -95,9 +95,11 @@ class FieldSetTest extends SapphireTest {
 		);
 		$this->assertFalse($untabbedFields->hasTabSet());
 		
-		$tabbedFields = new FieldSet(new TabSet(
-			new Tab('Tab1')
-		));
+		$tabbedFields = new FieldSet(
+			new TabSet('Root',
+				new Tab('Tab1')
+			)
+		);
 		$this->assertTrue($tabbedFields->hasTabSet());
 	}
 	
@@ -222,6 +224,44 @@ class FieldSetTest extends SapphireTest {
 		$this->assertEquals(1, $otherTab->Fields()->Count());
 	}
 	
+	function testTabTitles() {
+		$set = new FieldSet(
+			$rootTabSet = new TabSet('Root',
+				$tabSetWithoutTitle = new TabSet('TabSetWithoutTitle'),
+				$tabSetWithTitle = new TabSet('TabSetWithTitle', 'My TabSet Title',
+					new Tab('ExistingChildTab')
+				)
+			)
+		);
+		
+		$this->assertEquals(
+			$tabSetWithTitle->Title(),
+			'My TabSet Title',
+			'Automatic conversion of tab identifiers through findOrMakeTab() with FormField::name_to_label()'
+		);
+		
+		$tabWithoutTitle = $set->findOrMakeTab('Root.TabWithoutTitle');
+		$this->assertEquals(
+			$tabWithoutTitle->Title(),
+			'Tab Without Title',
+			'Automatic conversion of tab identifiers through findOrMakeTab() with FormField::name_to_label()'
+		);
+		
+		$tabWithTitle = $set->findOrMakeTab('Root.TabWithTitle', 'My Tab with Title');
+		$this->assertEquals(
+			$tabWithTitle->Title(),
+			'My Tab with Title',
+			'Setting of simple tab titles through findOrMakeTab()'
+		);
+		
+		$childTabWithTitle = $set->findOrMakeTab('Root.TabSetWithoutTitle.NewChildTab', 'My Child Tab Title');
+		$this->assertEquals(
+			$childTabWithTitle->Title(),
+			'My Child Tab Title',
+			'Setting of nested tab titles through findOrMakeTab() works on last child tab'
+		);
+	}
+	
 	/**
 	 * Test pushing a field to a set.
 	 *