diff --git a/frappe/automation/doctype/assignment_rule/assignment_rule.py b/frappe/automation/doctype/assignment_rule/assignment_rule.py index 50a6f8b17e..a8c75bffd9 100644 --- a/frappe/automation/doctype/assignment_rule/assignment_rule.py +++ b/frappe/automation/doctype/assignment_rule/assignment_rule.py @@ -1,32 +1,47 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2019, Frappe Technologies and contributors +# Copyright (c) 2022, Frappe Technologies and contributors # License: MIT. See LICENSE +from typing import Dict, Iterable, List + import frappe -from frappe.model.document import Document -from frappe.desk.form import assign_to -import frappe.cache_manager from frappe import _ +from frappe.cache_manager import clear_doctype_map, get_doctype_map +from frappe.desk.form import assign_to from frappe.model import log_types +from frappe.model.document import Document + class AssignmentRule(Document): - def validate(self): + self.validate_document_types() + self.validate_assignment_days() + + def clear_cache(self): + super().clear_cache() + clear_doctype_map(self.doctype, self.document_type) + clear_doctype_map(self.doctype, f"due_date_rules_for_{self.document_type}") + + def validate_document_types(self): + if self.document_type == "ToDo": + frappe.throw( + _('Assignment Rule is not allowed on {0} document type').format( + frappe.bold("ToDo") + ) + ) + + def validate_assignment_days(self): assignment_days = self.get_assignment_days() - if not len(set(assignment_days)) == len(assignment_days): + + if len(set(assignment_days)) != len(assignment_days): repeated_days = get_repeated(assignment_days) - frappe.throw(_("Assignment Day {0} has been repeated.").format(frappe.bold(repeated_days))) - if self.document_type == 'ToDo': - frappe.throw(_('Assignment Rule is not allowed on {0} document type').format(frappe.bold("ToDo"))) + plural = "s" if len(repeated_days) > 1 else "" - def on_update(self): - clear_assignment_rule_cache(self) - - def after_rename(self, old, new, merge): - clear_assignment_rule_cache(self) - - def on_trash(self): - clear_assignment_rule_cache(self) + frappe.throw( + _("Assignment Day{0} {1} has been repeated.").format( + plural, + frappe.bold(", ".join(repeated_days)) + ) + ) def apply_unassign(self, doc, assignments): if (self.unassign_condition and @@ -35,7 +50,6 @@ class AssignmentRule(Document): return False - def apply_assign(self, doc): if self.safe_eval('assign_condition', doc): return self.do_assignment(doc) @@ -141,65 +155,68 @@ class AssignmentRule(Document): def is_rule_not_applicable_today(self): today = frappe.flags.assignment_day or frappe.utils.get_weekday() assignment_days = self.get_assignment_days() - if assignment_days and not today in assignment_days: - return True + return assignment_days and today not in assignment_days - return False -def get_assignments(doc): +def get_assignments(doc) -> List[Dict]: return frappe.get_all('ToDo', fields = ['name', 'assignment_rule'], filters = dict( reference_type = doc.get('doctype'), reference_name = doc.get('name'), status = ('!=', 'Cancelled') - ), limit = 5) + ), limit=5) + @frappe.whitelist() def bulk_apply(doctype, docnames): - import json - docnames = json.loads(docnames) - + docnames = frappe.parse_json(docnames) background = len(docnames) > 5 + for name in docnames: if background: frappe.enqueue('frappe.automation.doctype.assignment_rule.assignment_rule.apply', doc=None, doctype=doctype, name=name) else: - apply(None, doctype=doctype, name=name) + apply(doctype=doctype, name=name) + def reopen_closed_assignment(doc): - todo_list = frappe.db.get_all('ToDo', filters = dict( - reference_type = doc.doctype, - reference_name = doc.name, - status = 'Closed' - )) - if not todo_list: - return False + todo_list = frappe.get_all("ToDo", filters={ + "reference_type": doc.doctype, + "reference_name": doc.name, + "status": "Closed", + }, pluck="name") + for todo in todo_list: - todo_doc = frappe.get_doc('ToDo', todo.name) + todo_doc = frappe.get_doc('ToDo', todo) todo_doc.status = 'Open' todo_doc.save(ignore_permissions=True) - return True -def apply(doc, method=None, doctype=None, name=None): - if not doctype: - doctype = doc.doctype + return bool(todo_list) - if (frappe.flags.in_patch + +def apply(doc=None, method=None, doctype=None, name=None): + doctype = doctype or doc.doctype + + skip_assignment_rules = ( + frappe.flags.in_patch or frappe.flags.in_install or frappe.flags.in_setup_wizard - or doctype in log_types): + or doctype in log_types + ) + + if skip_assignment_rules: return if not doc and doctype and name: doc = frappe.get_doc(doctype, name) - assignment_rules = frappe.cache_manager.get_doctype_map('Assignment Rule', doc.doctype, dict( - document_type = doc.doctype, disabled = 0), order_by = 'priority desc') - - assignment_rule_docs = [] + assignment_rules = get_doctype_map("Assignment Rule", doc.doctype, filters={ + "document_type": doc.doctype, "disabled": 0 + }, order_by="priority desc") # multiple auto assigns - for d in assignment_rules: - assignment_rule_docs.append(frappe.get_cached_doc('Assignment Rule', d.get('name'))) + assignment_rule_docs: List[AssignmentRule] = [ + frappe.get_cached_doc("Assignment Rule", d.get('name')) for d in assignment_rules + ] if not assignment_rule_docs: return @@ -235,6 +252,7 @@ def apply(doc, method=None, doctype=None, name=None): # apply close rule only if assignments exists assignments = get_assignments(doc) + if assignments: for assignment_rule in assignment_rule_docs: if assignment_rule.is_rule_not_applicable_today(): @@ -242,38 +260,74 @@ def apply(doc, method=None, doctype=None, name=None): if not new_apply: # only reopen if close condition is not satisfied - if not assignment_rule.safe_eval('close_condition', doc): - reopen = reopen_closed_assignment(doc) - if reopen: + to_close_todos = assignment_rule.safe_eval('close_condition', doc) + + if to_close_todos: + # close todo status + todos_to_close = frappe.get_all("ToDo", filters={ + "reference_type": doc.doctype, + "reference_name": doc.name, + }, pluck="name") + + for todo in todos_to_close: + _todo = frappe.get_doc("ToDo", todo) + _todo.status = "Closed" + _todo.save() + break + + else: + reopened = reopen_closed_assignment(doc) + if reopened: break + + # print(f"Rule:{assignment_rule}\nDoc: {doc}\nReOpened: {reopened}") + assignment_rule.close_assignments(doc) + def update_due_date(doc, state=None): - # called from hook - if (frappe.flags.in_patch - or frappe.flags.in_install - or frappe.flags.in_migrate + """Run on_update on every Document (via hooks.py) + """ + skip_document_update = ( + frappe.flags.in_migrate + or frappe.flags.in_patch or frappe.flags.in_import - or frappe.flags.in_setup_wizard): + or frappe.flags.in_setup_wizard + or frappe.flags.in_install + ) + + if skip_document_update: return - assignment_rules = frappe.cache_manager.get_doctype_map('Assignment Rule', 'due_date_rules_for_' + doc.doctype, dict( - document_type = doc.doctype, - disabled = 0, - due_date_based_on = ['is', 'set'] - )) + + assignment_rules = get_doctype_map( + doctype="Assignment Rule", + name=f"due_date_rules_for_{doc.doctype}", + filters={ + "due_date_based_on": ["is", "set"], + "document_type": doc.doctype, + "disabled": 0, + } + ) + for rule in assignment_rules: - rule_doc = frappe.get_cached_doc('Assignment Rule', rule.get('name')) + rule_doc = frappe.get_cached_doc("Assignment Rule", rule.get("name")) due_date_field = rule_doc.due_date_based_on - if doc.meta.has_field(due_date_field) and \ - doc.has_value_changed(due_date_field) and rule.get('name'): - assignment_todos = frappe.get_all('ToDo', { - 'assignment_rule': rule.get('name'), - 'status': 'Open', - 'reference_type': doc.doctype, - 'reference_name': doc.name - }) + field_updated = ( + doc.meta.has_field(due_date_field) + and doc.has_value_changed(due_date_field) + and rule.get("name") + ) + + if field_updated: + assignment_todos = frappe.get_all("ToDo", filters={ + "assignment_rule": rule.get("name"), + "reference_type": doc.doctype, + "reference_name": doc.name, + "status": "Open", + }, pluck="name") + for todo in assignment_todos: - todo_doc = frappe.get_doc('ToDo', todo.name) + todo_doc = frappe.get_doc('ToDo', todo) todo_doc.date = doc.get(due_date_field) todo_doc.flags.updater_reference = { 'doctype': 'Assignment Rule', @@ -282,20 +336,19 @@ def update_due_date(doc, state=None): } todo_doc.save(ignore_permissions=True) -def get_assignment_rules(): - return [d.document_type for d in frappe.db.get_all('Assignment Rule', fields=['document_type'], filters=dict(disabled = 0))] -def get_repeated(values): - unique_list = [] - diff = [] +def get_assignment_rules() -> List[str]: + return frappe.get_all("Assignment Rule", filters={"disabled": 0}, pluck="document_type") + + +def get_repeated(values: Iterable) -> List: + unique = set() + repeated = set() + for value in values: - if value not in unique_list: - unique_list.append(str(value)) + if value in unique: + repeated.add(value) else: - if value not in diff: - diff.append(str(value)) - return " ".join(diff) + unique.add(value) -def clear_assignment_rule_cache(rule): - frappe.cache_manager.clear_doctype_map('Assignment Rule', rule.document_type) - frappe.cache_manager.clear_doctype_map('Assignment Rule', 'due_date_rules_for_' + rule.document_type) + return [str(x) for x in repeated] diff --git a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py b/frappe/automation/doctype/assignment_rule/test_assignment_rule.py index 84b8dbd3c8..63dbf69d3b 100644 --- a/frappe/automation/doctype/assignment_rule/test_assignment_rule.py +++ b/frappe/automation/doctype/assignment_rule/test_assignment_rule.py @@ -1,12 +1,22 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2019, Frappe Technologies and Contributors +# Copyright (c) 2021, Frappe Technologies and Contributors # License: MIT. See LICENSE -import frappe + import unittest -from frappe.utils import random_string + +import frappe from frappe.test_runner import make_test_records +from frappe.utils import random_string + class TestAutoAssign(unittest.TestCase): + @classmethod + def setUpClass(cls): + frappe.db.delete("Assignment Rule") + + @classmethod + def tearDownClass(cls): + frappe.db.rollback() + def setUp(self): make_test_records("User") days = [ @@ -129,7 +139,7 @@ class TestAutoAssign(unittest.TestCase): reference_type = 'Note', reference_name = note.name, status = 'Open' - ))[0] + ), limit=1)[0] todo = frappe.get_doc('ToDo', todo['name']) self.assertEqual(todo.allocated_to, 'test@example.com') @@ -151,7 +161,7 @@ class TestAutoAssign(unittest.TestCase): reference_type = 'Note', reference_name = note.name, status = 'Open' - ))[0] + ), limit=1)[0] todo = frappe.get_doc('ToDo', todo['name']) self.assertEqual(todo.allocated_to, 'test@example.com') diff --git a/frappe/automation/doctype/auto_repeat/auto_repeat.py b/frappe/automation/doctype/auto_repeat/auto_repeat.py index 5ab6c86c00..0277b8e402 100644 --- a/frappe/automation/doctype/auto_repeat/auto_repeat.py +++ b/frappe/automation/doctype/auto_repeat/auto_repeat.py @@ -96,7 +96,15 @@ class AutoRepeat(Document): auto_repeat_days = self.get_auto_repeat_days() if not len(set(auto_repeat_days)) == len(auto_repeat_days): repeated_days = get_repeated(auto_repeat_days) - frappe.throw(_('Auto Repeat Day {0} has been repeated.').format(frappe.bold(repeated_days))) + plural = "s" if len(repeated_days) > 1 else "" + + frappe.throw( + _("Auto Repeat Day{0} {1} has been repeated.").format( + plural, + frappe.bold(", ".join(repeated_days)) + ) + ) + def update_auto_repeat_id(self): #check if document is already on auto repeat diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 3a78a6a599..96c8f271d9 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -488,10 +488,12 @@ def update_parent_document_on_communication(doc): def update_first_response_time(parent, communication): if parent.meta.has_field("first_response_time") and not parent.get("first_response_time"): if is_system_user(communication.sender): - first_responded_on = communication.creation - if parent.meta.has_field("first_responded_on") and communication.sent_or_received == "Sent": - parent.db_set("first_responded_on", first_responded_on) - parent.db_set("first_response_time", round(time_diff_in_seconds(first_responded_on, parent.creation), 2)) + if communication.sent_or_received == "Sent": + first_responded_on = communication.creation + if parent.meta.has_field("first_responded_on"): + parent.db_set("first_responded_on", first_responded_on) + first_response_time = round(time_diff_in_seconds(first_responded_on, parent.creation), 2) + parent.db_set("first_response_time", first_response_time) def set_avg_response_time(parent, communication): if parent.meta.has_field("avg_response_time") and communication.sent_or_received == "Sent": diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index ad0c3e8e6f..3754288145 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1283,7 +1283,7 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): roles = [p.role for p in doc.get("permissions") or []] + default_roles for role in list(set(roles)): - if not frappe.db.exists("Role", role): + if frappe.db.table_exists("Role", cached=False) and not frappe.db.exists("Role", role): r = frappe.get_doc(dict(doctype= "Role", role_name=role, desk_access=1)) r.flags.ignore_mandatory = r.flags.ignore_permissions = True r.insert() diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 4362a52c34..12c227464d 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -15,6 +15,10 @@ from frappe.core.doctype.doctype.doctype import (UniqueFieldnameError, # test_records = frappe.get_test_records('DocType') class TestDocType(unittest.TestCase): + + def tearDown(self): + frappe.db.rollback() + def test_validate_name(self): self.assertRaises(frappe.NameError, new_doctype("_Some DocType").insert) self.assertRaises(frappe.NameError, new_doctype("8Some DocType").insert) @@ -42,6 +46,7 @@ class TestDocType(unittest.TestCase): doc1.insert() self.assertRaises(frappe.UniqueValidationError, doc2.insert) + frappe.db.rollback() dt.fields[0].unique = 0 dt.save() diff --git a/frappe/database/database.py b/frappe/database/database.py index a157343be6..65242e0419 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -164,10 +164,7 @@ class Database(object): frappe.errprint(("Execution time: {0} sec").format(round(time_end - time_start, 2))) except Exception as e: - if frappe.conf.db_type == 'postgres': - self.rollback() - - elif self.is_syntax_error(e): + if self.is_syntax_error(e): # only for mariadb frappe.errprint('Syntax error in query:') frappe.errprint(query) @@ -178,6 +175,9 @@ class Database(object): elif self.is_timedout(e): raise frappe.QueryTimeoutError(e) + elif frappe.conf.db_type == 'postgres': + raise + if ignore_ddl and (self.is_missing_column(e) or self.is_missing_table(e) or self.cant_drop_field_or_key(e)): pass else: @@ -267,9 +267,7 @@ class Database(object): """Raises exception if more than 20,000 `INSERT`, `UPDATE` queries are executed in one transaction. This is to ensure that writes are always flushed otherwise this could cause the system to hang.""" - if self.transaction_writes and \ - query and query.strip().split()[0].lower() in ['start', 'alter', 'drop', 'create', "begin", "truncate"]: - raise Exception('This statement can cause implicit commit') + self.check_implicit_commit(query) if query and query.strip().lower() in ('commit', 'rollback'): self.transaction_writes = 0 @@ -282,6 +280,11 @@ class Database(object): else: frappe.throw(_("Too many writes in one request. Please send smaller requests"), frappe.ValidationError) + def check_implicit_commit(self, query): + if self.transaction_writes and \ + query and query.strip().split()[0].lower() in ['start', 'alter', 'drop', 'create', "begin", "truncate"]: + raise Exception('This statement can cause implicit commit') + def fetch_as_dict(self, formatted=0, as_utf8=0): """Internal. Converts results to dict.""" result = self._cursor.fetchall() @@ -701,6 +704,8 @@ class Database(object): self.sql("""update `tab{0}` set {1} where name=%(name)s""".format(dt, ', '.join(set_values)), values, debug=debug) + + frappe.clear_document_cache(dt, values['name']) else: # for singles keys = list(to_update) @@ -713,10 +718,11 @@ class Database(object): self.sql('''insert into `tabSingles` (doctype, field, value) values (%s, %s, %s)''', (dt, key, value), debug=debug) + frappe.clear_document_cache(dt, dn) + if dt in self.value_cache: del self.value_cache[dt] - frappe.clear_document_cache(dt, dn) @staticmethod def set(doc, field, val): @@ -835,9 +841,9 @@ class Database(object): 'parent': dt }) - def table_exists(self, doctype): + def table_exists(self, doctype, cached=True): """Returns True if table for given doctype exists.""" - return ("tab" + doctype) in self.get_tables() + return ("tab" + doctype) in self.get_tables(cached=cached) def has_table(self, doctype): return self.table_exists(doctype) diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index 008635b1b3..0ce6fbb265 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -3,7 +3,7 @@ from typing import List, Tuple, Union import psycopg2 import psycopg2.extensions -from psycopg2.extensions import ISOLATION_LEVEL_AUTOCOMMIT +from psycopg2.extensions import ISOLATION_LEVEL_REPEATABLE_READ from psycopg2.errorcodes import STRING_DATA_RIGHT_TRUNCATION import frappe @@ -69,7 +69,7 @@ class PostgresDatabase(Database): conn = psycopg2.connect("host='{}' dbname='{}' user='{}' password='{}' port={}".format( self.host, self.user, self.user, self.password, self.port )) - conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT) # TODO: Remove this + conn.set_isolation_level(ISOLATION_LEVEL_REPEATABLE_READ) return conn @@ -103,7 +103,7 @@ class PostgresDatabase(Database): return super(PostgresDatabase, self).sql(*args, **kwargs) - def get_tables(self): + def get_tables(self, cached=True): return [d[0] for d in self.sql("""select table_name from information_schema.tables where table_catalog='{0}' @@ -138,6 +138,10 @@ class PostgresDatabase(Database): # http://initd.org/psycopg/docs/extensions.html?highlight=datatype#psycopg2.extensions.QueryCanceledError return isinstance(e, psycopg2.extensions.QueryCanceledError) + @staticmethod + def is_syntax_error(e): + return isinstance(e, psycopg2.errors.SyntaxError) + @staticmethod def is_table_missing(e): return getattr(e, 'pgcode', None) == '42P01' @@ -255,8 +259,8 @@ class PostgresDatabase(Database): key=key ) - def check_transaction_status(self, query): - pass + def check_implicit_commit(self, query): + pass # postgres can run DDL in transactions without implicit commits def has_index(self, table_name, index_name): return self.sql("""SELECT 1 FROM pg_indexes WHERE tablename='{table_name}' diff --git a/frappe/desk/doctype/todo/todo.py b/frappe/desk/doctype/todo/todo.py index eabb28a6f3..e689faafbe 100644 --- a/frappe/desk/doctype/todo/todo.py +++ b/frappe/desk/doctype/todo/todo.py @@ -69,15 +69,13 @@ class ToDo(Document): return try: - assignments = [d[0] for d in frappe.get_all("ToDo", - filters={ - "reference_type": self.reference_type, - "reference_name": self.reference_name, - "status": ("!=", "Cancelled") - }, - fields=["allocated_to"], as_list=True)] - + assignments = frappe.get_all("ToDo", filters={ + "reference_type": self.reference_type, + "reference_name": self.reference_name, + "status": ("!=", "Cancelled") + }, pluck="allocated_to") assignments.reverse() + frappe.db.set_value(self.reference_type, self.reference_name, "_assign", json.dumps(assignments), update_modified=False) diff --git a/frappe/desk/form/assign_to.py b/frappe/desk/form/assign_to.py index 7ea87b8d15..049d33c1ec 100644 --- a/frappe/desk/form/assign_to.py +++ b/frappe/desk/form/assign_to.py @@ -19,11 +19,11 @@ def get(args=None): if not args: args = frappe.local.form_dict - return frappe.get_all('ToDo', fields=['allocated_to', 'name'], filters=dict( - reference_type = args.get('doctype'), - reference_name = args.get('name'), - status = ('!=', 'Cancelled') - ), limit=5) + return frappe.get_all("ToDo", fields=["allocated_to as owner", "name"], filters={ + "reference_type": args.get("doctype"), + "reference_name": args.get("name"), + "status": ("!=", "Cancelled") + }, limit=5) @frappe.whitelist() def add(args=None): diff --git a/frappe/desk/form/load.py b/frappe/desk/form/load.py index 89e6598859..0e644c3cf5 100644 --- a/frappe/desk/form/load.py +++ b/frappe/desk/form/load.py @@ -253,7 +253,7 @@ def get_communication_data(doctype, name, start=0, limit=20, after=None, fields= def get_assignments(dt, dn): cl = frappe.get_all("ToDo", - fields=['name', 'owner', 'description', 'status'], + fields=['name', 'allocated_to as owner', 'description', 'status'], filters={ 'reference_type': dt, 'reference_name': dn, diff --git a/frappe/integrations/doctype/ldap_settings/ldap_settings.py b/frappe/integrations/doctype/ldap_settings/ldap_settings.py index 1c5abb454c..7c9c64ba3c 100644 --- a/frappe/integrations/doctype/ldap_settings/ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/ldap_settings.py @@ -58,9 +58,9 @@ class LDAPSettings(Document): import ssl if self.require_trusted_certificate == 'Yes': - tls_configuration = ldap3.Tls(validate=ssl.CERT_REQUIRED, version=ssl.PROTOCOL_TLSv1) + tls_configuration = ldap3.Tls(validate=ssl.CERT_REQUIRED, version=ssl.PROTOCOL_TLS_CLIENT) else: - tls_configuration = ldap3.Tls(validate=ssl.CERT_NONE, version=ssl.PROTOCOL_TLSv1) + tls_configuration = ldap3.Tls(validate=ssl.CERT_NONE, version=ssl.PROTOCOL_TLS_CLIENT) if self.local_private_key_file: tls_configuration.private_key_file = self.local_private_key_file diff --git a/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py b/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py index 7b0638876b..41997fb4c7 100644 --- a/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py +++ b/frappe/integrations/doctype/ldap_settings/test_ldap_settings.py @@ -296,7 +296,7 @@ class LDAP_TestCase(): if local_doc['require_trusted_certificate'] == 'Yes': tls_validate = ssl.CERT_REQUIRED - tls_version = ssl.PROTOCOL_TLSv1 + tls_version = ssl.PROTOCOL_TLS_CLIENT tls_configuration = ldap3.Tls(validate=tls_validate, version=tls_version) self.assertTrue(kwargs['auto_bind'] == ldap3.AUTO_BIND_TLS_BEFORE_BIND, @@ -304,7 +304,7 @@ class LDAP_TestCase(): else: tls_validate = ssl.CERT_NONE - tls_version = ssl.PROTOCOL_TLSv1 + tls_version = ssl.PROTOCOL_TLS_CLIENT tls_configuration = ldap3.Tls(validate=tls_validate, version=tls_version) self.assertTrue(kwargs['auto_bind'], diff --git a/frappe/model/delete_doc.py b/frappe/model/delete_doc.py index ac976e976c..2fddcf9e33 100644 --- a/frappe/model/delete_doc.py +++ b/frappe/model/delete_doc.py @@ -117,7 +117,8 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reloa doctype=doc.doctype, name=doc.name, is_async=False if frappe.flags.in_test else True) - + # clear cache for Document + doc.clear_cache() # delete global search entry delete_for_document(doc) # delete tag link entry diff --git a/frappe/model/document.py b/frappe/model/document.py index e519ab257b..e25469c68a 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -396,6 +396,7 @@ class Document(BaseDocument): "parenttype": self.doctype, "parentfield": fieldname }) + def get_doc_before_save(self): return getattr(self, '_doc_before_save', None) @@ -468,10 +469,12 @@ class Document(BaseDocument): self._original_modified = self.modified self.modified = now() self.modified_by = frappe.session.user - if not self.creation: - self.creation = self.modified + + # We'd probably want the creation and owner to be set via API + # or Data import at some point, that'd have to be handled here if self.is_new(): - self.owner = self.flags.owner or self.modified_by + self.creation = self.modified + self.owner = self.modified_by for d in self.get_all_children(): d.modified = self.modified @@ -501,7 +504,6 @@ class Document(BaseDocument): self._sanitize_content() self._save_passwords() self.validate_workflow() - self.validate_owner() children = self.get_all_children() for d in children: @@ -544,11 +546,6 @@ class Document(BaseDocument): if not self._action == 'save': set_workflow_state_on_action(self, workflow, self._action) - def validate_owner(self): - """Validate if the owner of the Document has changed""" - if not self.is_new() and self.has_value_changed('owner'): - frappe.throw(_('Document owner cannot be changed')) - def validate_set_only_once(self): """Validate that fields are not changed if not in insert""" set_only_once_fields = self.meta.get_set_only_once_fields() @@ -568,8 +565,12 @@ class Document(BaseDocument): fail = value != original_value if fail: - frappe.throw(_("Value cannot be changed for {0}").format(self.meta.get_label(field.fieldname)), - frappe.CannotChangeConstantError) + frappe.throw( + _("Value cannot be changed for {0}").format( + frappe.bold(self.meta.get_label(field.fieldname)) + ), + exc=frappe.CannotChangeConstantError + ) return False diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 252c463d3d..a483f3f2d6 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -67,6 +67,10 @@ class Meta(Document): _metaclass = True default_fields = list(default_fields)[1:] special_doctypes = ("DocField", "DocPerm", "DocType", "Module Def", 'DocType Action', 'DocType Link', 'DocType State') + standard_set_once_fields = [ + frappe._dict(fieldname="creation", fieldtype="Datetime"), + frappe._dict(fieldname="owner", fieldtype="Data"), + ] def __init__(self, doctype): self._fields = {} @@ -154,6 +158,12 @@ class Meta(Document): '''Return fields with `set_only_once` set''' if not hasattr(self, "_set_only_once_fields"): self._set_only_once_fields = self.get("fields", {"set_only_once": 1}) + fieldnames = [d.fieldname for d in self._set_only_once_fields] + + for df in self.standard_set_once_fields: + if df.fieldname not in fieldnames: + self._set_only_once_fields.append(df) + return self._set_only_once_fields def get_table_fields(self): diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index 53991235c9..2cc5818414 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -110,6 +110,7 @@ def rename_doc( if merge: frappe.delete_doc(doctype, old) + new_doc.clear_cache() frappe.clear_cache() if rebuild_search: frappe.enqueue('frappe.utils.global_search.rebuild_for_doctype', doctype=doctype) diff --git a/frappe/tests/test_assign.py b/frappe/tests/test_assign.py index 971f9ce071..05bf7e2fb3 100644 --- a/frappe/tests/test_assign.py +++ b/frappe/tests/test_assign.py @@ -13,7 +13,7 @@ class TestAssign(unittest.TestCase): added = assign(todo, "test@example.com") - self.assertTrue("test@example.com" in [d.allocated_to for d in added]) + self.assertTrue("test@example.com" in [d.owner for d in added]) removed = frappe.desk.form.assign_to.remove(todo.doctype, todo.name, "test@example.com") diff --git a/frappe/tests/test_document.py b/frappe/tests/test_document.py index 46638f5bf2..29cec8b230 100644 --- a/frappe/tests/test_document.py +++ b/frappe/tests/test_document.py @@ -252,22 +252,3 @@ class TestDocument(unittest.TestCase): 'currency': 100000 }) self.assertEquals(d.get_formatted('currency', currency='INR', format="#,###.##"), '₹ 100,000.00') - - def test_owner_changed(self): - frappe.delete_doc_if_exists("User", "hello@example.com") - frappe.set_user("Administrator") - - d = frappe.get_doc({ - "doctype": "User", - "email": "hello@example.com", - "first_name": "John" - }) - d.insert() - self.assertEqual(frappe.db.get_value("User", d.owner), d.owner) - - d.set("owner", "johndoe@gmail.com") - self.assertRaises(frappe.ValidationError, d.save) - - d.reload() - d.save() - self.assertEqual(frappe.db.get_value("User", d.owner), d.owner) diff --git a/frappe/tests/test_frappe_client.py b/frappe/tests/test_frappe_client.py index 66e1160eea..e84163eb41 100644 --- a/frappe/tests/test_frappe_client.py +++ b/frappe/tests/test_frappe_client.py @@ -10,8 +10,9 @@ import requests import base64 class TestFrappeClient(unittest.TestCase): + PASSWORD = "admin" def test_insert_many(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Note", {"title": ("in", ('Sing','a','song','of','sixpence'))}) frappe.db.commit() @@ -30,7 +31,7 @@ class TestFrappeClient(unittest.TestCase): self.assertTrue(frappe.db.get_value('Note', {'title': 'sixpence'})) def test_create_doc(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Note", {"title": "test_create"}) frappe.db.commit() @@ -39,13 +40,13 @@ class TestFrappeClient(unittest.TestCase): self.assertTrue(frappe.db.get_value('Note', {'title': 'test_create'})) def test_list_docs(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) doc_list = server.get_list("Note") self.assertTrue(len(doc_list)) def test_get_doc(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Note", {"title": "get_this"}) frappe.db.commit() @@ -56,7 +57,7 @@ class TestFrappeClient(unittest.TestCase): self.assertTrue(doc) def test_get_value(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Note", {"title": "get_value"}) frappe.db.commit() @@ -74,14 +75,14 @@ class TestFrappeClient(unittest.TestCase): self.assertRaises(FrappeException, server.get_value, "Note", "(select (password) from(__Auth) order by name desc limit 1)", {"title": "get_value"}) def test_get_single(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) server.set_value('Website Settings', 'Website Settings', 'title_prefix', 'test-prefix') self.assertEqual(server.get_value('Website Settings', 'title_prefix', 'Website Settings').get('title_prefix'), 'test-prefix') self.assertEqual(server.get_value('Website Settings', 'title_prefix').get('title_prefix'), 'test-prefix') frappe.db.set_value('Website Settings', None, 'title_prefix', '') def test_update_doc(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Note", {"title": ("in", ("Sing", "sing"))}) frappe.db.commit() @@ -93,7 +94,7 @@ class TestFrappeClient(unittest.TestCase): self.assertTrue(doc["title"] == changed_title) def test_update_child_doc(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Contact", {"first_name": "George", "last_name": "Steevens"}) frappe.db.delete("Contact", {"first_name": "William", "last_name": "Shakespeare"}) frappe.db.delete("Communication", {"reference_doctype": "Event"}) @@ -130,7 +131,7 @@ class TestFrappeClient(unittest.TestCase): self.assertTrue(frappe.db.exists("Communication Link", {"link_name": "William Shakespeare"})) def test_delete_doc(self): - server = FrappeClient(get_url(), "Administrator", "admin", verify=False) + server = FrappeClient(get_url(), "Administrator", self.PASSWORD, verify=False) frappe.db.delete("Note", {"title": "delete"}) frappe.db.commit() diff --git a/frappe/tests/test_permissions.py b/frappe/tests/test_permissions.py index b4e7db9956..fdff4d103e 100644 --- a/frappe/tests/test_permissions.py +++ b/frappe/tests/test_permissions.py @@ -12,6 +12,7 @@ from frappe.core.page.permission_manager.permission_manager import update, reset from frappe.test_runner import make_test_records_for_doctype from frappe.core.doctype.user_permission.user_permission import clear_user_permissions from frappe.desk.form.load import getdoc +from frappe.utils.data import now_datetime test_dependencies = ['Blogger', 'Blog Post', "User", "Contact", "Salutation"] @@ -197,6 +198,32 @@ class TestPermissions(unittest.TestCase): doc = frappe.get_doc("Blog Post", "-test-blog-post") self.assertTrue(doc.has_permission("read")) + def test_set_standard_fields_manually(self): + # check that creation and owner cannot be set manually + from datetime import timedelta + + fake_creation = now_datetime() + timedelta(days=-7) + fake_owner = frappe.db.get_value("User", {"name": ("!=", frappe.session.user)}) + + d = frappe.new_doc("ToDo") + d.description = "ToDo created via test_set_standard_fields_manually" + d.creation = fake_creation + d.owner = fake_owner + d.save() + self.assertNotEqual(d.creation, fake_creation) + self.assertNotEqual(d.owner, fake_owner) + + def test_dont_change_standard_constants(self): + # check that Document.creation cannot be changed + user = frappe.get_doc("User", frappe.session.user) + user.creation = now_datetime() + self.assertRaises(frappe.CannotChangeConstantError, user.save) + + # check that Document.owner cannot be changed + user.reload() + user.owner = frappe.db.get_value("User", {"name": ("!=", user.name)}) + self.assertRaises(frappe.CannotChangeConstantError, user.save) + def test_set_only_once(self): blog_post = frappe.get_meta("Blog Post") doc = frappe.get_doc("Blog Post", "-test-blog-post-1") diff --git a/frappe/utils/install.py b/frappe/utils/install.py index bdc5fdfbc5..5ca8c4878a 100644 --- a/frappe/utils/install.py +++ b/frappe/utils/install.py @@ -5,11 +5,11 @@ import getpass from frappe.utils.password import update_password def before_install(): + frappe.reload_doc("core", "doctype", "doctype_state") frappe.reload_doc("core", "doctype", "docfield") frappe.reload_doc("core", "doctype", "docperm") frappe.reload_doc("core", "doctype", "doctype_action") frappe.reload_doc("core", "doctype", "doctype_link") - frappe.reload_doc("core", "doctype", "doctype_state") frappe.reload_doc("desk", "doctype", "form_tour_step") frappe.reload_doc("desk", "doctype", "form_tour") frappe.reload_doc("core", "doctype", "doctype") diff --git a/frappe/website/doctype/blog_post/templates/blog_post_list.html b/frappe/website/doctype/blog_post/templates/blog_post_list.html index 2b3d5e250c..1aa30316fe 100644 --- a/frappe/website/doctype/blog_post/templates/blog_post_list.html +++ b/frappe/website/doctype/blog_post/templates/blog_post_list.html @@ -52,5 +52,39 @@ {% endblock %} {% block script %} - + {% endblock %} diff --git a/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py b/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py index 8fc8f38512..27dcfe5858 100644 --- a/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py +++ b/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py @@ -50,11 +50,10 @@ class TestPersonalDataDeletionRequest(unittest.TestCase): def test_unverified_record_removal(self): date_time_obj = datetime.strptime( self.delete_request.creation, "%Y-%m-%d %H:%M:%S.%f" - ) - date_time_obj += timedelta(days=-7) - self.delete_request.creation = date_time_obj - self.status = "Pending Verification" - self.delete_request.save() + ) + timedelta(days=-7) + self.delete_request.db_set("creation", date_time_obj) + self.delete_request.db_set("status", "Pending Verification") + remove_unverified_record() self.assertFalse( frappe.db.exists("Personal Data Deletion Request", self.delete_request.name) diff --git a/yarn.lock b/yarn.lock index d5bc1d669f..ba58f6b719 100644 --- a/yarn.lock +++ b/yarn.lock @@ -554,9 +554,9 @@ caniuse-api@^3.0.0: lodash.uniq "^4.5.0" caniuse-lite@^1.0.0, caniuse-lite@^1.0.30001109, caniuse-lite@^1.0.30001196, caniuse-lite@^1.0.30001219: - version "1.0.30001272" - resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001272.tgz" - integrity sha512-DV1j9Oot5dydyH1v28g25KoVm7l8MTxazwuiH3utWiAS6iL/9Nh//TGwqFEeqqN8nnWYQ8HHhUq+o4QPt9kvYw== + version "1.0.30001296" + resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001296.tgz" + integrity sha512-WfrtPEoNSoeATDlf4y3QvkwiELl9GyPLISV5GejTbbQRtQx4LhsXmc9IQ6XCL2d7UxCyEzToEZNMeqR79OUw8Q== caseless@~0.12.0: version "0.12.0"