Clean up

src/AppBundle/Model/BaseModel.php

@@ -10,8 +10,18 @@ class BaseModel extends \Illuminate\Database\Eloquent\Model {
 
 	public $timestamps = false;
 	public $useUuid = false;
-	protected $changedVersionedFieldValues = array();
+
+	// Diffable fields are those for which a diff is recorded on each change
+	// (such as the title or body of a note). The value of these fields is
+	// (currently) not recorded anywhere as-is - it needs to be rebuilt based on
+	// the diffs. The advantage of these fields is that they can be modified
+	// from different clients and the final value will be set correctly via
+	// three-way merge.
+	// These special fields need to be get and set via diffableField() and 
+	// setDiffableField()
+	protected $changedDiffableFields = array();
 	protected $diffableFields = array();
+
 	protected $isVersioned = false;
 	private $isNew = null;
 	private $revId = 0;
@@ -132,7 +142,7 @@ class BaseModel extends \Illuminate\Database\Eloquent\Model {
 			if ($k == 'rev_id') {
 				$this->revId = $v;
 			} else if (in_array($k, $this->diffableFields)) {
-				$this->changedVersionedFieldValues[$k] = $v;
+				$this->changedDiffableFields[$k] = $v;
 			} else {
 				$this->{$k} = $v;
 			}
@@ -163,7 +173,7 @@ class BaseModel extends \Illuminate\Database\Eloquent\Model {
 
 		$maxRevId = 0;
 		foreach ($this->diffableFields as $field) {
-			$r = $this->versionedFieldValue($field, true);
+			$r = $this->diffableField($field, true);
 			$output[$field] = $r['text'];
 			$maxRevId = max($maxRevId, $r['revId']);
 		}
@@ -173,12 +183,12 @@ class BaseModel extends \Illuminate\Database\Eloquent\Model {
 		return $output;
 	}
 
-	public function versionedFieldValue($fieldName, $returnRevId = false) {
+	public function diffableField($fieldName, $returnRevId = false) {
 		return Change::fullFieldText($this->id, $fieldName, null, $returnRevId);
 	}
 
-	public function setVersionedFieldValue($fieldName, $fieldValue) {
+	public function setDiffableField($fieldName, $fieldValue) {
-		$this->changedVersionedFieldValues[$fieldName] = $fieldValue;
+		$this->changedDiffableFields[$fieldName] = $fieldValue;
 	}
 
 	static public function createId() {
@@ -320,7 +330,7 @@ class BaseModel extends \Illuminate\Database\Eloquent\Model {
 		if ($isNew) $this->created_time = time();
 
 		if ($this->isVersioned) {
-			$changedFields = array_merge($this->getDirty(), $this->changedVersionedFieldValues);
+			$changedFields = array_merge($this->getDirty(), $this->changedDiffableFields);
 			unset($changedFields['updated_time']);
 		}
 
@@ -332,7 +342,7 @@ class BaseModel extends \Illuminate\Database\Eloquent\Model {
 			if (count($changedFields)) {
 				$this->recordChanges($isNew ? 'create' : 'update', $changedFields);
 			}
-			$this->changedVersionedFieldValues = array();
+			$this->changedDiffableFields = array();
 		}
 
 		return $output;
@@ -353,11 +363,11 @@ class BaseModel extends \Illuminate\Database\Eloquent\Model {
 			$change = $this->newChange($type);
 			$change->save();
 		} else if ($type == 'create' || $type == 'update') {
-			// When recording a "create" event, we only record the versioned fields because the complete history
+			// When recording a "create" event, we only record the diffable fields because the complete history
 			// is required to build their value. There's no need to record the other fields since they are
-			// simply new.
+			// new and the client needs to retrieve and save all of them.
 			//
-			// When recording an "update" event, all the modified fields are recorded.
+			// When recording an "update" event, all the modified fields, diffable or not, are recorded.
 			foreach ($changedFields as $field => $value) {
 				if ($type == 'create' && !in_array($field, $this->diffableFields)) continue;
 

src/AppBundle/Model/Change.php

@@ -60,6 +60,9 @@ class Change extends BaseModel {
 			$output[] = $change->toSyncItem();
 		}
 
+		// This is important so that the client knows that the last item in the list
+		// is really the last change that was made; and so they can keep this ID
+		// as reference for the next synchronization.
 		usort($output, function($a, $b) {
 			return strnatcmp($a['id'], $b['id']);
 		});

tests/Model/ChangeTest.php

@@ -58,7 +58,7 @@ class ChangeTest extends BaseTestCase {
 
 		$note = Note::find($noteId);
 
-		$this->assertEquals('test', $note->versionedFieldValue('body'));
+		$this->assertEquals('test', $note->diffableField('body'));
 
 		$note->fromPublicArray(array('body' => 'test'));
 		$note->owner_id = $this->userId();
@@ -66,7 +66,7 @@ class ChangeTest extends BaseTestCase {
 
 		$note = Note::find($noteId);
 
-		$this->assertEquals('test', $note->versionedFieldValue('body'));
+		$this->assertEquals('test', $note->diffableField('body'));
 	}
 
 	public function testDiff3Ways() {
@@ -129,7 +129,7 @@ class ChangeTest extends BaseTestCase {
 
 	public function testRevId() {
 		$n = new Note();
-		$n->setVersionedFieldValue('body', 'abcd efgh');
+		$n->setDiffableField('body', 'abcd efgh');
 		$n->save();
 
 		$noteId = $n->id;
@@ -138,7 +138,7 @@ class ChangeTest extends BaseTestCase {
 		$d = $n->toPublicArray();
 		$this->assertEquals(1, $d['rev_id']);
 
-		$n->setVersionedFieldValue('body', '123456');
+		$n->setDiffableField('body', '123456');
 		$n->save();
 
 		$n = Note::find($noteId);

tests/Model/NoteTest.php

@@ -12,14 +12,14 @@ class NoteTest extends BaseTestCase {
 
 	public function testCanSaveAndLoad() {
 		$note = new Note();
-		$note->setVersionedFieldValue('title', 'the title');
+		$note->setDiffableField('title', 'the title');
-		$note->setVersionedFieldValue('body', 'the body');
+		$note->setDiffableField('body', 'the body');
 		$note->save();
 
 		$note = $note->find($note->id);
 		$this->assertNotNull($note);
-		$this->assertEquals('the title', $note->versionedFieldValue('title'));
+		$this->assertEquals('the title', $note->diffableField('title'));
-		$this->assertEquals('the body', $note->versionedFieldValue('body'));
+		$this->assertEquals('the body', $note->diffableField('body'));
 	}
 
 	public function testFromToPublicArray() {