Merge pull request #15426 from gavindsouza/feedback-bs

refactor: Fractional ratings patch

frappe/database/mariadb/database.py

@@ -135,9 +135,10 @@ class MariaDBDatabase(Database):
 		table_name = get_table_name(doctype)
 		return self.sql(f"DESC `{table_name}`")
 
-	def change_column_type(self, doctype: str, column: str, type: str) -> Union[List, Tuple]:
+	def change_column_type(self, doctype: str, column: str, type: str, nullable: bool = False) -> Union[List, Tuple]:
 		table_name = get_table_name(doctype)
-		return self.sql(f"ALTER TABLE `{table_name}` MODIFY `{column}` {type} NOT NULL")
+		null_constraint = "NOT NULL" if not nullable else ""
+		return self.sql(f"ALTER TABLE `{table_name}` MODIFY `{column}` {type} {null_constraint}")
 
 	# exception types
 	@staticmethod

frappe/database/postgres/database.py

@@ -183,9 +183,12 @@ class PostgresDatabase(Database):
 		table_name = get_table_name(doctype)
 		return self.sql(f"SELECT COLUMN_NAME FROM information_schema.COLUMNS WHERE TABLE_NAME = '{table_name}'")
 
-	def change_column_type(self, doctype: str, column: str, type: str) -> Union[List, Tuple]:
+	def change_column_type(self, doctype: str, column: str, type: str, nullable: bool = False) -> Union[List, Tuple]:
 		table_name = get_table_name(doctype)
-		return self.sql(f'ALTER TABLE "{table_name}" ALTER COLUMN "{column}" TYPE {type}')
+		null_constraint = "SET NOT NULL" if not nullable else "DROP NOT NULL"
+		return self.sql(f"""ALTER TABLE "{table_name}"
+								ALTER COLUMN "{column}" TYPE {type},
+								ALTER COLUMN "{column}" {null_constraint}""")
 
 	def create_auth_table(self):
 		self.sql_ddl("""create table if not exists "__Auth" (

frappe/patches.txt

@@ -188,5 +188,5 @@ frappe.patches.v14_0.copy_mail_data #08.03.21
 frappe.patches.v14_0.update_workspace2 # 20.09.2021
 frappe.patches.v14_0.update_github_endpoints #08-11-2021
 frappe.patches.v14_0.remove_db_aggregation
-frappe.patches.v14_0.save_ratings_in_fraction
+frappe.patches.v14_0.save_ratings_in_fraction #23-12-2021
 frappe.patches.v14_0.update_color_names_in_kanban_board_column

frappe/patches/v14_0/save_ratings_in_fraction.py

@@ -1,12 +1,39 @@
 import frappe
+from frappe.query_builder import DocType
+
 
 def execute():
-	rating_fields = frappe.get_all("DocField", fields=["parent", "fieldname"], filters={"fieldtype": "Rating"})
+	RATING_FIELD_TYPE = "decimal(3,2)"
+	rating_fields = frappe.get_all(
+		"DocField", fields=["parent", "fieldname"], filters={"fieldtype": "Rating"}
+	)
+
+	custom_rating_fields = frappe.get_all(
+		"Custom Field", fields=["dt", "fieldname"], filters={"fieldtype": "Rating"}
+	)
+
+	for _field in rating_fields + custom_rating_fields:
+		doctype_name = _field.get("parent") or _field.get("dt")
+		doctype = DocType(doctype_name)
+		field = _field.fieldname
+
+		# TODO: Add postgres support (for the check)
+		if (
+			frappe.conf.db_type == "mariadb"
+			and frappe.db.get_column_type(doctype_name, field) == RATING_FIELD_TYPE
+		):
+			continue
+
+		# commit any changes so far for upcoming DDL
+		frappe.db.commit()
+
+		# alter column types for rating fieldtype
+		frappe.db.change_column_type(doctype_name, column=field, type=RATING_FIELD_TYPE, nullable=True)
 
-	custom_rating_fields = frappe.get_all("Custom Field", fields=["dt", "fieldname"], filters={"fieldtype": "Rating"})
+		# update data: int => decimal
+		frappe.qb.update(doctype).set(
+			doctype[field], doctype[field] / 5
+		).run()
 
-	for field in rating_fields + custom_rating_fields:
-		doctype_name = field.get("parent") or field.get("dt")
-		doctype = frappe.qb.DocType(doctype_name)
-		field = field.fieldname
-		(frappe.qb.update(doctype_name).set(doctype[field], doctype[field]/5)).run()
+		# commit to flush updated rows
+		frappe.db.commit()