From 16472b882761447ce33ca50e62d22bf0bd408ae5 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Wed, 28 Oct 2015 14:33:44 +1300
Subject: [PATCH] BUG Prevent subsequent write being triggered in
 onAfterWrite() Fix minor JS error

---
 .../editableformfields/EditableFormField.php  | 28 +++++++++++++------
 javascript/FieldEditor.js                     |  8 ++++--
 tests/EditableFormFieldTest.php               | 21 ++++++++++++++
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/code/model/editableformfields/EditableFormField.php b/code/model/editableformfields/EditableFormField.php
index 1f37ed7..8d464b6 100755
--- a/code/model/editableformfields/EditableFormField.php
+++ b/code/model/editableformfields/EditableFormField.php
@@ -276,7 +276,12 @@ class EditableFormField extends DataObject {
 	public function onBeforeWrite() {
 		parent::onBeforeWrite();
 
-		if($this->Name === 'Field') {
+		// Set a field name.
+		if(!$this->Name) {
+			// New random name
+			$this->Name = $this->generateName();
+
+		} elseif($this->Name === 'Field') {
 			throw new ValidationException('Field name cannot be "Field"');
 		}
 
@@ -289,16 +294,21 @@ class EditableFormField extends DataObject {
 	}
 
 	/**
-	 * @return void
+	 * Generate a new non-conflicting Name value
+	 *
+	 * @return string
 	 */
-	public function onAfterWrite() {
-		parent::onAfterWrite();
+	protected function generateName() {
+		do {
+			// Generate a new random name after this class
+			$class = get_class($this);
+			$entropy = substr(sha1(uniqid()), 0, 5);
+			$name = "{$class}_{$entropy}";
 
-		// Set a field name.
-		if(!$this->Name) {
-			$this->Name = get_class($this) . '_' . $this->ID;
-			$this->write();
-		}
+			// Check if it conflicts
+			$exists = EditableFormField::get()->filter('Name', $name)->count() > 0;
+		} while($exists);
+		return $name;
 	}
 
 	/**
diff --git a/javascript/FieldEditor.js b/javascript/FieldEditor.js
index b2e6cf1..6d998d4 100644
--- a/javascript/FieldEditor.js
+++ b/javascript/FieldEditor.js
@@ -82,9 +82,9 @@
 				this.on('addnewinline', function () {
 					self.one('reload', function () {
 						//If fieldgroup, focus on the start marker
-						var $newField = self.find('.ss-gridfield-item').last()
+						var $newField = self.find('.ss-gridfield-item').last(), $groupEnd;
 						if ($newField.attr('data-class') === 'EditableFieldGroupEnd') {
-							var $groupEnd = $newField;
+							$groupEnd = $newField;
 							$groupEnd.prev().find('.col-Title input').focus();
 							$newField = $groupEnd.add($groupEnd.prev());
 							$groupEnd.css('visibility', 'hidden');
@@ -101,7 +101,9 @@
 						setTimeout(function () {
 							$newField.removeClass('newField').addClass('flashBackground');
 							$(".cms-content-fields").scrollTop($(".cms-content-fields")[0].scrollHeight);
-							$groupEnd.css('visibility', 'visible');
+							if($groupEnd) {
+								$groupEnd.css('visibility', 'visible');
+							}
 						}, 500);
 					});
 				});
diff --git a/tests/EditableFormFieldTest.php b/tests/EditableFormFieldTest.php
index ff716f8..1258b20 100644
--- a/tests/EditableFormFieldTest.php
+++ b/tests/EditableFormFieldTest.php
@@ -110,4 +110,25 @@ class EditableFormFieldTest extends FunctionalTest {
 		$this->assertNotContains('jpg', $formField->getValidator()->getAllowedExtensions());
 	}
 
+	/**
+	 * Verify that unique names are automatically generated for each formfield
+	 */
+	public function testUniqueName() {
+		$textfield1 = new EditableTextField();
+		$this->assertEmpty($textfield1->Name);
+
+		// Write values
+		$textfield1->write();
+		$textfield2 = new EditableTextField();
+		$textfield2->write();
+		$checkboxField = new EditableCheckbox();
+		$checkboxField->write();
+
+		// Test values are in the expected format
+		$this->assertRegExp('/^EditableTextField_.+/', $textfield1->Name);
+		$this->assertRegExp('/^EditableTextField_.+/', $textfield2->Name);
+		$this->assertRegExp('/^EditableCheckbox_.+/', $checkboxField->Name);
+		$this->assertNotEquals($textfield1->Name, $textfield2->Name);
+	}
+
 }