vassili/seitime-frappe
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix: more bad migrations and sanity test (#33112)

Browse source 
* test: prevent unnecessary migrations

* fix: Avoid resyncing JSON repeatedly

* fix: Varchar not nullable defaults should be casted

* fix: force cast to float before

Bad default values cause it to break.
This commit is contained in:
Ankush Menat 2025-06-26 16:01:55 +05:30 • committed by GitHub
parent 56c7295b8c
commit 1a1dc0a62c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 26 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
frappe/database/schema.py
View file

@ -275,7 +275,10 @@ class DbColumn:
return
# type
if current_def["type"] != column_type:
if current_def["type"] != column_type and not (
# XXX: MariaDB JSON is same as longtext and information schema still returns longtext
current_def["type"] == "longtext" and column_type == "json" and frappe.db.db_type == "mariadb"
):
self.table.change_type.append(self)
# unique
@ -313,20 +316,25 @@ class DbColumn:
else:
cur_default = current_def.get("default")
new_default = self.default
if cur_default == "NULL" or cur_default is None:
if cur_default == "NULL":
cur_default = None
else:
# Strip quotes from default value
# eg. database returns default value as "'System Manager'"
cur_default = cur_default.lstrip("'").rstrip("'")
cur_default = cur_default.lstrip("'").rstrip("'").replace("\\\\", "\\")
fieldtype = self.fieldtype
db_field_type = frappe.db.type_map.get(fieldtype)
if fieldtype in ["Int", "Check"]:
cur_default = cint(cur_default)
new_default = cint(new_default)
elif fieldtype in ["Currency", "Float", "Percent"]:
cur_default = flt(cur_default)
new_default = flt(new_default)
elif db_field_type and db_field_type[0] in ("varchar", "longtext", "text"):
new_default = cstr(new_default)
if not current_def.get("not_nullable"):
cur_default = cstr(cur_default)
return cur_default != new_default
def default_changed_for_decimal(self, current_def):
@ -336,7 +344,7 @@ class DbColumn:
elif (
current_def["default"]
and float(current_def["default"]) == 0.0
and flt(current_def["default"]) == 0.0
and self.default in ("", None, 0.0)
):
return False

5
frappe/tests/classes/integration_test_case.py
View file

@ -4,6 +4,7 @@ from contextlib import AbstractContextManager, contextmanager
from types import MappingProxyType
import frappe
from frappe.database.utils import get_query_type
from frappe.utils import cint
from ..utils.generators import get_missing_records_module_overrides, make_test_records
@ -112,7 +113,7 @@ class IntegrationTestCase(UnitTestCase):
self._secondary_connection.rollback()
@contextmanager
def assertQueryCount(self, count: int) -> AbstractContextManager[None]:
def assertQueryCount(self, count: int, query_type: tuple[str] | None = None):
queries = []
def _sql_with_count(*args, **kwargs):
@ -124,6 +125,8 @@ class IntegrationTestCase(UnitTestCase):
orig_sql = frappe.db.__class__.sql
frappe.db.__class__.sql = _sql_with_count
yield
if query_type:
queries = [q for q in queries if get_query_type(q) in query_type]
self.assertLessEqual(len(queries), count, msg="Queries executed: \n" + "\n\n".join(queries))
finally:
frappe.db.__class__.sql = orig_sql

10
frappe/tests/test_db_update.py
View file

@ -175,6 +175,16 @@ class TestDBUpdate(IntegrationTestCase):
self.assertEqual(frappe.db.get_column_type(referring_doctype.name, link), "uuid")
class TestDBUpdateSanityChecks(IntegrationTestCase):
@run_only_if(db_type_is.MARIADB)
def test_no_unnecessary_migrates(self):
for doctype in frappe.get_all("DocType", {"is_virtual": 0, "custom": 0}, pluck="name"):
with self.subTest(f"Check {doctype}"):
frappe.reload_doctype(doctype, force=True)
with self.assertQueryCount(0, query_type=("alter",)):
frappe.reload_doctype(doctype, force=True)
def get_fieldtype_from_def(field_def):
fieldtuple = frappe.db.type_map.get(field_def.fieldtype, ("", 0))
fieldtype = fieldtuple[0]