diff --git a/.eslintrc b/.eslintrc index 937f11586c..adc4aebb28 100644 --- a/.eslintrc +++ b/.eslintrc @@ -5,7 +5,7 @@ "es6": true }, "parserOptions": { - "ecmaVersion": 9, + "ecmaVersion": 11, "sourceType": "module" }, "extends": "eslint:recommended", diff --git a/.github/helper/install_dependencies.sh b/.github/helper/install_dependencies.sh index d16f5b62ad..f0e8016860 100644 --- a/.github/helper/install_dependencies.sh +++ b/.github/helper/install_dependencies.sh @@ -2,6 +2,13 @@ set -e +# Check for merge conflicts before proceeding +python -m compileall -f "${GITHUB_WORKSPACE}" +if grep -lr --exclude-dir=node_modules "^<<<<<<< " "${GITHUB_WORKSPACE}" + then echo "Found merge conflicts" + exit 1 +fi + # install wkhtmltopdf wget -O /tmp/wkhtmltox.tar.xz https://github.com/frappe/wkhtmltopdf/raw/master/wkhtmltox-0.12.3_linux-generic-amd64.tar.xz tar -xf /tmp/wkhtmltox.tar.xz -C /tmp diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a6c1243f64..9c7ecf989e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,7 +22,6 @@ jobs: npm install @semantic-release/git @semantic-release/exec --no-save - name: Create Release env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_TOKEN: ${{ secrets.RELEASE_TOKEN }} GITHUB_TOKEN: ${{ secrets.RELEASE_TOKEN }} GIT_AUTHOR_NAME: "Frappe PR Bot" diff --git a/frappe/__init__.py b/frappe/__init__.py index 07f75ecd31..d54686304f 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -199,6 +199,7 @@ def init(site, sites_path=None, new_site=False): } ) local.rollback_observers = [] + local.locked_documents = [] local.before_commit = [] local.test_objects = {} @@ -1507,10 +1508,11 @@ def get_newargs(fn, kwargs): if hasattr(fn, "fnargs"): fnargs = fn.fnargs else: - fullargspec = inspect.getfullargspec(fn) - fnargs = fullargspec.args - fnargs.extend(fullargspec.kwonlyargs) - varkw = fullargspec.varkw + signature = inspect.signature(fn) + fnargs = list(signature.parameters) + varkw = "kwargs" in fnargs + if varkw: + fnargs.pop(-1) newargs = {} for a in kwargs: @@ -2263,7 +2265,4 @@ def mock(type, size=1, locale="en"): return squashify(results) -def validate_and_sanitize_search_inputs(fn): - from frappe.desk.search import validate_and_sanitize_search_inputs as func - - return func(fn) +from frappe.desk.search import validate_and_sanitize_search_inputs # noqa diff --git a/frappe/core/doctype/server_script/server_script.json b/frappe/core/doctype/server_script/server_script.json index 548d21bb60..9312ae178b 100644 --- a/frappe/core/doctype/server_script/server_script.json +++ b/frappe/core/doctype/server_script/server_script.json @@ -49,7 +49,7 @@ "fieldname": "doctype_event", "fieldtype": "Select", "label": "DocType Event", - "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Insert\nAfter Save\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)" + "options": "Before Insert\nBefore Validate\nBefore Save\nAfter Insert\nAfter Save\nBefore Submit\nAfter Submit\nBefore Cancel\nAfter Cancel\nBefore Delete\nAfter Delete\nBefore Save (Submitted Document)\nAfter Save (Submitted Document)\nOn Payment Authorization" }, { "depends_on": "eval:doc.script_type==='API'", @@ -109,7 +109,7 @@ "link_fieldname": "server_script" } ], - "modified": "2022-04-07 19:41:23.178772", + "modified": "2022-04-27 11:42:52.032963", "modified_by": "Administrator", "module": "Core", "name": "Server Script", diff --git a/frappe/core/doctype/server_script/server_script_utils.py b/frappe/core/doctype/server_script/server_script_utils.py index 5300baa199..b807b43d10 100644 --- a/frappe/core/doctype/server_script/server_script_utils.py +++ b/frappe/core/doctype/server_script/server_script_utils.py @@ -17,6 +17,7 @@ EVENT_MAP = { "after_delete": "After Delete", "before_update_after_submit": "Before Save (Submitted Document)", "on_update_after_submit": "After Save (Submitted Document)", + "on_payment_authorized": "On Payment Authorization", } diff --git a/frappe/database/postgres/framework_postgres.sql b/frappe/database/postgres/framework_postgres.sql index 1e79bf67d8..2cae3ab82f 100644 --- a/frappe/database/postgres/framework_postgres.sql +++ b/frappe/database/postgres/framework_postgres.sql @@ -240,7 +240,7 @@ CREATE TABLE "tabDocType" ( DROP TABLE IF EXISTS "tabSeries"; CREATE TABLE "tabSeries" ( - "name" varchar(100) DEFAULT NULL, + "name" varchar(100), "current" bigint NOT NULL DEFAULT 0, PRIMARY KEY ("name") ) ; diff --git a/frappe/database/query.py b/frappe/database/query.py index 8d8a767370..136f5c86b6 100644 --- a/frappe/database/query.py +++ b/frappe/database/query.py @@ -108,11 +108,14 @@ def change_orderby(order: str): tuple: field, order """ order = order.split() - if order[1].lower() == "asc": - orderby, order = order[0], Order.asc - return orderby, order - orderby, order = order[0], Order.desc - return orderby, order + + try: + if order[1].lower() == "asc": + return order[0], Order.asc + except IndexError: + pass + + return order[0], Order.desc OPERATOR_MAP = { @@ -175,10 +178,13 @@ class Query: """ if kwargs.get("orderby"): orderby = kwargs.get("orderby") - order = kwargs.get("order") if kwargs.get("order") else Order.desc if isinstance(orderby, str) and len(orderby.split()) > 1: - orderby, order = change_orderby(orderby) - conditions = conditions.orderby(orderby, order=order) + for ordby in orderby.split(","): + if ordby := ordby.strip(): + orderby, order = change_orderby(ordby) + conditions = conditions.orderby(orderby, order=order) + else: + conditions = conditions.orderby(orderby, order=kwargs.get("order") or Order.desc) if kwargs.get("limit"): conditions = conditions.limit(kwargs.get("limit")) @@ -288,7 +294,7 @@ class Query: table: str, fields: Union[List, Tuple], filters: Union[Dict[str, Union[str, int]], str, int] = None, - **kwargs + **kwargs, ): criterion = self.build_conditions(table, filters, **kwargs) if isinstance(fields, (list, tuple)): diff --git a/frappe/desk/search.py b/frappe/desk/search.py index ba4c5fb4fb..eb1a2e82ba 100644 --- a/frappe/desk/search.py +++ b/frappe/desk/search.py @@ -1,12 +1,10 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import functools import json import re -import wrapt - -# Search import frappe from frappe import _, is_whitelisted from frappe.permissions import has_permission @@ -314,17 +312,20 @@ def relevance_sorter(key, query, as_dict): return (cstr(value).lower().startswith(query.lower()) is not True, value) -@wrapt.decorator -def validate_and_sanitize_search_inputs(fn, instance, args, kwargs): - kwargs.update(dict(zip(fn.__code__.co_varnames, args))) - sanitize_searchfield(kwargs["searchfield"]) - kwargs["start"] = cint(kwargs["start"]) - kwargs["page_len"] = cint(kwargs["page_len"]) +def validate_and_sanitize_search_inputs(fn): + @functools.wraps(fn) + def wrapper(*args, **kwargs): + kwargs.update(dict(zip(fn.__code__.co_varnames, args))) + sanitize_searchfield(kwargs["searchfield"]) + kwargs["start"] = cint(kwargs["start"]) + kwargs["page_len"] = cint(kwargs["page_len"]) - if kwargs["doctype"] and not frappe.db.exists("DocType", kwargs["doctype"]): - return [] + if kwargs["doctype"] and not frappe.db.exists("DocType", kwargs["doctype"]): + return [] - return fn(**kwargs) + return fn(**kwargs) + + return wrapper @frappe.whitelist() diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 6aa881ed5c..b04ad4db40 100644 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -124,7 +124,7 @@ class Newsletter(WebsiteGenerator): ) def get_success_recipients(self) -> List[str]: - """Recipients who have already recieved the newsletter. + """Recipients who have already received the newsletter. Couldn't think of a better name ;) """ @@ -132,7 +132,7 @@ class Newsletter(WebsiteGenerator): "Email Queue Recipient", filters={ "status": ("in", ["Not Sent", "Sending", "Sent"]), - "parentfield": ("in", self.get_linked_email_queue()), + "parent": ("in", self.get_linked_email_queue()), }, pluck="recipient", ) diff --git a/frappe/email/doctype/newsletter/test_newsletter.py b/frappe/email/doctype/newsletter/test_newsletter.py index c62b7e84aa..81702f3a09 100644 --- a/frappe/email/doctype/newsletter/test_newsletter.py +++ b/frappe/email/doctype/newsletter/test_newsletter.py @@ -221,3 +221,24 @@ class TestNewsletter(TestNewsletterMixin, unittest.TestCase): newsletter.reload() self.assertEqual(newsletter.email_sent, 0) + + def test_retry_partially_sent_newsletter(self): + frappe.db.delete("Email Queue") + frappe.db.delete("Email Queue Recipient") + frappe.db.delete("Newsletter") + + newsletter = self.get_newsletter() + newsletter.send_emails() + email_queue_list = [frappe.get_doc("Email Queue", e.name) for e in frappe.get_all("Email Queue")] + self.assertEqual(len(email_queue_list), 4) + + # emulate partial send + email_queue_list[0].status = "Error" + email_queue_list[0].recipients[0].status = "Error" + email_queue_list[0].save() + newsletter.email_sent = False + + # retry + newsletter.send_emails() + email_queue_list = [frappe.get_doc("Email Queue", e.name) for e in frappe.get_all("Email Queue")] + self.assertEqual(len(email_queue_list), 5) diff --git a/frappe/geo/country_info.json b/frappe/geo/country_info.json index 23cadc2156..c1031fe211 100644 --- a/frappe/geo/country_info.json +++ b/frappe/geo/country_info.json @@ -954,6 +954,8 @@ "smallest_currency_fraction_value": 0.01, "currency_symbol": "\u20ac", "number_format": "#.###,##", + "date_format": "dd.mm.yyyy", + "time_format": "HH:mm", "timezones": [ "Europe/Berlin" ] diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 93446fb99e..186ef52c12 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -241,7 +241,6 @@ class BaseDocument(object): raise AttributeError(key) value = get_controller(value["doctype"])(value) - value.init_valid_columns() value.parent = self.name value.parenttype = self.doctype @@ -350,7 +349,7 @@ class BaseDocument(object): @property def docstatus(self): - return DocStatus(self.get("docstatus")) + return DocStatus(cint(self.get("docstatus"))) @docstatus.setter def docstatus(self, value): diff --git a/frappe/model/document.py b/frappe/model/document.py index 67e1de0932..c5e61563f8 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -989,6 +989,16 @@ class Document(BaseDocument): self.docstatus = DocStatus.cancelled() return self.save() + @whitelist.__func__ + def _rename( + self, name: str, merge: bool = False, force: bool = False, validate_rename: bool = True + ): + """Rename the document. Triggers frappe.rename_doc, then reloads.""" + from frappe.model.rename_doc import rename_doc + + self.name = rename_doc(doc=self, new=name, merge=merge, force=force, validate=validate_rename) + self.reload() + @whitelist.__func__ def submit(self): """Submit the document. Sets `docstatus` = 1, then saves.""" @@ -999,6 +1009,13 @@ class Document(BaseDocument): """Cancel the document. Sets `docstatus` = 2, then saves.""" return self._cancel() + @whitelist.__func__ + def rename( + self, name: str, merge: bool = False, force: bool = False, validate_rename: bool = True + ): + """Rename the document to `name`. This transforms the current object.""" + return self._rename(name=name, merge=merge, force=force, validate_rename=validate_rename) + def delete(self, ignore_permissions=False): """Delete document.""" frappe.delete_doc( @@ -1398,21 +1415,22 @@ class Document(BaseDocument): # See: Stock Reconciliation from frappe.utils.background_jobs import enqueue - if hasattr(self, "_" + action): - action = "_" + action + if hasattr(self, f"_{action}"): + action = f"_{action}" - if file_lock.lock_exists(self.get_signature()): + try: + self.lock() + except frappe.DocumentLockedError: frappe.throw( _("This document is currently queued for execution. Please try again"), title=_("Document Queued"), ) - self.lock() - enqueue( + return enqueue( "frappe.model.document.execute_action", - doctype=self.doctype, - name=self.name, - action=action, + __doctype=self.doctype, + __name=self.name, + __action=action, **kwargs, ) @@ -1433,10 +1451,13 @@ class Document(BaseDocument): if lock_exists: raise frappe.DocumentLockedError file_lock.create_lock(signature) + frappe.local.locked_documents.append(self) def unlock(self): """Delete the lock file for this document""" file_lock.delete_lock(self.get_signature()) + if self in frappe.local.locked_documents: + frappe.local.locked_documents.remove(self) # validation helpers def validate_from_to_dates(self, from_date_field, to_date_field): @@ -1495,12 +1516,12 @@ class Document(BaseDocument): return f"{doctype}({name})" -def execute_action(doctype, name, action, **kwargs): +def execute_action(__doctype, __name, __action, **kwargs): """Execute an action on a document (called by background worker)""" - doc = frappe.get_doc(doctype, name) + doc = frappe.get_doc(__doctype, __name) doc.unlock() try: - getattr(doc, action)(**kwargs) + getattr(doc, __action)(**kwargs) except Exception: frappe.db.rollback() @@ -1511,4 +1532,4 @@ def execute_action(doctype, name, action, **kwargs): msg = "
" + frappe.get_traceback() + "
" doc.add_comment("Comment", _("Action Failed") + "

" + msg) - doc.notify_update() + doc.notify_update() diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index dee364ae8d..a0cd10f967 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -4,12 +4,15 @@ from typing import TYPE_CHECKING, Dict, List, Optional import frappe from frappe import _, bold +from frappe.model.document import Document from frappe.model.dynamic_links import get_dynamic_link_map from frappe.model.naming import validate_name from frappe.model.utils.user_settings import sync_user_settings, update_user_settings_data from frappe.query_builder import Field -from frappe.utils import cint +from frappe.query_builder.utils import DocType, Table +from frappe.utils.data import sbool from frappe.utils.password import rename_password +from frappe.utils.scheduler import is_scheduler_inactive if TYPE_CHECKING: from frappe.model.meta import Meta @@ -23,10 +26,19 @@ def update_document_title( title: Optional[str] = None, name: Optional[str] = None, merge: bool = False, + enqueue: bool = False, **kwargs, ) -> str: """ - Update title from header in form view + Update the name or title of a document. Returns `name` if document was renamed, + `docname` if renaming operation was queued. + + :param doctype: DocType of the document + :param docname: Name of the document + :param title: New Title of the document + :param name: New Name of the document + :param merge: Merge the current Document with the existing one if exists + :param enqueue: Enqueue the rename operation, title is updated in current process """ # to maintain backwards API compatibility @@ -38,6 +50,10 @@ def update_document_title( if not isinstance(obj, (str, type(None))): frappe.throw(f"{obj=} must be of type str or None") + # handle bad API usages + merge = sbool(merge) + enqueue = sbool(enqueue) + doc = frappe.get_doc(doctype, docname) doc.check_permission(permtype="write") @@ -49,11 +65,34 @@ def update_document_title( name_updated = updated_name and (updated_name != doc.name) if name_updated: - docname = rename_doc(doctype=doctype, old=docname, new=updated_name, merge=merge) + if enqueue and not is_scheduler_inactive(): + current_name = doc.name + + # before_name hook may have DocType specific validations or transformations + transformed_name = doc.run_method("before_rename", current_name, updated_name, merge) + if isinstance(transformed_name, dict): + transformed_name = transformed_name.get("new") + transformed_name = transformed_name or updated_name + + # run rename validations before queueing + # use savepoints to avoid partial renames / commits + validate_rename( + doctype=doctype, + old=current_name, + new=transformed_name, + meta=doc.meta, + merge=merge, + save_point=True, + ) + + doc.queue_action("rename", name=transformed_name, merge=merge) + else: + doc.rename(updated_name, merge=merge) if title_updated: try: - frappe.db.set_value(doctype, docname, title_field, updated_title) + setattr(doc, title_field, updated_title) + doc.save() frappe.msgprint(_("Saved"), alert=True, indicator="green") except Exception as e: if frappe.db.is_duplicate_entry(e): @@ -64,44 +103,64 @@ def update_document_title( ) raise - return docname + return doc.name def rename_doc( - doctype: str, - old: str, - new: str, + doctype: Optional[str] = None, + old: Optional[str] = None, + new: str = None, force: bool = False, merge: bool = False, ignore_permissions: bool = False, ignore_if_exists: bool = False, show_alert: bool = True, rebuild_search: bool = True, + doc: Optional[Document] = None, + validate: bool = True, ) -> str: - """Rename a doc(dt, old) to doc(dt, new) and update all linked fields of type "Link".""" - if not frappe.db.exists(doctype, old): - frappe.errprint(_("Failed: {0} to {1} because {0} doesn't exist.").format(old, new)) - return + """Rename a doc(dt, old) to doc(dt, new) and update all linked fields of type "Link". - if ignore_if_exists and frappe.db.exists(doctype, new): - frappe.errprint(_("Failed: {0} to {1} because {1} already exists.").format(old, new)) - return + doc: Document object to be renamed. + new: New name for the record. If None, and doctype is specified, new name may be automatically generated via before_rename hooks. + doctype: DocType of the document. Not required if doc is passed. + old: Current name of the document. Not required if doc is passed. + force: Allow even if document is not allowed to be renamed. + merge: Merge with existing document of new name. + ignore_permissions: Ignore user permissions while renaming. + ignore_if_exists: Don't raise exception if document with new name already exists. This will quietely overwrite the existing document. + show_alert: Display alert if document is renamed successfully. + rebuild_search: Rebuild linked doctype search after renaming. + validate: Validate before renaming. If False, it is assumed that the caller has already validated. + """ + old_usage_style = doctype and old and new + new_usage_style = doc and new - if old == new: - frappe.errprint( - _("Ignored: {0} to {1} no changes made because old and new name are the same.").format(old, new) + if not (new_usage_style or old_usage_style): + raise TypeError( + "{doctype, old, new} or {doc, new} are required arguments for frappe.model.rename_doc" ) - return - force = cint(force) - merge = cint(merge) + old = old or doc.name + doctype = doctype or doc.doctype + force = sbool(force) + merge = sbool(merge) meta = frappe.get_meta(doctype) - # call before_rename - old_doc = frappe.get_doc(doctype, old) - out = old_doc.run_method("before_rename", old, new, merge) or {} - new = (out.get("new") or new) if isinstance(out, dict) else (out or new) - new = validate_rename(doctype, new, meta, merge, force, ignore_permissions) + if validate: + old_doc = doc or frappe.get_doc(doctype, old) + out = old_doc.run_method("before_rename", old, new, merge) or {} + new = (out.get("new") or new) if isinstance(out, dict) else (out or new) + new = validate_rename( + doctype=doctype, + old=old, + new=new, + meta=meta, + merge=merge, + force=force, + ignore_permissions=ignore_permissions, + ignore_if_exists=ignore_if_exists, + ) if not merge: rename_parent_and_child(doctype, old, new, meta) @@ -139,11 +198,12 @@ def rename_doc( rename_password(doctype, old, new) # update user_permissions - frappe.db.sql( - """UPDATE `tabDefaultValue` SET `defvalue`=%s WHERE `parenttype`='User Permission' - AND `defkey`=%s AND `defvalue`=%s""", - (new, doctype, old), - ) + DefaultValue = DocType("DefaultValue") + frappe.qb.update(DefaultValue).set(DefaultValue.defvalue, new).where( + (DefaultValue.parenttype == "User Permission") + & (DefaultValue.defkey == doctype) + & (DefaultValue.defvalue == old) + ).run() if merge: new_doc.add_comment("Edit", _("merged {0} into {1}").format(frappe.bold(old), frappe.bold(new))) @@ -207,15 +267,13 @@ def update_user_settings(old: str, new: str, link_fields: List[Dict]) -> None: # find the user settings for the linked doctypes linked_doctypes = {d.parent for d in link_fields if not d.issingle} - user_settings_details = frappe.db.sql( - """SELECT `user`, `doctype`, `data` - FROM `__UserSettings` - WHERE `data` like %s - AND `doctype` IN ('{doctypes}')""".format( - doctypes="', '".join(linked_doctypes) - ), - (old), - as_dict=1, + UserSettings = Table("__UserSettings") + + user_settings_details = ( + frappe.qb.from_(UserSettings) + .select("user", "doctype", "data") + .where(UserSettings.data.like(old) & UserSettings.doctype.isin(linked_doctypes)) + .run(as_dict=True) ) # create the dict using the doctype name as key and values as list of the user settings @@ -240,37 +298,33 @@ def update_customizations(old: str, new: str) -> None: def update_attachments(doctype: str, old: str, new: str) -> None: - try: - if old != "File Data" and doctype != "DocType": - frappe.db.sql( - """update `tabFile` set attached_to_name=%s - where attached_to_name=%s and attached_to_doctype=%s""", - (new, old, doctype), - ) - except frappe.db.ProgrammingError as e: - if not frappe.db.is_column_missing(e): - raise + if doctype != "DocType": + File = DocType("File") + + frappe.qb.update(File).set(File.attached_to_name, new).where( + (File.attached_to_name == old) & (File.attached_to_doctype == doctype) + ).run() def rename_versions(doctype: str, old: str, new: str) -> None: - frappe.db.sql( - """UPDATE `tabVersion` SET `docname`=%s WHERE `ref_doctype`=%s AND `docname`=%s""", - (new, doctype, old), - ) + Version = DocType("Version") + + frappe.qb.update(Version).set(Version.docname, new).where( + (Version.docname == old) & (Version.ref_doctype == doctype) + ).run() def rename_eps_records(doctype: str, old: str, new: str) -> None: - epl = frappe.qb.DocType("Energy Point Log") - ( - frappe.qb.update(epl) - .set(epl.reference_name, new) - .where((epl.reference_doctype == doctype) & (epl.reference_name == old)) + EPL = DocType("Energy Point Log") + + frappe.qb.update(EPL).set(EPL.reference_name, new).where( + (EPL.reference_doctype == doctype) & (EPL.reference_name == old) ).run() def rename_parent_and_child(doctype: str, old: str, new: str, meta: "Meta") -> None: - # rename the doc - frappe.db.sql("UPDATE `tab{0}` SET `name`={1} WHERE `name`={1}".format(doctype, "%s"), (new, old)) + frappe.qb.update(doctype).set("name", new).where(Field("name") == old).run() + update_autoname_field(doctype, new, meta) update_child_docs(old, new, meta) @@ -280,20 +334,36 @@ def update_autoname_field(doctype: str, new: str, meta: "Meta") -> None: if meta.get("autoname"): field = meta.get("autoname").split(":") if field and field[0] == "field": - frappe.db.sql( - "UPDATE `tab{0}` SET `{1}`={2} WHERE `name`={2}".format(doctype, field[1], "%s"), (new, new) - ) + frappe.qb.update(doctype).set(field[1], new).where(Field("name") == new).run() def validate_rename( - doctype: str, new: str, meta: "Meta", merge: bool, force: bool, ignore_permissions: bool + doctype: str, + old: str, + new: str, + meta: "Meta", + merge: bool, + force: bool = False, + ignore_permissions: bool = False, + ignore_if_exists: bool = False, + save_point=False, ) -> str: # using for update so that it gets locked and someone else cannot edit it while this rename is going on! + if save_point: + _SAVE_POINT = f"validate_rename_{frappe.generate_hash(length=8)}" + frappe.db.savepoint(_SAVE_POINT) + exists = ( frappe.qb.from_(doctype).where(Field("name") == new).for_update().select("name").run(pluck=True) ) exists = exists[0] if exists else None + if not frappe.db.exists(doctype, old): + frappe.throw(_("Can't rename {0} to {1} because {0} doesn't exist.").format(old, new)) + + if old == new: + frappe.throw(_("No changes made because old and new name are the same.").format(old, new)) + if merge and not exists: frappe.throw(_("{0} {1} does not exist, select a new target to merge").format(doctype, new)) @@ -301,7 +371,7 @@ def validate_rename( # for fixing case, accents exists = None - if (not merge) and exists: + if not merge and exists and not ignore_if_exists: frappe.throw(_("Another {0} with name {1} exists, select another name").format(doctype, new)) if not ( @@ -315,6 +385,9 @@ def validate_rename( # validate naming like it's done in doc.py new = validate_name(doctype, new) + if save_point: + frappe.db.rollback(save_point=_SAVE_POINT) + return new @@ -337,9 +410,7 @@ def rename_doctype(doctype: str, old: str, new: str) -> None: def update_child_docs(old: str, new: str, meta: "Meta") -> None: # update "parent" for df in meta.get_table_fields(): - frappe.db.sql( - "update `tab%s` set parent=%s where parent=%s" % (df.options, "%s", "%s"), (new, old) - ) + frappe.qb.update(df.options).set("parent", new).where(Field("parent") == old).run() def update_link_field_values(link_fields: List[Dict], old: str, new: str, doctype: str) -> None: @@ -384,57 +455,46 @@ def get_link_fields(doctype: str) -> List[Dict]: frappe.flags.link_fields = {} if doctype not in frappe.flags.link_fields: - link_fields = frappe.db.sql( - """\ - select parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.parent) as issingle - from tabDocField df - where - df.options=%s and df.fieldtype='Link'""", - (doctype,), - as_dict=1, + dt = DocType("DocType") + df = DocType("DocField") + cf = DocType("Custom Field") + ps = DocType("Property Setter") + + st_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == df.parent).as_("issingle") + standard_fields = ( + frappe.qb.from_(df) + .select(df.parent, df.fieldname, st_issingle) + .where((df.options == doctype) & (df.fieldtype == "Link")) + .run(as_dict=True) ) - # get link fields from tabCustom Field - custom_link_fields = frappe.db.sql( - """\ - select dt as parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.dt) as issingle - from `tabCustom Field` df - where - df.options=%s and df.fieldtype='Link'""", - (doctype,), - as_dict=1, + cf_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == cf.dt).as_("issingle") + custom_fields = ( + frappe.qb.from_(cf) + .select(cf.dt.as_("parent"), cf.fieldname, cf_issingle) + .where((cf.options == doctype) & (cf.fieldtype == "Link")) + .run(as_dict=True) ) - # add custom link fields list to link fields list - link_fields += custom_link_fields - - # remove fields whose options have been changed using property setter - property_setter_link_fields = frappe.db.sql( - """\ - select ps.doc_type as parent, ps.field_name as fieldname, - (select issingle from tabDocType dt - where dt.name = ps.doc_type) as issingle - from `tabProperty Setter` ps - where - ps.property_type='options' and - ps.field_name is not null and - ps.value=%s""", - (doctype,), - as_dict=1, + ps_issingle = ( + frappe.qb.from_(dt).select(dt.issingle).where(dt.name == ps.doc_type).as_("issingle") + ) + property_setter_fields = ( + frappe.qb.from_(ps) + .select(ps.doc_type.as_("parent"), ps.field_name.as_("fieldname"), ps_issingle) + .where((ps.property == "options") & (ps.value == doctype) & (ps.field_name.notnull())) + .run(as_dict=True) ) - link_fields += property_setter_link_fields - - frappe.flags.link_fields[doctype] = link_fields + frappe.flags.link_fields[doctype] = standard_fields + custom_fields + property_setter_fields return frappe.flags.link_fields[doctype] def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: + CustomField = DocType("Custom Field") + PropertySetter = DocType("Property Setter") + if frappe.conf.developer_mode: for name in frappe.get_all("DocField", filters={"options": old}, pluck="parent"): doctype = frappe.get_doc("DocType", name) @@ -446,23 +506,18 @@ def update_options_for_fieldtype(fieldtype: str, old: str, new: str) -> None: if save: doctype.save() else: - frappe.db.sql( - """update `tabDocField` set options=%s - where fieldtype=%s and options=%s""", - (new, fieldtype, old), - ) + DocField = DocType("DocField") + frappe.qb.update(DocField).set(DocField.options, new).where( + (DocField.fieldtype == fieldtype) & (DocField.options == old) + ).run() - frappe.db.sql( - """update `tabCustom Field` set options=%s - where fieldtype=%s and options=%s""", - (new, fieldtype, old), - ) + frappe.qb.update(CustomField).set(CustomField.options, new).where( + (CustomField.fieldtype == fieldtype) & (CustomField.options == old) + ).run() - frappe.db.sql( - """update `tabProperty Setter` set value=%s - where property='options' and value=%s""", - (new, old), - ) + frappe.qb.update(PropertySetter).set(PropertySetter.value, new).where( + (PropertySetter.property == "options") & (PropertySetter.value == old) + ).run() def get_select_fields(old: str, new: str) -> List[Dict]: @@ -470,108 +525,87 @@ def get_select_fields(old: str, new: str) -> List[Dict]: get select type fields where doctype's name is hardcoded as new line separated list """ + df = DocType("DocField") + dt = DocType("DocType") + cf = DocType("Custom Field") + ps = DocType("Property Setter") + # get link fields from tabDocField - select_fields = frappe.db.sql( - """ - select parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.parent) as issingle - from tabDocField df - where - df.parent != %s and df.fieldtype = 'Select' and - df.options like {0} """.format( - frappe.db.escape("%" + old + "%") - ), - (new,), - as_dict=1, + st_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == df.parent).as_("issingle") + standard_fields = ( + frappe.qb.from_(df) + .select(df.parent, df.fieldname, st_issingle) + .where((df.parent != new) & (df.fieldtype == "Select") & (df.options.like(f"%{old}%"))) + .run(as_dict=True) ) # get link fields from tabCustom Field - custom_select_fields = frappe.db.sql( - """ - select dt as parent, fieldname, - (select issingle from tabDocType dt - where dt.name = df.dt) as issingle - from `tabCustom Field` df - where - df.dt != %s and df.fieldtype = 'Select' and - df.options like {0} """.format( - frappe.db.escape("%" + old + "%") - ), - (new,), - as_dict=1, + cf_issingle = frappe.qb.from_(dt).select(dt.issingle).where(dt.name == cf.dt).as_("issingle") + custom_select_fields = ( + frappe.qb.from_(cf) + .select(cf.dt.as_("parent"), cf.fieldname, cf_issingle) + .where((cf.dt != new) & (cf.fieldtype == "Select") & (cf.options.like(f"%{old}%"))) + .run(as_dict=True) ) - # add custom link fields list to link fields list - select_fields += custom_select_fields - # remove fields whose options have been changed using property setter - property_setter_select_fields = frappe.db.sql( - """ - select ps.doc_type as parent, ps.field_name as fieldname, - (select issingle from tabDocType dt - where dt.name = ps.doc_type) as issingle - from `tabProperty Setter` ps - where - ps.doc_type != %s and - ps.property_type='options' and - ps.field_name is not null and - ps.value like {0} """.format( - frappe.db.escape("%" + old + "%") - ), - (new,), - as_dict=1, + ps_issingle = ( + frappe.qb.from_(dt).select(dt.issingle).where(dt.name == ps.doc_type).as_("issingle") + ) + property_setter_select_fields = ( + frappe.qb.from_(ps) + .select(ps.doc_type.as_("parent"), ps.field_name.as_("fieldname"), ps_issingle) + .where( + (ps.doc_type != new) + & (ps.property == "options") + & (ps.field_name.notnull()) + & (ps.value.like(f"%{old}%")) + ) + .run(as_dict=True) ) - select_fields += property_setter_select_fields - - return select_fields + return standard_fields + custom_select_fields + property_setter_select_fields def update_select_field_values(old: str, new: str): - frappe.db.sql( - """ - update `tabDocField` set options=replace(options, %s, %s) - where - parent != %s and fieldtype = 'Select' and - (options like {0} or options like {1})""".format( - frappe.db.escape("%" + "\n" + old + "%"), frappe.db.escape("%" + old + "\n" + "%") - ), - (old, new, new), - ) + from frappe.query_builder.functions import Replace - frappe.db.sql( - """ - update `tabCustom Field` set options=replace(options, %s, %s) - where - dt != %s and fieldtype = 'Select' and - (options like {0} or options like {1})""".format( - frappe.db.escape("%" + "\n" + old + "%"), frappe.db.escape("%" + old + "\n" + "%") - ), - (old, new, new), - ) + DocField = DocType("DocField") + CustomField = DocType("Custom Field") + PropertySetter = DocType("Property Setter") - frappe.db.sql( - """ - update `tabProperty Setter` set value=replace(value, %s, %s) - where - doc_type != %s and field_name is not null and - property='options' and - (value like {0} or value like {1})""".format( - frappe.db.escape("%" + "\n" + old + "%"), frappe.db.escape("%" + old + "\n" + "%") - ), - (old, new, new), - ) + frappe.qb.update(DocField).set(DocField.options, Replace(DocField.options, old, new)).where( + (DocField.fieldtype == "Select") + & (DocField.parent != new) + & (DocField.options.like(f"%\n{old}%") | DocField.options.like(f"%{old}\n%")) + ).run() + + frappe.qb.update(CustomField).set( + CustomField.options, Replace(CustomField.options, old, new) + ).where( + (CustomField.fieldtype == "Select") + & (CustomField.dt != new) + & (CustomField.options.like(f"%\n{old}%") | CustomField.options.like(f"%{old}\n%")) + ).run() + + frappe.qb.update(PropertySetter).set( + PropertySetter.value, Replace(PropertySetter.value, old, new) + ).where( + (PropertySetter.property == "options") + & (PropertySetter.field_name.notnull()) + & (PropertySetter.doc_type != new) + & (PropertySetter.value.like(f"%\n{old}%") | PropertySetter.value.like(f"%{old}\n%")) + ).run() def update_parenttype_values(old: str, new: str): - child_doctypes = frappe.db.get_all( + child_doctypes = frappe.get_all( "DocField", fields=["options", "fieldname"], filters={"parent": new, "fieldtype": ["in", frappe.model.table_fields]}, ) - custom_child_doctypes = frappe.db.get_all( + custom_child_doctypes = frappe.get_all( "Custom Field", fields=["options", "fieldname"], filters={"dt": new, "fieldtype": ["in", frappe.model.table_fields]}, @@ -586,35 +620,30 @@ def update_parenttype_values(old: str, new: str): pluck="value", ) - child_doctypes = list(d["options"] for d in child_doctypes) - child_doctypes += property_setter_child_doctypes + child_doctypes = set(list(d["options"] for d in child_doctypes) + property_setter_child_doctypes) for doctype in child_doctypes: - frappe.db.sql(f"update `tab{doctype}` set parenttype=%s where parenttype=%s", (new, old)) + Table = DocType(doctype) + frappe.qb.update(Table).set(Table.parenttype, new).where(Table.parenttype == old).run() def rename_dynamic_links(doctype: str, old: str, new: str): + Singles = DocType("Singles") for df in get_dynamic_link_map().get(doctype, []): # dynamic link in single, just one value to check if frappe.get_meta(df.parent).issingle: refdoc = frappe.db.get_singles_dict(df.parent) if refdoc.get(df.options) == doctype and refdoc.get(df.fieldname) == old: - - frappe.db.sql( - """update tabSingles set value=%s where - field=%s and value=%s and doctype=%s""", - (new, df.fieldname, old, df.parent), - ) + frappe.qb.update(Singles).set(Singles.value, new).where( + (Singles.field == df.fieldname) & (Singles.doctype == df.parent) & (Singles.value == old) + ).run() else: # because the table hasn't been renamed yet! parent = df.parent if df.parent != new else old - frappe.db.sql( - """update `tab{parent}` set {fieldname}=%s - where {options}=%s and {fieldname}=%s""".format( - parent=parent, fieldname=df.fieldname, options=df.options - ), - (new, doctype, old), - ) + + frappe.qb.update(parent).set(df.fieldname, new).where( + (Field(df.options) == doctype) & (Field(df.fieldname) == old) + ).run() def bulk_rename( diff --git a/frappe/parallel_test_runner.py b/frappe/parallel_test_runner.py index 65c8eb470b..4fd03773ef 100644 --- a/frappe/parallel_test_runner.py +++ b/frappe/parallel_test_runner.py @@ -46,7 +46,7 @@ class ParallelTestRunner: if hasattr(test_module, "global_test_dependencies"): for doctype in test_module.global_test_dependencies: - make_test_records(doctype) + make_test_records(doctype, commit=True) elapsed = time.time() - start_time elapsed = click.style(f" ({elapsed:.03}s)", fg="red") @@ -76,7 +76,7 @@ class ParallelTestRunner: def create_test_dependency_records(self, module, path, filename): if hasattr(module, "test_dependencies"): for doctype in module.test_dependencies: - make_test_records(doctype) + make_test_records(doctype, commit=True) if os.path.basename(os.path.dirname(path)) == "doctype": # test_data_migration_connector.py > data_migration_connector.json @@ -86,7 +86,7 @@ class ParallelTestRunner: with open(test_record_file_path, "r") as f: doc = json.loads(f.read()) doctype = doc["name"] - make_test_records(doctype) + make_test_records(doctype, commit=True) def get_module(self, path, filename): app_path = frappe.get_pymodule_path(self.app) diff --git a/frappe/public/js/frappe/form/form.js b/frappe/public/js/frappe/form/form.js index 0c8939cf5d..c56ffc592d 100644 --- a/frappe/public/js/frappe/form/form.js +++ b/frappe/public/js/frappe/form/form.js @@ -179,7 +179,7 @@ frappe.ui.form.Form = class FrappeForm { grid_shortcut_keys.forEach(row => { frappe.ui.keys.add_shortcut({ shortcut: row.shortcut, - page: this, + page: this.page, description: __(row.description), ignore_inputs: true, condition: () => !this.is_new() @@ -1765,12 +1765,15 @@ frappe.ui.form.Form = class FrappeForm { // scroll to input frappe.utils.scroll_to($el, true, 15); - // highlight input - $el.addClass('has-error'); + // focus if text field + $el.find('input, select, textarea').focus(); + + // highlight control inside field + let control_element = $el.find('.form-control') + control_element.addClass('highlight'); setTimeout(() => { - $el.removeClass('has-error'); - $el.find('input, select, textarea').focus(); - }, 1000); + control_element.removeClass('highlight'); + }, 2000); } setup_docinfo_change_listener() { diff --git a/frappe/public/js/frappe/form/multi_select_dialog.js b/frappe/public/js/frappe/form/multi_select_dialog.js index 61922a2422..92d2759f7f 100644 --- a/frappe/public/js/frappe/form/multi_select_dialog.js +++ b/frappe/public/js/frappe/form/multi_select_dialog.js @@ -151,7 +151,7 @@ frappe.ui.form.MultiSelectDialog = class MultiSelectDialog { } is_child_selection_enabled() { - return this.dialog.fields_dict['allow_child_item_selection'].get_value(); + return this.dialog.fields_dict['allow_child_item_selection']?.get_value(); } toggle_child_selection() { diff --git a/frappe/public/js/frappe/form/toolbar.js b/frappe/public/js/frappe/form/toolbar.js index 6841640341..a19062d209 100644 --- a/frappe/public/js/frappe/form/toolbar.js +++ b/frappe/public/js/frappe/form/toolbar.js @@ -84,16 +84,15 @@ frappe.ui.form.Toolbar = class Toolbar { message: __("Unchanged") }); } - rename_document_title(new_name, new_title, merge=false) { + rename_document_title(input_name, input_title, merge=false) { + let confirm_message = null; const docname = this.frm.doc.name; const title_field = this.frm.meta.title_field || ''; const doctype = this.frm.doctype; - let confirm_message=null; - - if (new_name) { + if (input_name) { const warning = __("This cannot be undone"); - const message = __("Are you sure you want to merge {0} with {1}?", [docname.bold(), new_name.bold()]); + const message = __("Are you sure you want to merge {0} with {1}?", [docname.bold(), input_name.bold()]); confirm_message = `${message}
${warning}`; } @@ -101,22 +100,45 @@ frappe.ui.form.Toolbar = class Toolbar { return frappe.xcall("frappe.model.rename_doc.update_document_title", { doctype, docname, - name: new_name, - title: new_title, + name: input_name, + title: input_title, + enqueue: true, merge, freeze: true, freeze_message: __("Updating related fields...") }).then(new_docname => { - if (new_name != docname) { - $(document).trigger("rename", [doctype, docname, new_docname || new_name]); + const reload_form = (input_name) => { + $(document).trigger("rename", [doctype, docname, input_name]); if (locals[doctype] && locals[doctype][docname]) delete locals[doctype][docname]; + this.frm.reload_doc(); + } + + // handle document renaming queued action + if (input_name && (new_docname == docname)) { + frappe.socketio.doc_subscribe(doctype, input_name); + frappe.realtime.on("doc_update", data => { + if (data.doctype == doctype && data.name == input_name) { + reload_form(input_name); + frappe.show_alert({ + message: __('Document renamed from {0} to {1}', [docname.bold(), input_name.bold()]), + indicator: 'success', + }); + } + }); + frappe.show_alert( + __('Document renaming from {0} to {1} has been queued', [docname.bold(), input_name.bold()]) + ); + } + + // handle document sync rename action + if (input_name && ((new_docname || input_name) != docname)) { + reload_form(new_docname || input_name); } - this.frm.reload_doc(); }); }; return new Promise((resolve, reject) => { - if (new_title === this.frm.doc[title_field] && new_name === docname) { + if (input_title === this.frm.doc[title_field] && input_name === docname) { this.show_unchanged_document_alert(); resolve(); } else if (merge) { @@ -569,7 +591,8 @@ frappe.ui.form.Toolbar = class Toolbar { primary_action: ({ fieldname }) => { dialog.hide(); this.frm.scroll_to_field(fieldname); - } + }, + animate: false, }); dialog.show(); diff --git a/frappe/public/js/frappe/ui/keyboard.js b/frappe/public/js/frappe/ui/keyboard.js index e28a8f680d..85ce248175 100644 --- a/frappe/public/js/frappe/ui/keyboard.js +++ b/frappe/public/js/frappe/ui/keyboard.js @@ -37,7 +37,7 @@ frappe.ui.keys.add_shortcut = ({shortcut, action, description, page, target, con if (is_input_focused && !ignore_inputs) return; if (!condition()) return; - if (!page || page.wrapper.is(':visible')) { + if (action && (!page || page.wrapper.is(':visible'))) { let prevent_default = action(e); // prevent default if true is explicitly returned // or nothing returned (undefined) @@ -221,11 +221,11 @@ frappe.ui.keys.add_shortcut({ }); frappe.ui.keys.on('escape', function(e) { - close_grid_and_dialog(); + handle_escape_key(); }); frappe.ui.keys.on('esc', function(e) { - close_grid_and_dialog(); + handle_escape_key(); }); frappe.ui.keys.on('enter', function(e) { @@ -293,6 +293,11 @@ frappe.ui.keyCode = { BACKSPACE: 8 } +function handle_escape_key() { + close_grid_and_dialog(); + document.activeElement?.blur(); +} + function close_grid_and_dialog() { // close open grid row var open_row = $(".grid-row-open"); @@ -308,10 +313,3 @@ function close_grid_and_dialog() { return false; } } - -// blur when escape is pressed on dropdowns -$(document).on('keydown', '.dropdown-toggle', (e) => { - if (e.which === frappe.ui.keyCode.ESCAPE) { - $(e.currentTarget).blur(); - } -}); diff --git a/frappe/public/js/frappe/views/file/file_view.js b/frappe/public/js/frappe/views/file/file_view.js index b351ce6109..de06e6013e 100644 --- a/frappe/public/js/frappe/views/file/file_view.js +++ b/frappe/public/js/frappe/views/file/file_view.js @@ -390,7 +390,7 @@ frappe.views.FileView = class FileView extends frappe.views.ListView { return `
- diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index 71cfa88db1..69aee9b350 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -55,6 +55,10 @@ def DocType(*args, **kwargs): return frappe.qb.DocType(*args, **kwargs) +def Table(*args, **kwargs): + return frappe.qb.Table(*args, **kwargs) + + def patch_query_execute(): """Patch the Query Builder with helper execute method This excludes the use of `frappe.db.sql` method while diff --git a/frappe/test_runner.py b/frappe/test_runner.py index 509be36f86..96feac532f 100644 --- a/frappe/test_runner.py +++ b/frappe/test_runner.py @@ -226,7 +226,7 @@ def run_tests_for_doctype( if force: for name in frappe.db.sql_list("select name from `tab%s`" % doctype): frappe.delete_doc(doctype, name, force=True) - make_test_records(doctype, verbose=verbose, force=force) + make_test_records(doctype, verbose=verbose, force=force, commit=True) modules.append(importlib.import_module(test_module)) return _run_unittest( @@ -245,7 +245,7 @@ def run_tests_for_module( module = importlib.import_module(module) if hasattr(module, "test_dependencies"): for doctype in module.test_dependencies: - make_test_records(doctype, verbose=verbose) + make_test_records(doctype, verbose=verbose, commit=True) frappe.db.commit() return _run_unittest( @@ -330,7 +330,7 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False): if hasattr(module, "test_dependencies"): for doctype in module.test_dependencies: - make_test_records(doctype, verbose=verbose) + make_test_records(doctype, verbose=verbose, commit=True) is_ui_test = True if hasattr(module, "TestDriver") else False @@ -346,12 +346,12 @@ def _add_test(app, path, filename, verbose, test_suite=None, ui_tests=False): with open(txt_file, "r") as f: doc = json.loads(f.read()) doctype = doc["name"] - make_test_records(doctype, verbose) + make_test_records(doctype, verbose, commit=True) test_suite.addTest(unittest.TestLoader().loadTestsFromModule(module)) -def make_test_records(doctype, verbose=0, force=False): +def make_test_records(doctype, verbose=0, force=False, commit=False): if not frappe.db: frappe.connect() @@ -364,8 +364,8 @@ def make_test_records(doctype, verbose=0, force=False): if options not in frappe.local.test_objects: frappe.local.test_objects[options] = [] - make_test_records(options, verbose, force) - make_test_records_for_doctype(options, verbose, force) + make_test_records(options, verbose, force, commit=commit) + make_test_records_for_doctype(options, verbose, force, commit=commit) def get_modules(doctype): @@ -405,7 +405,7 @@ def get_dependencies(doctype): return options_list -def make_test_records_for_doctype(doctype, verbose=0, force=False): +def make_test_records_for_doctype(doctype, verbose=0, force=False, commit=False): if not force and doctype in get_test_record_log(): return @@ -420,17 +420,19 @@ def make_test_records_for_doctype(doctype, verbose=0, force=False): elif hasattr(test_module, "test_records"): if doctype in frappe.local.test_objects: frappe.local.test_objects[doctype] += make_test_objects( - doctype, test_module.test_records, verbose, force + doctype, test_module.test_records, verbose, force, commit=commit ) else: frappe.local.test_objects[doctype] = make_test_objects( - doctype, test_module.test_records, verbose, force + doctype, test_module.test_records, verbose, force, commit=commit ) else: test_records = frappe.get_test_records(doctype) if test_records: - frappe.local.test_objects[doctype] += make_test_objects(doctype, test_records, verbose, force) + frappe.local.test_objects[doctype] += make_test_objects( + doctype, test_records, verbose, force, commit=commit + ) elif verbose: print_mandatory_fields(doctype) @@ -438,7 +440,7 @@ def make_test_records_for_doctype(doctype, verbose=0, force=False): add_to_test_record_log(doctype) -def make_test_objects(doctype, test_records=None, verbose=None, reset=False): +def make_test_objects(doctype, test_records=None, verbose=None, reset=False, commit=False): """Make test objects from given list of `test_records` or from `test_records.json`""" records = [] @@ -495,7 +497,8 @@ def make_test_objects(doctype, test_records=None, verbose=None, reset=False): records.append(d.name) - frappe.db.commit() + if commit: + frappe.db.commit() return records diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 5b469cd5db..6cba55c425 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -87,6 +87,15 @@ class TestDB(unittest.TestCase): frappe.db.get_values("User", filters=[["name", "=", "Administrator"]], fieldname="email"), ) + # test multiple orderby's + delimiter = '"' if frappe.db.db_type == "postgres" else "`" + self.assertIn( + "ORDER BY {deli}creation{deli} DESC,{deli}modified{deli} ASC,{deli}name{deli} DESC".format( + deli=delimiter + ), + frappe.db.get_value("DocType", "DocField", order_by="creation desc, modified asc, name", run=0), + ) + def test_get_value_limits(self): # check both dict and list style filters diff --git a/frappe/tests/test_permissions.py b/frappe/tests/test_permissions.py index 70297a4f54..4164b0be36 100644 --- a/frappe/tests/test_permissions.py +++ b/frappe/tests/test_permissions.py @@ -24,25 +24,26 @@ test_dependencies = ["Blogger", "Blog Post", "User", "Contact", "Salutation"] class TestPermissions(FrappeTestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + frappe.clear_cache(doctype="Blog Post") + user = frappe.get_doc("User", "test1@example.com") + user.add_roles("Website Manager") + user.add_roles("System Manager") + + user = frappe.get_doc("User", "test2@example.com") + user.add_roles("Blogger") + + user = frappe.get_doc("User", "test3@example.com") + user.add_roles("Sales User") + + user = frappe.get_doc("User", "testperm@example.com") + user.add_roles("Website Manager") + def setUp(self): frappe.clear_cache(doctype="Blog Post") - if not frappe.flags.permission_user_setup_done: - user = frappe.get_doc("User", "test1@example.com") - user.add_roles("Website Manager") - user.add_roles("System Manager") - - user = frappe.get_doc("User", "test2@example.com") - user.add_roles("Blogger") - - user = frappe.get_doc("User", "test3@example.com") - user.add_roles("Sales User") - - user = frappe.get_doc("User", "testperm@example.com") - user.add_roles("Website Manager") - - frappe.flags.permission_user_setup_done = True - reset("Blogger") reset("Blog Post") diff --git a/frappe/tests/test_rename_doc.py b/frappe/tests/test_rename_doc.py index 38fc7b32bd..8bf76b3e13 100644 --- a/frappe/tests/test_rename_doc.py +++ b/frappe/tests/test_rename_doc.py @@ -107,8 +107,25 @@ class TestRenameDoc(unittest.TestCase): def setUp(self): frappe.flags.link_fields = {} + if self._testMethodName == "test_doc_rename_method": + self.property_setter = frappe.get_doc( + { + "doctype": "Property Setter", + "doctype_or_field": "DocType", + "doc_type": self.test_doctype, + "property": "allow_rename", + "property_type": "Check", + "value": "1", + } + ).insert() + super().setUp() + def tearDown(self) -> None: + if self._testMethodName == "test_doc_rename_method": + self.property_setter.delete() + return super().tearDown() + def test_rename_doc(self): """Rename an existing document via frappe.rename_doc""" old_name = choice(self.available_documents) @@ -247,3 +264,12 @@ class TestRenameDoc(unittest.TestCase): update_linked_doctypes("User", "ToDo", "str", "str") self.assertTrue("Function frappe.model.rename_doc.update_linked_doctypes" in stdout.getvalue()) + + def test_doc_rename_method(self): + name = choice(self.available_documents) + new_name = f"{name}-{frappe.generate_hash(length=4)}" + doc = frappe.get_doc(self.test_doctype, name) + doc.rename(new_name, merge=frappe.db.exists(self.test_doctype, new_name)) + self.assertEqual(doc.name, new_name) + self.available_documents.append(new_name) + self.available_documents.remove(name) diff --git a/frappe/tests/test_test_utils.py b/frappe/tests/test_test_utils.py new file mode 100644 index 0000000000..4e5c424ca6 --- /dev/null +++ b/frappe/tests/test_test_utils.py @@ -0,0 +1,34 @@ +import frappe +from frappe.tests.utils import FrappeTestCase, change_settings + + +class TestTestUtils(FrappeTestCase): + SHOW_TRANSACTION_COMMIT_WARNINGS = True + + def test_document_assertions(self): + + currency = frappe.new_doc("Currency") + currency.currency_name = "STONKS" + currency.smallest_currency_fraction_value = 0.420_001 + currency.save() + + self.assertDocumentEqual(currency.as_dict(), currency) + + def test_thread_locals(self): + frappe.flags.temp_flag_to_be_discarded = True + + def test_temp_setting_changes(self): + current_setting = frappe.get_system_settings("logout_on_password_reset") + + with change_settings("System Settings", {"logout_on_password_reset": int(not current_setting)}): + updated_settings = frappe.get_system_settings("logout_on_password_reset") + self.assertNotEqual(current_setting, updated_settings) + + restored_settings = frappe.get_system_settings("logout_on_password_reset") + self.assertEqual(current_setting, restored_settings) + + +def tearDownModule(): + """assertions for ensuring tests didn't leave state behind""" + assert "temp_flag_to_be_discarded" not in frappe.flags + assert not frappe.db.exists("Currency", "STONKS") diff --git a/frappe/tests/utils.py b/frappe/tests/utils.py index bad368afd0..7d00a0c1f9 100644 --- a/frappe/tests/utils.py +++ b/frappe/tests/utils.py @@ -1,23 +1,87 @@ import copy +import datetime import signal import unittest from contextlib import contextmanager import frappe +from frappe.model.base_document import BaseDocument +from frappe.utils import cint + +datetime_like_types = (datetime.datetime, datetime.date, datetime.time, datetime.timedelta) class FrappeTestCase(unittest.TestCase): """Base test class for Frappe tests.""" - @classmethod - def setUpClass(cls) -> None: - frappe.db.commit() - return super().setUpClass() + SHOW_TRANSACTION_COMMIT_WARNINGS = False @classmethod - def tearDownClass(cls) -> None: - frappe.db.rollback() - return super().tearDownClass() + def setUpClass(cls) -> None: + # flush changes done so far to avoid flake + frappe.db.commit() + frappe.db.begin() + if cls.SHOW_TRANSACTION_COMMIT_WARNINGS: + frappe.db.add_before_commit(_commit_watcher) + + # enqueue teardown actions (executed in LIFO order) + cls.addClassCleanup(_restore_thread_locals, copy.deepcopy(frappe.local.flags)) + cls.addClassCleanup(_rollback_db) + + return super().setUpClass() + + # --- Frappe Framework specific assertions + def assertDocumentEqual(self, expected, actual): + """Compare a (partial) expected document with actual Document.""" + + if isinstance(expected, BaseDocument): + expected = expected.as_dict() + + for field, value in expected.items(): + if isinstance(value, list): + actual_child_docs = actual.get(field) + self.assertEqual(len(value), len(actual_child_docs), msg=f"{field} length should be same") + for exp_child, actual_child in zip(value, actual_child_docs): + self.assertDocumentEqual(exp_child, actual_child) + else: + self._compare_field(value, actual.get(field), actual, field) + + def _compare_field(self, expected, actual, doc, field): + msg = f"{field} should be same." + + if isinstance(expected, float): + precision = doc.precision(field) + self.assertAlmostEqual(expected, actual, f"{field} should be same to {precision} digits") + elif isinstance(expected, (bool, int)): + self.assertEqual(expected, cint(actual), msg=msg) + elif isinstance(expected, datetime_like_types): + self.assertEqual(str(expected), str(actual), msg=msg) + else: + self.assertEqual(expected, actual, msg=msg) + + +def _commit_watcher(): + import traceback + + print("Warning:, transaction committed during tests.") + traceback.print_stack(limit=5) + + +def _rollback_db(): + frappe.local.before_commit = [] + frappe.local.rollback_observers = [] + frappe.db.value_cache = {} + frappe.db.rollback() + + +def _restore_thread_locals(flags): + frappe.local.flags = flags + frappe.local.error_log = [] + frappe.local.message_log = [] + frappe.local.debug_log = [] + frappe.local.realtime_log = [] + frappe.local.conf = frappe._dict(frappe.get_site_config()) + frappe.local.cache = {} @contextmanager diff --git a/frappe/translate.py b/frappe/translate.py index d95c8eb3e8..0ebf4eaf1b 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -22,7 +22,7 @@ from pypika.terms import PseudoColumn import frappe from frappe.model.utils import InvalidIncludePath, render_include from frappe.query_builder import DocType, Field -from frappe.utils import get_bench_path, is_html, strip, strip_html_tags +from frappe.utils import cstr, get_bench_path, is_html, strip, strip_html_tags TRANSLATE_PATTERN = re.compile( r"_\([\s\n]*" # starts with literal `_(`, ignore following whitespace/newlines @@ -319,11 +319,11 @@ def get_translation_dict_from_file(path, lang, app): elif len(item) in [2, 3]: translation_map[item[0]] = strip(item[1]) elif item: - raise Exception( - "Bad translation in '{app}' for language '{lang}': {values}".format( - app=app, lang=lang, values=repr(item).encode("utf-8") - ) + msg = "Bad translation in '{app}' for language '{lang}': {values}".format( + app=app, lang=lang, values=cstr(item) ) + frappe.log_error(message=msg, title="Error in translation file") + frappe.msgprint(msg) return translation_map diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index b2795e16c3..bc89e5279e 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -174,6 +174,11 @@ def execute_job(site, method, event, job_name, kwargs, user=None, is_async=True, frappe.db.commit() finally: + # background job hygiene: release file locks if unreleased + # if this breaks something, move it to failed jobs alone - gavin@frappe.io + for doc in frappe.local.locked_documents: + doc.unlock() + frappe.monitor.stop() if is_async: frappe.destroy() diff --git a/frappe/utils/backups.py b/frappe/utils/backups.py index e2579444bd..927ae9c2db 100644 --- a/frappe/utils/backups.py +++ b/frappe/utils/backups.py @@ -461,7 +461,7 @@ class BackupGenerator: ) if self.verbose: - print(command + "\n") + print(command.replace(args.password, "*" * 10) + "\n") frappe.utils.execute_in_shell(command, low_priority=True) diff --git a/frappe/utils/csvutils.py b/frappe/utils/csvutils.py index 547372778b..b61e209b72 100644 --- a/frappe/utils/csvutils.py +++ b/frappe/utils/csvutils.py @@ -215,7 +215,10 @@ def get_csv_content_from_google_sheets(url): def validate_google_sheets_url(url): - if "docs.google.com/spreadsheets" not in url: + from urllib.parse import urlparse + + u = urlparse(url) + if u.scheme != "https" or u.netloc != "docs.google.com" or "/spreadsheets/" not in u.path: frappe.throw( _('"{0}" is not a valid Google Sheets URL').format(url), title=_("Invalid URL"), diff --git a/frappe/utils/fixtures.py b/frappe/utils/fixtures.py index f00d310c9d..42e86e9f11 100644 --- a/frappe/utils/fixtures.py +++ b/frappe/utils/fixtures.py @@ -17,11 +17,9 @@ def sync_fixtures(app=None): frappe.flags.in_fixtures = True for app in apps: - if os.path.exists(frappe.get_app_path(app, "fixtures")): - fixture_files = sorted(os.listdir(frappe.get_app_path(app, "fixtures"))) - for fname in fixture_files: - if fname.endswith(".json"): - import_doc(frappe.get_app_path(app, "fixtures", fname)) + fixtures_path = frappe.get_app_path(app, "fixtures") + if os.path.exists(fixtures_path): + import_doc(fixtures_path) import_custom_scripts(app) diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index fc53243021..30cf38bcf9 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -146,6 +146,7 @@ def get_safe_globals(): ), make_get_request=frappe.integrations.utils.make_get_request, make_post_request=frappe.integrations.utils.make_post_request, + get_payment_gateway_controller=frappe.integrations.utils.get_payment_gateway_controller, socketio_port=frappe.conf.socketio_port, get_hooks=get_hooks, enqueue=safe_enqueue, diff --git a/frappe/utils/scheduler.py b/frappe/utils/scheduler.py index d1cda3d0fc..156f2ab04d 100755 --- a/frappe/utils/scheduler.py +++ b/frappe/utils/scheduler.py @@ -24,6 +24,15 @@ from frappe.utils.background_jobs import get_jobs DATETIME_FORMAT = "%Y-%m-%d %H:%M:%S" +def cprint(*args, **kwargs): + """Prints only if called from STDOUT""" + try: + os.get_terminal_size() + print(*args, **kwargs) + except Exception: + pass + + def start_scheduler(): """Run enqueue_events_for_all_sites every 2 minutes (default). Specify scheduler_interval in seconds in common_site_config.json""" @@ -94,9 +103,11 @@ def enqueue_events(site): def is_scheduler_inactive(): if frappe.local.conf.maintenance_mode: + cprint("Maintenance mode is ON") return True if frappe.local.conf.pause_scheduler: + cprint("frappe.conf.pause_scheduler is SET") return True if is_scheduler_disabled(): @@ -107,9 +118,15 @@ def is_scheduler_inactive(): def is_scheduler_disabled(): if frappe.conf.disable_scheduler: + cprint("frappe.conf.disable_scheduler is SET") return True - return not frappe.utils.cint(frappe.db.get_single_value("System Settings", "enable_scheduler")) + scheduler_disabled = not frappe.utils.cint( + frappe.db.get_single_value("System Settings", "enable_scheduler") + ) + if scheduler_disabled: + cprint("SystemSettings.enable_scheduler is UNSET") + return scheduler_disabled def toggle_scheduler(enable): diff --git a/frappe/website/doctype/blog_post/test_blog_post.py b/frappe/website/doctype/blog_post/test_blog_post.py index 0eddad4bfe..558795458b 100644 --- a/frappe/website/doctype/blog_post/test_blog_post.py +++ b/frappe/website/doctype/blog_post/test_blog_post.py @@ -1,12 +1,12 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import re -import unittest from bs4 import BeautifulSoup import frappe from frappe.custom.doctype.customize_form.customize_form import reset_customization +from frappe.tests.utils import FrappeTestCase from frappe.utils import random_string, set_request from frappe.website.doctype.blog_post.blog_post import get_blog_list from frappe.website.serve import get_response @@ -16,7 +16,7 @@ from frappe.website.website_generator import WebsiteGenerator test_dependencies = ["Blog Post"] -class TestBlogPost(unittest.TestCase): +class TestBlogPost(FrappeTestCase): def setUp(self): reset_customization("Blog Post") @@ -61,7 +61,7 @@ class TestBlogPost(unittest.TestCase): category_page_link = list(soup.find_all("a", href=re.compile(blog.blog_category)))[0] category_page_url = category_page_link["href"] - cached_value = frappe.db.value_cache[("DocType", "Blog Post", "name")] + cached_value = frappe.db.value_cache.get(("DocType", "Blog Post", "name")) frappe.db.value_cache[("DocType", "Blog Post", "name")] = (("Blog Post",),) # Visit the category page (by following the link found in above stage) diff --git a/frappe/www/unsubscribe.py b/frappe/www/unsubscribe.py index bae54f740d..d679bb3319 100644 --- a/frappe/www/unsubscribe.py +++ b/frappe/www/unsubscribe.py @@ -8,7 +8,7 @@ no_cache = True def get_context(context): frappe.flags.ignore_permissions = True # Called for confirmation. - if "email" in frappe.form_dict: + if "email" in frappe.form_dict and frappe.request.method == "GET": if verify_request(): user_email = frappe.form_dict["email"] context.email = user_email @@ -18,7 +18,7 @@ def get_context(context): context.status = "waiting_for_confirmation" # Called when form is submitted. - elif "user_email" in frappe.form_dict: + elif "user_email" in frappe.form_dict and frappe.request.method == "POST": context.status = "unsubscribed" email = frappe.form_dict["user_email"] email_group = get_email_groups(email) diff --git a/requirements.txt b/requirements.txt index 6878bd8bcd..e14c35157b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -67,7 +67,6 @@ traceback-with-variables~=2.0.4 urllib3~=1.26.4 Werkzeug~=2.0.3 Whoosh~=2.7.4 -wrapt~=1.14.0 xlrd~=2.0.1 zxcvbn-python~=4.4.24 tenacity~=8.0.1