From 646d2271b874eb5d173b77065ef1db59d97ffe84 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 01:04:08 +0530 Subject: [PATCH 01/15] fix: allow setting autoincrement autoname from customize form * chore: updated the warning under autoincrement autoname --- cypress/integration/customize_form.js | 1 + frappe/custom/doctype/customize_form/customize_form.js | 4 ---- frappe/custom/doctype/customize_form/customize_form.json | 6 +++--- frappe/public/js/frappe/doctype/index.js | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/cypress/integration/customize_form.js b/cypress/integration/customize_form.js index 70615085c3..01b4ebd731 100644 --- a/cypress/integration/customize_form.js +++ b/cypress/integration/customize_form.js @@ -6,6 +6,7 @@ context('Customize Form', () => { cy.fill_field("doc_type", "ToDo", "Link").blur(); cy.click_form_section("Naming"); const naming_rule_default_autoname_map = { + "Autoincrement": "autoincrement", "Set by user": "prompt", "By fieldname": "field:", 'By "Naming Series" field': "naming_series:", diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index 4ce2c73fa3..4c2d207df9 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -152,10 +152,6 @@ frappe.ui.form.on("Customize Form", { }, __("Actions") ); - - const is_autoname_autoincrement = frm.doc.autoname === 'autoincrement'; - frm.set_df_property("naming_rule", "hidden", is_autoname_autoincrement); - frm.set_df_property("autoname", "read_only", is_autoname_autoincrement); } frm.events.setup_export(frm); diff --git a/frappe/custom/doctype/customize_form/customize_form.json b/frappe/custom/doctype/customize_form/customize_form.json index a0bc994c45..6bb29fa525 100644 --- a/frappe/custom/doctype/customize_form/customize_form.json +++ b/frappe/custom/doctype/customize_form/customize_form.json @@ -56,7 +56,7 @@ "fieldtype": "Select", "label": "Naming Rule", "length": 40, - "options": "\nSet by user\nBy fieldname\nBy \"Naming Series\" field\nExpression\nExpression (old style)\nRandom\nBy script" + "options": "\nSet by user\nAutoincrement\nBy fieldname\nBy \"Naming Series\" field\nExpression\nExpression (old style)\nRandom\nBy script" }, { "fieldname": "doc_type", @@ -287,7 +287,7 @@ "label": "Naming" }, { - "description": "Naming Options:\n
  1. field:[fieldname] - By Field
    2. \n
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", + "description": "Naming Options:\n
  1. field:[fieldname] - By Field
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present)
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", "fieldname": "autoname", "fieldtype": "Data", "label": "Auto Name" @@ -319,7 +319,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2022-04-21 15:36:16.772277", + "modified": "2022-04-30 15:36:16.772277", "modified_by": "Administrator", "module": "Custom", "name": "Customize Form", diff --git a/frappe/public/js/frappe/doctype/index.js b/frappe/public/js/frappe/doctype/index.js index 09f020f370..8acd32cc7f 100644 --- a/frappe/public/js/frappe/doctype/index.js +++ b/frappe/public/js/frappe/doctype/index.js @@ -48,7 +48,7 @@ frappe.model.DocTypeController = class DocTypeController extends frappe.ui.form. set_naming_rule_description() { let naming_rule_description = { 'Set by user': '', - 'Autoincrement': 'Uses Auto Increment feature of database.
WARNING: After using this option, any other naming option will not be accessible.', + 'Autoincrement': 'Uses Auto Increment feature of database.
WARNING: Can only change to/from this option, if no data is present in the doctype.', 'By fieldname': 'Format: field:[fieldname]. Valid fieldname must exist', 'By "Naming Series" field': 'Format: naming_series:[fieldname]. Default fieldname is naming_series', 'Expression': 'Format: format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.', From aa6a21fc2c16476744fe9ba14bbaebfe820c3c4a Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 01:05:26 +0530 Subject: [PATCH 02/15] chore: add regex to consider space in cast_name --- frappe/model/db_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index acb63b5bfa..b24adc42c8 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -922,7 +922,7 @@ def cast_name(column: str) -> str: if "cast(" not in column.lower() and "::" not in column: if re.search(r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\s*\)", **kwargs): return re.sub( - r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\)", r"locate(\1, cast(\2 as varchar))", **kwargs + r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\s*\)", r"locate(\1, cast(\2 as varchar))", **kwargs ) elif match := re.search(r"(strpos|ifnull|coalesce)\(\s*([`\"]?name[`\"]?)\s*,", **kwargs): From 7ec5e882787def53077e5d2d40ab157d09f18cb8 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 01:14:01 +0530 Subject: [PATCH 03/15] chore: remove unnecessary groups from search regex in cast_name --- frappe/model/db_query.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index b24adc42c8..005b7e3741 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -920,12 +920,12 @@ def cast_name(column: str) -> str: kwargs = {"string": column, "flags": re.IGNORECASE} if "cast(" not in column.lower() and "::" not in column: - if re.search(r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\s*\)", **kwargs): + if re.search(r"locate\([^,]+,\s*[`\"]?name[`\"]?\s*\)", **kwargs): return re.sub( r"locate\(([^,]+),\s*([`\"]?name[`\"]?)\s*\)", r"locate(\1, cast(\2 as varchar))", **kwargs ) - elif match := re.search(r"(strpos|ifnull|coalesce)\(\s*([`\"]?name[`\"]?)\s*,", **kwargs): + elif match := re.search(r"(strpos|ifnull|coalesce)\(\s*[`\"]?name[`\"]?\s*,", **kwargs): func = match.groups()[0] return re.sub(rf"{func}\(\s*([`\"]?name[`\"]?)\s*,", rf"{func}(cast(\1 as varchar),", **kwargs) From ef68e81b93447ada43a9a8dd847565f53a9426c9 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 12:13:57 +0530 Subject: [PATCH 04/15] fix: revert removing unnecessary branching from -> https://github.com/frappe/frappe/pull/16307/files\#diff-d0ce4df383292475f6121492e8a2242314bbe9678f0acd020c45d7b2ae85a214L68-R68 --- frappe/public/js/frappe/form/controls/base_input.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/base_input.js b/frappe/public/js/frappe/form/controls/base_input.js index 5a5af389ee..b7fe61b385 100644 --- a/frappe/public/js/frappe/form/controls/base_input.js +++ b/frappe/public/js/frappe/form/controls/base_input.js @@ -65,7 +65,11 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control }; var update_input = function() { - me.set_input(me.value); + if (me.doctype && me.docname) { + me.set_input(me.value); + } else { + me.set_input(me.value || null); + } }; if (me.disp_status != "None") { From 9f0c40dbba206dd4ac1122e1d4cfb69437a2151c Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 14:36:56 +0530 Subject: [PATCH 05/15] fix: create sequence onchange from any other autoname whcih is not autoincrement * chore: use class variable for determining sequence cache * chore: use sql_ddl when creating sequence --- frappe/core/doctype/doctype/doctype.py | 25 +++++++++++++------ .../doctype/customize_form/customize_form.py | 7 ++---- frappe/database/mariadb/database.py | 2 ++ frappe/database/mariadb/schema.py | 2 +- frappe/database/postgres/database.py | 2 ++ frappe/database/postgres/schema.py | 3 ++- frappe/database/sequence.py | 4 +-- 7 files changed, 29 insertions(+), 16 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 6bd7f2306f..f906011d97 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -102,6 +102,7 @@ class DocType(Document): validate_series(self) self.validate_document_type() validate_fields(self) + self.can_change_name_type = check_if_can_change_name_type(self) if not self.istable: validate_permissions(self) @@ -124,12 +125,6 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_("Standard DocType cannot have default print format, use Customize Form")) - if check_if_can_change_name_type(self): - change_name_column_type( - self.name, - "bigint" if self.autoname == "autoincrement" else f"varchar({frappe.db.VARCHAR_LEN})", - ) - def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" core_doctypes = [ @@ -177,6 +172,9 @@ class DocType(Document): ) def after_insert(self): + if self.can_change_name_type: + setup_name_type_and_sequence(self) + # clear user cache so that on the next reload this doctype is included in boot clear_user_cache(frappe.session.user) @@ -930,6 +928,9 @@ def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: and autoname_before_save != "autoincrement" or (not is_autoname_autoincrement and autoname_before_save == "autoincrement") ): + if frappe.get_meta(doctype_name).issingle: + return False + if not frappe.get_all(doctype_name, limit=1): # allow changing the column type if there is no data return True @@ -942,8 +943,18 @@ def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: return False +def setup_name_type_and_sequence(dt: DocType) -> None: + doctype_name = dt.doc_type if dt.doctype == "Customize Form" else dt.name + name_type = f"varchar({frappe.db.VARCHAR_LEN})" + if dt.autoname == "autoincrement": + name_type = "bigint" + frappe.db.create_sequence(doctype_name, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) + + change_name_column_type(doctype_name, name_type) + + def change_name_column_type(doctype_name: str, type: str) -> None: - return frappe.db.change_column_type( + frappe.db.change_column_type( doctype_name, "name", type, True if frappe.db.db_type == "mariadb" else False ) diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index 262542fd4b..c06b353932 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -11,9 +11,9 @@ import frappe import frappe.translate from frappe import _ from frappe.core.doctype.doctype.doctype import ( - change_name_column_type, check_email_append_to, check_if_can_change_name_type, + setup_name_type_and_sequence, validate_fields_for_doctype, validate_series, ) @@ -173,10 +173,7 @@ class CustomizeForm(Document): check_email_append_to(self) if can_change_name_type: - change_name_column_type( - self.doc_type, - "bigint" if self.autoname == "autoincrement" else f"varchar({frappe.db.VARCHAR_LEN})", - ) + setup_name_type_and_sequence(self) if self.flags.update_db: frappe.db.updatedb(self.doc_type) diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index 28d78471d2..ee8a5ecdfd 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -19,6 +19,8 @@ class MariaDBDatabase(Database): DataError = pymysql.err.DataError REGEX_CHARACTER = "regexp" + SEQUENCE_CACHE = 50 + def setup_type_map(self): self.db_type = "mariadb" self.type_map = { diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 784fa23c13..87ba905ce2 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -47,7 +47,7 @@ class MariaDBTable(DBTable): # By default the cache is 1000 which will mess up the sequence when # using the system after a restore. # issue link: https://jira.mariadb.org/browse/MDEV-21786 - frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=50) + frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) # NOTE: not used nextval func as default as the ability to restore # database with sequences has bugs in mariadb and gives a scary error. diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index d69e0bea94..15ba86aeba 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -31,6 +31,8 @@ class PostgresDatabase(Database): InterfaceError = psycopg2.InterfaceError REGEX_CHARACTER = "~" + SEQUENCE_CACHE = 0 + def setup_type_map(self): self.db_type = "postgres" self.type_map = { diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index 2abd5f37c7..0777ed6cc0 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -38,7 +38,8 @@ class PostgresTable(DBTable): # Since we're opening and closing connections for every transaction this results in skipping the cache # to the next non-cached value hence not using cache in postgres. # ref: https://stackoverflow.com/questions/21356375/postgres-9-0-4-sequence-skipping-numbers - frappe.db.create_sequence(self.doctype, check_not_exists=True) + frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) + name_column = "name bigint primary key" # TODO: set docstatus length diff --git a/frappe/database/sequence.py b/frappe/database/sequence.py index ede4689485..6a352d20d1 100644 --- a/frappe/database/sequence.py +++ b/frappe/database/sequence.py @@ -5,7 +5,7 @@ def create_sequence( doctype_name: str, *, slug: str = "_id_seq", - temporary=False, + temporary: bool = False, check_not_exists: bool = False, cycle: bool = False, cache: int = 0, @@ -51,7 +51,7 @@ def create_sequence( else: query += " cycle" - db.sql(query) + db.sql_ddl(query) return sequence_name From cfc905b567cb316b37c8dd7eeb006ebd2ded5144 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 16:16:41 +0530 Subject: [PATCH 06/15] fix: use cast when chaninging name type from varchar to bigint in postgres --- frappe/core/doctype/doctype/doctype.py | 15 ++++++++++----- frappe/database/postgres/database.py | 7 ++++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index f906011d97..18ab59544b 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -125,6 +125,9 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_("Standard DocType cannot have default print format, use Customize Form")) + if self.can_change_name_type: + setup_name_type_and_sequence(self) + def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" core_doctypes = [ @@ -172,9 +175,6 @@ class DocType(Document): ) def after_insert(self): - if self.can_change_name_type: - setup_name_type_and_sequence(self) - # clear user cache so that on the next reload this doctype is included in boot clear_user_cache(frappe.session.user) @@ -954,10 +954,15 @@ def setup_name_type_and_sequence(dt: DocType) -> None: def change_name_column_type(doctype_name: str, type: str) -> None: - frappe.db.change_column_type( - doctype_name, "name", type, True if frappe.db.db_type == "mariadb" else False + # postgres requires cast when converting from varchar to bigint + args = ( + (doctype_name, "name", type, False, True) + if (frappe.db.db_type == "postgres") + else (doctype_name, "name", type, True) ) + frappe.db.change_column_type(*args) + def validate_links_table_fieldnames(meta): """Validate fieldnames in Links table""" diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index 15ba86aeba..3334f0ecd8 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -211,18 +211,19 @@ class PostgresDatabase(Database): ) def change_column_type( - self, doctype: str, column: str, type: str, nullable: bool = False + self, doctype: str, column: str, type: str, nullable: bool = False, use_cast: bool = False ) -> Union[List, Tuple]: table_name = get_table_name(doctype) null_constraint = "SET NOT NULL" if not nullable else "DROP NOT NULL" + using_cast = f'using "{column}"::{type}' if use_cast else "" # postgres allows ddl in transactions but since we've currently made # things same as mariadb (raising exception on ddl commands if the transaction has any writes), # hence using sql_ddl here for committing and then moving forward. return self.sql_ddl( f"""ALTER TABLE "{table_name}" - ALTER COLUMN "{column}" TYPE {type}, - ALTER COLUMN "{column}" {null_constraint}""" + ALTER COLUMN "{column}" TYPE {type} {using_cast}, + ALTER COLUMN "{column}" {null_constraint}""" ) def create_auth_table(self): From 87916060c942c49a3e39133601ed38e3aae32654 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 16:17:49 +0530 Subject: [PATCH 07/15] test: change autoname from customize form --- .../customize_form/test_customize_form.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/frappe/custom/doctype/customize_form/test_customize_form.py b/frappe/custom/doctype/customize_form/test_customize_form.py index 5a1f629beb..b031ca0790 100644 --- a/frappe/custom/doctype/customize_form/test_customize_form.py +++ b/frappe/custom/doctype/customize_form/test_customize_form.py @@ -396,3 +396,54 @@ class TestCustomizeForm(unittest.TestCase): d.label = "" d.run_method("save_customization") self.assertEqual(d.label, "") + + def test_change_autoname(self): + # delete all data from Event doctype + frappe.db.sql("delete from `tabEvent`") + + d = self.get_customize_form("Event") + naming_rule = d.naming_rule + autoname = d.autoname + + # change autoname + d.naming_rule = "Autoincrement" + d.autoname = "autoincrement" + d.run_method("save_customization") + + # check if name type has been changed + self.assertEqual( + frappe.db.sql( + """select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tabEvent'""" + )[0][0], + "bigint", + ) + + if frappe.db.db_type == "mariadb": + table_name = "information_schema.tables" + conditions = "table_type = 'sequence' and table_name = 'event_id_seq'" + else: + table_name = "information_schema.sequences" + conditions = "sequence_name = 'event_id_seq'" + + # check if sequence table is created + self.assertTrue( + frappe.db.sql( + f"""select * from {table_name} + where {conditions}""" + ) + ) + + # change the autoname/naming rule back to original + d.autoname = autoname + d.naming_rule = naming_rule + d.run_method("save_customization") + + # check if name type has changed + self.assertEqual( + frappe.db.sql( + """select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tabEvent'""" + )[0][0], + "varchar" if frappe.db.db_type == "mariadb" else "character varying", + ) From 2ba1b6a5499365cd8560c9a508a62eefcbb5117e Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 16:27:09 +0530 Subject: [PATCH 08/15] chore: better naming for setup_name_type_and_sequence --- frappe/core/doctype/doctype/doctype.py | 8 ++++---- frappe/custom/doctype/customize_form/customize_form.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 18ab59544b..9051cb579b 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -125,9 +125,6 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_("Standard DocType cannot have default print format, use Customize Form")) - if self.can_change_name_type: - setup_name_type_and_sequence(self) - def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" core_doctypes = [ @@ -372,6 +369,9 @@ class DocType(Document): def on_update(self): """Update database schema, make controller templates if `custom` is not set and clear cache.""" + if self.can_change_name_type: + change_name_type_and_make_sequence(self) + try: frappe.db.updatedb(self.name, Meta(self)) except Exception as e: @@ -943,7 +943,7 @@ def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: return False -def setup_name_type_and_sequence(dt: DocType) -> None: +def change_name_type_and_make_sequence(dt: DocType) -> None: doctype_name = dt.doc_type if dt.doctype == "Customize Form" else dt.name name_type = f"varchar({frappe.db.VARCHAR_LEN})" if dt.autoname == "autoincrement": diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index c06b353932..d77f85790c 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -11,9 +11,9 @@ import frappe import frappe.translate from frappe import _ from frappe.core.doctype.doctype.doctype import ( + change_name_type_and_make_sequence, check_email_append_to, check_if_can_change_name_type, - setup_name_type_and_sequence, validate_fields_for_doctype, validate_series, ) @@ -173,7 +173,7 @@ class CustomizeForm(Document): check_email_append_to(self) if can_change_name_type: - setup_name_type_and_sequence(self) + change_name_type_and_make_sequence(self) if self.flags.update_db: frappe.db.updatedb(self.doc_type) From 3946a84cc34e60d6b35376eec6d1deb90ad8bfcc Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 16:41:00 +0530 Subject: [PATCH 09/15] fix: move change_name_type_and_make_sequence to validate method of Doctype --- frappe/core/doctype/doctype/doctype.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 9051cb579b..2559091bf1 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -102,7 +102,6 @@ class DocType(Document): validate_series(self) self.validate_document_type() validate_fields(self) - self.can_change_name_type = check_if_can_change_name_type(self) if not self.istable: validate_permissions(self) @@ -125,6 +124,9 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_("Standard DocType cannot have default print format, use Customize Form")) + if check_if_can_change_name_type(self): + change_name_type_and_make_sequence(self) + def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" core_doctypes = [ @@ -369,8 +371,6 @@ class DocType(Document): def on_update(self): """Update database schema, make controller templates if `custom` is not set and clear cache.""" - if self.can_change_name_type: - change_name_type_and_make_sequence(self) try: frappe.db.updatedb(self.name, Meta(self)) From de359bfedc7d101eea711d013f03ee9372cc7883 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sat, 30 Apr 2022 20:45:30 +0530 Subject: [PATCH 10/15] chore: use set and get methods to set attributes This lets us split the ddl things to on_update hook i.e post save --- frappe/core/doctype/doctype/doctype.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 2559091bf1..6d4ec9c497 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -100,6 +100,7 @@ class DocType(Document): self.set_default_in_list_view() self.set_default_translatable() validate_series(self) + self.set("can_change_name_type", check_if_can_change_name_type(self)) self.validate_document_type() validate_fields(self) @@ -124,9 +125,6 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_("Standard DocType cannot have default print format, use Customize Form")) - if check_if_can_change_name_type(self): - change_name_type_and_make_sequence(self) - def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" core_doctypes = [ @@ -372,6 +370,9 @@ class DocType(Document): def on_update(self): """Update database schema, make controller templates if `custom` is not set and clear cache.""" + if self.get("can_change_name_type"): + change_name_type_and_make_sequence(self) + try: frappe.db.updatedb(self.name, Meta(self)) except Exception as e: From edce9a18657ec876138e8566b4110555f590b4ac Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 5 May 2022 20:33:08 +0530 Subject: [PATCH 11/15] Revert "fix: allow setting autoincrement autoname from customize form" This reverts commit d43d9594f0ed6f6d88466879bc36a5c5bca24b77. --- cypress/integration/customize_form.js | 1 - frappe/custom/doctype/customize_form/customize_form.js | 4 ++++ frappe/custom/doctype/customize_form/customize_form.json | 6 +++--- frappe/public/js/frappe/doctype/index.js | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cypress/integration/customize_form.js b/cypress/integration/customize_form.js index 01b4ebd731..70615085c3 100644 --- a/cypress/integration/customize_form.js +++ b/cypress/integration/customize_form.js @@ -6,7 +6,6 @@ context('Customize Form', () => { cy.fill_field("doc_type", "ToDo", "Link").blur(); cy.click_form_section("Naming"); const naming_rule_default_autoname_map = { - "Autoincrement": "autoincrement", "Set by user": "prompt", "By fieldname": "field:", 'By "Naming Series" field': "naming_series:", diff --git a/frappe/custom/doctype/customize_form/customize_form.js b/frappe/custom/doctype/customize_form/customize_form.js index 4c2d207df9..4ce2c73fa3 100644 --- a/frappe/custom/doctype/customize_form/customize_form.js +++ b/frappe/custom/doctype/customize_form/customize_form.js @@ -152,6 +152,10 @@ frappe.ui.form.on("Customize Form", { }, __("Actions") ); + + const is_autoname_autoincrement = frm.doc.autoname === 'autoincrement'; + frm.set_df_property("naming_rule", "hidden", is_autoname_autoincrement); + frm.set_df_property("autoname", "read_only", is_autoname_autoincrement); } frm.events.setup_export(frm); diff --git a/frappe/custom/doctype/customize_form/customize_form.json b/frappe/custom/doctype/customize_form/customize_form.json index 6bb29fa525..a0bc994c45 100644 --- a/frappe/custom/doctype/customize_form/customize_form.json +++ b/frappe/custom/doctype/customize_form/customize_form.json @@ -56,7 +56,7 @@ "fieldtype": "Select", "label": "Naming Rule", "length": 40, - "options": "\nSet by user\nAutoincrement\nBy fieldname\nBy \"Naming Series\" field\nExpression\nExpression (old style)\nRandom\nBy script" + "options": "\nSet by user\nBy fieldname\nBy \"Naming Series\" field\nExpression\nExpression (old style)\nRandom\nBy script" }, { "fieldname": "doc_type", @@ -287,7 +287,7 @@ "label": "Naming" }, { - "description": "Naming Options:\n
  1. field:[fieldname] - By Field
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present)
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", + "description": "Naming Options:\n
  1. field:[fieldname] - By Field
    2. \n
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", "fieldname": "autoname", "fieldtype": "Data", "label": "Auto Name" @@ -319,7 +319,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2022-04-30 15:36:16.772277", + "modified": "2022-04-21 15:36:16.772277", "modified_by": "Administrator", "module": "Custom", "name": "Customize Form", diff --git a/frappe/public/js/frappe/doctype/index.js b/frappe/public/js/frappe/doctype/index.js index 8acd32cc7f..09f020f370 100644 --- a/frappe/public/js/frappe/doctype/index.js +++ b/frappe/public/js/frappe/doctype/index.js @@ -48,7 +48,7 @@ frappe.model.DocTypeController = class DocTypeController extends frappe.ui.form. set_naming_rule_description() { let naming_rule_description = { 'Set by user': '', - 'Autoincrement': 'Uses Auto Increment feature of database.
WARNING: Can only change to/from this option, if no data is present in the doctype.', + 'Autoincrement': 'Uses Auto Increment feature of database.
WARNING: After using this option, any other naming option will not be accessible.', 'By fieldname': 'Format: field:[fieldname]. Valid fieldname must exist', 'By "Naming Series" field': 'Format: naming_series:[fieldname]. Default fieldname is naming_series', 'Expression': 'Format: format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.', From 82aa8dee08354c5cc96ad402786ab3da3297ce8c Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 12 May 2022 15:18:02 +0530 Subject: [PATCH 12/15] fix: don't allow autoincrement autoname from customize form * test: test_change_autoname for doctype * test: test_autoincrement_autoname for customize form --- frappe/core/doctype/doctype/doctype.py | 35 ++++++------- frappe/core/doctype/doctype/test_doctype.py | 51 +++++++++++++++++++ .../doctype/customize_form/customize_form.py | 8 +-- .../customize_form/test_customize_form.py | 50 ++---------------- frappe/desk/doctype/event/event.json | 3 +- 5 files changed, 76 insertions(+), 71 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 6d4ec9c497..bba47e573a 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -100,7 +100,7 @@ class DocType(Document): self.set_default_in_list_view() self.set_default_translatable() validate_series(self) - self.set("can_change_name_type", check_if_can_change_name_type(self)) + self.set("can_change_name_type", validate_autoincrement_autoname(self)) self.validate_document_type() validate_fields(self) @@ -902,26 +902,25 @@ def validate_series(dt, autoname=None, name=None): frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0])) -def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: - def get_autoname_before_save(doctype: str, to_be_customized_dt: str) -> str: - if doctype == "Customize Form": - property_value = frappe.db.get_value( - "Property Setter", {"doc_type": to_be_customized_dt, "property": "autoname"}, "value" - ) +def validate_autoincrement_autoname(dt: DocType) -> bool: + """Checks if chan change to/from autoincrement autoname""" + def get_autoname_before_save(dt) -> str: + if dt.name == "Customize Form": + property_value = frappe.db.get_value( + "Property Setter", {"doc_type": dt.doc_type, "property": "autoname"}, "value" + ) # initially no property setter is set, # hence getting autoname value from the doctype itself if not property_value: - return frappe.db.get_value("DocType", to_be_customized_dt, "autoname") or "" + return frappe.db.get_value("DocType", dt.doc_type, "autoname") or "" return property_value return getattr(dt.get_doc_before_save(), "autoname", "") - doctype_name = dt.doc_type if dt.doctype == "Customize Form" else dt.name - if not dt.is_new(): - autoname_before_save = get_autoname_before_save(dt.doctype, doctype_name) + autoname_before_save = get_autoname_before_save(dt) is_autoname_autoincrement = dt.autoname == "autoincrement" if ( @@ -929,17 +928,19 @@ def check_if_can_change_name_type(dt: DocType, raise_err: bool = True) -> bool: and autoname_before_save != "autoincrement" or (not is_autoname_autoincrement and autoname_before_save == "autoincrement") ): - if frappe.get_meta(doctype_name).issingle: + if dt.name == "Customize Form": + frappe.throw(_("Cannot change to/from autoincrement autoname in Customize Form")) + + if frappe.get_meta(dt.name).issingle: return False - if not frappe.get_all(doctype_name, limit=1): + if not frappe.get_all(dt.name, limit=1): # allow changing the column type if there is no data return True - if raise_err: - frappe.throw( - _("Can only change to/from Autoincrement naming rule when there is no data in the doctype") - ) + frappe.throw( + _("Can only change to/from Autoincrement naming rule when there is no data in the doctype") + ) return False diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 59475a95a7..03908c530e 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -38,6 +38,57 @@ class TestDocType(unittest.TestCase): doc = new_doctype(name).insert() doc.delete() + def test_change_autoname(self): + # delete all data from Event doctype + frappe.db.sql("delete from `tabEvent`") + + d = frappe.get_doc("DocType", "Event") + naming_rule = d.naming_rule + autoname = d.autoname + + # change autoname + d.naming_rule = "Autoincrement" + d.autoname = "autoincrement" + d.save() + + # check if name type has been changed + self.assertEqual( + frappe.db.sql( + """select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tabEvent'""" + )[0][0], + "bigint", + ) + + if frappe.db.db_type == "mariadb": + table_name = "information_schema.tables" + conditions = "table_type = 'sequence' and table_name = 'event_id_seq'" + else: + table_name = "information_schema.sequences" + conditions = "sequence_name = 'event_id_seq'" + + # check if sequence table is created + self.assertTrue( + frappe.db.sql( + f"""select * from {table_name} + where {conditions}""" + ) + ) + + # change the autoname/naming rule back to original + d.autoname = autoname + d.naming_rule = naming_rule + d.save() + + # check if name type has changed + self.assertEqual( + frappe.db.sql( + """select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tabEvent'""" + )[0][0], + "varchar" if frappe.db.db_type == "mariadb" else "character varying", + ) + def test_doctype_unique_constraint_dropped(self): if frappe.db.exists("DocType", "With_Unique"): frappe.delete_doc("DocType", "With_Unique") diff --git a/frappe/custom/doctype/customize_form/customize_form.py b/frappe/custom/doctype/customize_form/customize_form.py index d77f85790c..903d7fc27d 100644 --- a/frappe/custom/doctype/customize_form/customize_form.py +++ b/frappe/custom/doctype/customize_form/customize_form.py @@ -11,9 +11,8 @@ import frappe import frappe.translate from frappe import _ from frappe.core.doctype.doctype.doctype import ( - change_name_type_and_make_sequence, check_email_append_to, - check_if_can_change_name_type, + validate_autoincrement_autoname, validate_fields_for_doctype, validate_series, ) @@ -163,7 +162,7 @@ class CustomizeForm(Document): return validate_series(self, self.autoname, self.doc_type) - can_change_name_type = check_if_can_change_name_type(self) + validate_autoincrement_autoname(self) self.flags.update_db = False self.flags.rebuild_doctype_for_global_search = False self.set_property_setters() @@ -172,9 +171,6 @@ class CustomizeForm(Document): validate_fields_for_doctype(self.doc_type) check_email_append_to(self) - if can_change_name_type: - change_name_type_and_make_sequence(self) - if self.flags.update_db: frappe.db.updatedb(self.doc_type) diff --git a/frappe/custom/doctype/customize_form/test_customize_form.py b/frappe/custom/doctype/customize_form/test_customize_form.py index b031ca0790..4712f272b5 100644 --- a/frappe/custom/doctype/customize_form/test_customize_form.py +++ b/frappe/custom/doctype/customize_form/test_customize_form.py @@ -397,53 +397,9 @@ class TestCustomizeForm(unittest.TestCase): d.run_method("save_customization") self.assertEqual(d.label, "") - def test_change_autoname(self): - # delete all data from Event doctype - frappe.db.sql("delete from `tabEvent`") - + def test_autoincrement_autoname(self): d = self.get_customize_form("Event") - naming_rule = d.naming_rule - autoname = d.autoname - - # change autoname - d.naming_rule = "Autoincrement" d.autoname = "autoincrement" - d.run_method("save_customization") - # check if name type has been changed - self.assertEqual( - frappe.db.sql( - """select data_type FROM information_schema.columns - where column_name = 'name' and table_name = 'tabEvent'""" - )[0][0], - "bigint", - ) - - if frappe.db.db_type == "mariadb": - table_name = "information_schema.tables" - conditions = "table_type = 'sequence' and table_name = 'event_id_seq'" - else: - table_name = "information_schema.sequences" - conditions = "sequence_name = 'event_id_seq'" - - # check if sequence table is created - self.assertTrue( - frappe.db.sql( - f"""select * from {table_name} - where {conditions}""" - ) - ) - - # change the autoname/naming rule back to original - d.autoname = autoname - d.naming_rule = naming_rule - d.run_method("save_customization") - - # check if name type has changed - self.assertEqual( - frappe.db.sql( - """select data_type FROM information_schema.columns - where column_name = 'name' and table_name = 'tabEvent'""" - )[0][0], - "varchar" if frappe.db.db_type == "mariadb" else "character varying", - ) + with self.assertRaises(frappe.ValidationError): + d.run_method("save_customization") diff --git a/frappe/desk/doctype/event/event.json b/frappe/desk/doctype/event/event.json index 2f67c36fc0..bce3b1e65a 100644 --- a/frappe/desk/doctype/event/event.json +++ b/frappe/desk/doctype/event/event.json @@ -277,7 +277,7 @@ "icon": "fa fa-calendar", "idx": 1, "links": [], - "modified": "2021-11-18 05:06:24.881742", + "modified": "2022-05-12 05:43:27.935510", "modified_by": "Administrator", "module": "Desk", "name": "Event", @@ -312,6 +312,7 @@ "sender_field": "sender", "sort_field": "modified", "sort_order": "DESC", + "states": [], "subject_field": "subject", "title_field": "subject", "track_changes": 1, From b1d61906d1fbac3133becbf66b7f13b20adfac04 Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 12 May 2022 22:41:25 +0530 Subject: [PATCH 13/15] test: update and rename test_change_autoname * chore: remove unnecessary decorators for skiprun --- frappe/core/doctype/doctype/test_doctype.py | 33 ++++++------- frappe/tests/test_db.py | 55 ++++++++++++++------- 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 03908c530e..11f5ef8a69 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -38,34 +38,30 @@ class TestDocType(unittest.TestCase): doc = new_doctype(name).insert() doc.delete() - def test_change_autoname(self): - # delete all data from Event doctype - frappe.db.sql("delete from `tabEvent`") - - d = frappe.get_doc("DocType", "Event") - naming_rule = d.naming_rule - autoname = d.autoname + def test_making_sequence_on_change(self): + frappe.delete_doc_if_exists("DocType", self._testMethodName) + dt = new_doctype(self._testMethodName).insert(ignore_permissions=True) + autoname = dt.autoname # change autoname - d.naming_rule = "Autoincrement" - d.autoname = "autoincrement" - d.save() + dt.autoname = "autoincrement" + dt.save() # check if name type has been changed self.assertEqual( frappe.db.sql( - """select data_type FROM information_schema.columns - where column_name = 'name' and table_name = 'tabEvent'""" + f"""select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tab{self._testMethodName}'""" )[0][0], "bigint", ) if frappe.db.db_type == "mariadb": table_name = "information_schema.tables" - conditions = "table_type = 'sequence' and table_name = 'event_id_seq'" + conditions = f"table_type = 'sequence' and table_name = '{self._testMethodName}_id_seq'" else: table_name = "information_schema.sequences" - conditions = "sequence_name = 'event_id_seq'" + conditions = f"sequence_name = '{self._testMethodName}_id_seq'" # check if sequence table is created self.assertTrue( @@ -76,15 +72,14 @@ class TestDocType(unittest.TestCase): ) # change the autoname/naming rule back to original - d.autoname = autoname - d.naming_rule = naming_rule - d.save() + dt.autoname = autoname + dt.save() # check if name type has changed self.assertEqual( frappe.db.sql( - """select data_type FROM information_schema.columns - where column_name = 'name' and table_name = 'tabEvent'""" + f"""select data_type FROM information_schema.columns + where column_name = 'name' and table_name = 'tab{self._testMethodName}'""" )[0][0], "varchar" if frappe.db.db_type == "mariadb" else "character varying", ) diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 6cba55c425..ce7f0260b2 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -524,10 +524,16 @@ class TestDDLCommandsMaria(unittest.TestCase): ) def test_change_type(self) -> None: + def get_table_description(): + return frappe.db.sql(f"DESC `tab{self.test_table_name}`") + + # try changing from int to varchar frappe.db.change_column_type("TestNotes", "id", "varchar(255)") - test_table_description = frappe.db.sql(f"DESC tab{self.test_table_name};") - self.assertGreater(len(test_table_description), 0) - self.assertIn("varchar(255)", test_table_description[0]) + self.assertIn("varchar(255)", get_table_description()[0]) + + # try changing from varchar to bigint + frappe.db.change_column_type("TestNotes", "id", "bigint") + self.assertIn("bigint(20)", get_table_description()[0]) def test_add_index(self) -> None: index_name = "test_index" @@ -736,21 +742,34 @@ class TestDDLCommandsPost(unittest.TestCase): self.assertEqual([("id",), ("content",)], frappe.db.describe(self.test_table_name)) def test_change_type(self) -> None: + from psycopg2.errors import DatatypeMismatch + + def get_table_description(): + return frappe.db.sql( + f""" + SELECT + table_name, + column_name, + data_type + FROM + information_schema.columns + WHERE + table_name = 'tab{self.test_table_name}'""" + ) + + # try changing from int to varchar frappe.db.change_column_type(self.test_table_name, "id", "varchar(255)") - check_change = frappe.db.sql( - f""" - SELECT - table_name, - column_name, - data_type - FROM - information_schema.columns - WHERE - table_name = 'tab{self.test_table_name}' - """ - ) - self.assertGreater(len(check_change), 0) - self.assertIn("character varying", check_change[0]) + self.assertIn("character varying", get_table_description()[0]) + + # try changing from varchar to int + try: + frappe.db.change_column_type(self.test_table_name, "id", "bigint") + except DatatypeMismatch: + frappe.db.rollback() + + # try changing from varchar to int (using cast) + frappe.db.change_column_type(self.test_table_name, "id", "bigint", use_cast=True) + self.assertIn("bigint", get_table_description()[0]) def test_add_index(self) -> None: index_name = "test_index" @@ -765,7 +784,6 @@ class TestDDLCommandsPost(unittest.TestCase): ) self.assertEqual(len(indexs_in_table), 1) - @run_only_if(db_type_is.POSTGRES) def test_modify_query(self): from frappe.database.postgres.database import modify_query @@ -783,7 +801,6 @@ class TestDDLCommandsPost(unittest.TestCase): modify_query(query), ) - @run_only_if(db_type_is.POSTGRES) def test_modify_values(self): from frappe.database.postgres.database import modify_values From f6dac7003346f34c50aeabedd546241eb0bdc71c Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 12 May 2022 23:46:10 +0530 Subject: [PATCH 14/15] refactor(minor): convert change_name_type_and_make_sequence function to setup_autoincrement_and_sequence method * chore(Customize Form): remove autoincrement naming option from autoname description * chore: move sequence cache comment from schema.py to database.py * chore: added docstrings to some functions --- frappe/core/doctype/doctype/doctype.py | 35 ++++++++++--------- .../customize_form/customize_form.json | 4 +-- frappe/database/mariadb/database.py | 7 ++++ frappe/database/mariadb/schema.py | 7 ---- frappe/database/postgres/database.py | 4 +++ frappe/database/postgres/schema.py | 5 --- 6 files changed, 32 insertions(+), 30 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index bba47e573a..4e21f3bcb4 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -371,7 +371,7 @@ class DocType(Document): """Update database schema, make controller templates if `custom` is not set and clear cache.""" if self.get("can_change_name_type"): - change_name_type_and_make_sequence(self) + self.setup_autoincrement_and_sequence() try: frappe.db.updatedb(self.name, Meta(self)) @@ -412,6 +412,17 @@ class DocType(Document): clear_linked_doctype_cache() + def setup_autoincrement_and_sequence(self): + """Changes name type and makes sequence on change (if required)""" + + name_type = f"varchar({frappe.db.VARCHAR_LEN})" + + if self.autoname == "autoincrement": + name_type = "bigint" + frappe.db.create_sequence(self.name, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) + + change_name_column_type(self.name, name_type) + def sync_global_search(self): """If global search settings are changed, rebuild search properties for this table""" global_search_fields_before_update = [ @@ -903,9 +914,9 @@ def validate_series(dt, autoname=None, name=None): def validate_autoincrement_autoname(dt: DocType) -> bool: - """Checks if chan change to/from autoincrement autoname""" + """Checks if can doctype can change to/from autoincrement autoname""" - def get_autoname_before_save(dt) -> str: + def get_autoname_before_save(dt: DocType) -> str: if dt.name == "Customize Form": property_value = frappe.db.get_value( "Property Setter", {"doc_type": dt.doc_type, "property": "autoname"}, "value" @@ -928,10 +939,11 @@ def validate_autoincrement_autoname(dt: DocType) -> bool: and autoname_before_save != "autoincrement" or (not is_autoname_autoincrement and autoname_before_save == "autoincrement") ): - if dt.name == "Customize Form": - frappe.throw(_("Cannot change to/from autoincrement autoname in Customize Form")) if frappe.get_meta(dt.name).issingle: + if dt.name == "Customize Form": + frappe.throw(_("Cannot change to/from autoincrement autoname in Customize Form")) + return False if not frappe.get_all(dt.name, limit=1): @@ -945,18 +957,9 @@ def validate_autoincrement_autoname(dt: DocType) -> bool: return False -def change_name_type_and_make_sequence(dt: DocType) -> None: - doctype_name = dt.doc_type if dt.doctype == "Customize Form" else dt.name - name_type = f"varchar({frappe.db.VARCHAR_LEN})" - if dt.autoname == "autoincrement": - name_type = "bigint" - frappe.db.create_sequence(doctype_name, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) - - change_name_column_type(doctype_name, name_type) - - def change_name_column_type(doctype_name: str, type: str) -> None: - # postgres requires cast when converting from varchar to bigint + """Changes name column type""" + args = ( (doctype_name, "name", type, False, True) if (frappe.db.db_type == "postgres") diff --git a/frappe/custom/doctype/customize_form/customize_form.json b/frappe/custom/doctype/customize_form/customize_form.json index a0bc994c45..4c40b80f53 100644 --- a/frappe/custom/doctype/customize_form/customize_form.json +++ b/frappe/custom/doctype/customize_form/customize_form.json @@ -287,7 +287,7 @@ "label": "Naming" }, { - "description": "Naming Options:\n
  1. field:[fieldname] - By Field
    2. \n
  2. autoincrement - Uses Databases' Auto Increment feature
  3. naming_series: - By Naming Series (field called naming_series must be present
  4. Prompt - Prompt user for a name
  5. [series] - Series by prefix (separated by a dot); for example PRE.#####
    6. \n
  6. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", + "description": "Naming Options:\n
  1. field:[fieldname] - By Field
  2. naming_series: - By Naming Series (field called naming_series must be present)
  3. Prompt - Prompt user for a name
  4. [series] - Series by prefix (separated by a dot); for example PRE.#####
    5. \n
  5. format:EXAMPLE-{MM}morewords{fieldname1}-{fieldname2}-{#####} - Replace all braced words (fieldnames, date words (DD, MM, YY), series) with their value. Outside braces, any characters can be used.
", "fieldname": "autoname", "fieldtype": "Data", "label": "Auto Name" @@ -319,7 +319,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2022-04-21 15:36:16.772277", + "modified": "2022-05-12 15:36:16.772277", "modified_by": "Administrator", "module": "Custom", "name": "Customize Form", diff --git a/frappe/database/mariadb/database.py b/frappe/database/mariadb/database.py index ee8a5ecdfd..7505ef3a7f 100644 --- a/frappe/database/mariadb/database.py +++ b/frappe/database/mariadb/database.py @@ -19,6 +19,13 @@ class MariaDBDatabase(Database): DataError = pymysql.err.DataError REGEX_CHARACTER = "regexp" + # NOTE: using a very small cache - as during backup, if the sequence was used in anyform, + # it drops the cache and uses the next non cached value in setval query and + # puts that in the backup file, which will start the counter + # from that value when inserting any new record in the doctype. + # By default the cache is 1000 which will mess up the sequence when + # using the system after a restore. + # issue link: https://jira.mariadb.org/browse/MDEV-21786 SEQUENCE_CACHE = 50 def setup_type_map(self): diff --git a/frappe/database/mariadb/schema.py b/frappe/database/mariadb/schema.py index 87ba905ce2..f402b4ec74 100644 --- a/frappe/database/mariadb/schema.py +++ b/frappe/database/mariadb/schema.py @@ -40,13 +40,6 @@ class MariaDBTable(DBTable): not self.meta.issingle and self.meta.autoname == "autoincrement" ) or self.doctype in log_types: - # NOTE: using a very small cache - as during backup, if the sequence was used in anyform, - # it drops the cache and uses the next non cached value in setval func and - # puts that in the backup file, which will start the counter - # from that value when inserting any new record in the doctype. - # By default the cache is 1000 which will mess up the sequence when - # using the system after a restore. - # issue link: https://jira.mariadb.org/browse/MDEV-21786 frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) # NOTE: not used nextval func as default as the ability to restore diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index 3334f0ecd8..8bd4113823 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -31,6 +31,10 @@ class PostgresDatabase(Database): InterfaceError = psycopg2.InterfaceError REGEX_CHARACTER = "~" + # NOTE; The sequence cache for postgres is per connection. + # Since we're opening and closing connections for every transaction this results in skipping the cache + # to the next non-cached value hence not using cache in postgres. + # ref: https://stackoverflow.com/questions/21356375/postgres-9-0-4-sequence-skipping-numbers SEQUENCE_CACHE = 0 def setup_type_map(self): diff --git a/frappe/database/postgres/schema.py b/frappe/database/postgres/schema.py index 0777ed6cc0..ef7ba33e12 100644 --- a/frappe/database/postgres/schema.py +++ b/frappe/database/postgres/schema.py @@ -34,12 +34,7 @@ class PostgresTable(DBTable): not self.meta.issingle and self.meta.autoname == "autoincrement" ) or self.doctype in log_types: - # The sequence cache is per connection. - # Since we're opening and closing connections for every transaction this results in skipping the cache - # to the next non-cached value hence not using cache in postgres. - # ref: https://stackoverflow.com/questions/21356375/postgres-9-0-4-sequence-skipping-numbers frappe.db.create_sequence(self.doctype, check_not_exists=True, cache=frappe.db.SEQUENCE_CACHE) - name_column = "name bigint primary key" # TODO: set docstatus length From f14fb2326dc97ac2b143a20589b77aa19a70f3e3 Mon Sep 17 00:00:00 2001 From: phot0n Date: Fri, 13 May 2022 00:09:51 +0530 Subject: [PATCH 15/15] test(customize form): rename test_autoincrement_autoname to test_change_to_autoincrement_autoname --- frappe/custom/doctype/customize_form/test_customize_form.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/custom/doctype/customize_form/test_customize_form.py b/frappe/custom/doctype/customize_form/test_customize_form.py index 4712f272b5..b00f45f5d2 100644 --- a/frappe/custom/doctype/customize_form/test_customize_form.py +++ b/frappe/custom/doctype/customize_form/test_customize_form.py @@ -397,7 +397,7 @@ class TestCustomizeForm(unittest.TestCase): d.run_method("save_customization") self.assertEqual(d.label, "") - def test_autoincrement_autoname(self): + def test_change_to_autoincrement_autoname(self): d = self.get_customize_form("Event") d.autoname = "autoincrement"