[versioning] check for empty field values (#3739)

* [versioning] check for empty field values

* [test] get_diff for changes across docs

frappe/core/doctype/version/test_version.py

@@ -3,9 +3,37 @@
 from __future__ import unicode_literals
 
 import frappe
-import unittest
-
-test_records = frappe.get_test_records('Version')
+import unittest, copy
+from frappe.test_runner import make_test_objects
+from frappe.core.doctype.version.version import get_diff
 
 class TestVersion(unittest.TestCase):
-	pass
+	def test_get_diff(self):
+		test_records = make_test_objects('Event', reset = True)
+		old_doc = frappe.get_doc("Event", test_records[0])
+		new_doc = copy.deepcopy(old_doc)
+
+		old_doc.color = None
+
+		diff = get_diff(old_doc, new_doc)['changed']
+
+		self.assertEquals(get_fieldnames(diff)[0], 'color')
+		self.assertTrue(get_old_values(diff)[0] is None)
+		self.assertEquals(get_new_values(diff)[0], 'blue')
+
+		new_doc.starts_on = "2017-07-20"
+
+		diff = get_diff(old_doc, new_doc)['changed']
+
+		self.assertEquals(get_fieldnames(diff)[0], 'starts_on')
+		self.assertEquals(get_old_values(diff)[0], '01-01-2014 00:00:00')
+		self.assertEquals(get_new_values(diff)[0], '07-20-2017 00:00:00')
+
+def get_fieldnames(change_array):
+	return [d[0] for d in change_array]
+
+def get_old_values(change_array):
+	return [d[1] for d in change_array]
+
+def get_new_values(change_array):
+	return [d[2] for d in change_array]

frappe/core/doctype/version/version.py

@@ -69,10 +69,13 @@ def get_diff(old, new, for_child=False):
 				if not d.name in new_row_by_name:
 					out.removed.append([df.fieldname, d.as_dict()])
 
-		elif (old_value != new_value
-			and old.get_formatted(df.fieldname) != new.get_formatted(df.fieldname)):
-			out.changed.append((df.fieldname, old.get_formatted(df.fieldname),
-				new.get_formatted(df.fieldname)))
+		elif (old_value != new_value):
+			# Check for None values
+			old_data = old.get_formatted(df.fieldname) if old_value else old_value
+			new_data = new.get_formatted(df.fieldname) if new_value else new_value
+
+			if old_data != new_data:
+				out.changed.append((df.fieldname, old_data, new_data))
 
 	# docstatus
 	if not for_child and old.docstatus != new.docstatus: