From 6dbbdd0afe77f69d8dad3af6fee5df6fb2336ce1 Mon Sep 17 00:00:00 2001
From: Gursheen Anand <gursheen@frappe.io>
Date: Fri, 18 Aug 2023 18:14:23 +0530
Subject: [PATCH] refactor: code cleanup

---
 .../document_comparator.json                  | 10 ++-
 .../document_comparator.py                    | 87 +++++++++----------
 2 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/frappe/core/doctype/document_comparator/document_comparator.json b/frappe/core/doctype/document_comparator/document_comparator.json
index fc0644731f..a41bab478f 100644
--- a/frappe/core/doctype/document_comparator/document_comparator.json
+++ b/frappe/core/doctype/document_comparator/document_comparator.json
@@ -16,14 +16,18 @@
   {
    "fieldname": "doctype_name",
    "fieldtype": "Link",
+   "in_list_view": 1,
    "label": "Doctype",
-   "options": "DocType"
+   "options": "DocType",
+   "reqd": 1
   },
   {
    "fieldname": "document",
    "fieldtype": "Dynamic Link",
+   "in_list_view": 1,
    "label": "Document",
-   "options": "doctype_name"
+   "options": "doctype_name",
+   "reqd": 1
   },
   {
    "fieldname": "column_break_peck",
@@ -44,7 +48,7 @@
  "index_web_pages_for_search": 1,
  "issingle": 1,
  "links": [],
- "modified": "2023-08-18 14:07:50.848659",
+ "modified": "2023-08-18 17:57:02.576653",
  "modified_by": "Administrator",
  "module": "Core",
  "name": "Document Comparator",
diff --git a/frappe/core/doctype/document_comparator/document_comparator.py b/frappe/core/doctype/document_comparator/document_comparator.py
index 4857b8e5b6..dcf6537e64 100644
--- a/frappe/core/doctype/document_comparator/document_comparator.py
+++ b/frappe/core/doctype/document_comparator/document_comparator.py
@@ -17,8 +17,8 @@ class DocumentComparator(Document):
 	if TYPE_CHECKING:
 		from frappe.types import DF
 
-		doctype_name: DF.Link | None
-		document: DF.DynamicLink | None
+		doctype_name: DF.Link
+		document: DF.DynamicLink
 	# end: auto-generated types
 	pass
 
@@ -32,58 +32,51 @@ class DocumentComparator(Document):
 			limit=5,
 		)
 		amended_docs = [frappe.get_doc(self.doctype_name, name) for name in amended_document_names]
+		self.docs_to_compare = len(amended_docs)
 
-		changed = {}
-		row_changed = {}
-		for i in range(1, len(amended_docs)):
+		self.changed = {}
+		self.row_changed = {}
+
+		for i in range(1, self.docs_to_compare):
 			diff = get_diff(amended_docs[i - 1], amended_docs[i], compare_cancelled=True)
-			changed = get_diff_grid(amended_docs, i, diff, "changed", changed)
-			row_changed = get_rows_updated_grid(amended_docs, i, diff, "row_changed", row_changed)
+			self.get_diff_grid(i, diff)
+			self.get_rows_updated_grid(i, diff)
 
-		return amended_document_names, {
-			"changed": changed,
-			"row_changed": row_changed,
-		}
+		return amended_document_names, {"changed": self.changed, "row_changed": self.row_changed}
 
-
-def get_diff_grid(amended_docs, i, diff, key, changed_fields):
-	for change in diff[key]:
-		fieldname = get_field_label(change[0], doctype=amended_docs[0].doctype)
-		value = change[-1]
-		if fieldname not in changed_fields:
-			changed_fields[fieldname] = [""] * len(amended_docs)
-		changed_fields[fieldname][i] = value if value else ""
-
-		if i == 1:
-			value = change[1]
-			changed_fields[fieldname][i - 1] = value if value else ""
-
-	return changed_fields
-
-
-def get_rows_updated_grid(amended_docs, i, diff, key, changed_fields):
-	# set an empty dictionary for each table
-	# so it does not get overwritten for every change in same table
-	for table in diff[key]:
-		table_name = get_field_label(table[0], doctype=amended_docs[0].doctype)
-		changed_fields[table_name] = {}
-
-	for change in diff[key]:
-		table_name = get_field_label(change[0], doctype=amended_docs[0].doctype)
-		index = change[1]
-		changed_fields[table_name][index] = {}
-		for field in change[-1]:
-			fieldname = get_field_label(field[0], is_child=True)
-			value = field[-1]
-			if fieldname not in changed_fields[table_name][index]:
-				changed_fields[table_name][index][fieldname] = [""] * len(amended_docs)
-			changed_fields[table_name][index][fieldname][i] = value if value else ""
+	def get_diff_grid(self, i, diff):
+		for change in diff.changed:
+			fieldname = get_field_label(change[0], doctype=self.doctype_name)
+			value = change[-1]
+			if fieldname not in self.changed:
+				self.changed[fieldname] = [""] * self.docs_to_compare
+			self.changed[fieldname][i] = value or ""
 
 			if i == 1:
-				value = field[1]
-				changed_fields[table_name][index][fieldname][i - 1] = value if value else ""
+				value = change[1]
+				self.changed[fieldname][i - 1] = value or ""
 
-	return changed_fields
+	def get_rows_updated_grid(self, i, diff):
+		# set an empty dictionary for each table
+		# so it does not get overwritten for every change in same table
+		for table in diff.row_changed:
+			table_name = get_field_label(table[0], doctype=self.doctype_name)
+			self.row_changed[table_name] = {}
+
+		for change in diff.row_changed:
+			table_name = get_field_label(change[0], doctype=self.doctype_name)
+			index = change[1]
+			self.row_changed[table_name][index] = {}
+			for field in change[-1]:
+				fieldname = get_field_label(field[0], is_child=True)
+				value = field[-1]
+				if fieldname not in self.row_changed[table_name][index]:
+					self.row_changed[table_name][index][fieldname] = [""] * self.docs_to_compare
+				self.row_changed[table_name][index][fieldname][i] = value or ""
+
+				if i == 1:
+					value = field[1]
+					self.row_changed[table_name][index][fieldname][i - 1] = value or ""
 
 
 def get_field_label(fieldname, doctype=None, is_child=False):