From 6cdf514fcbf6d8abd6663da63fc62ffafc10109d Mon Sep 17 00:00:00 2001 From: Saqib Ansari Date: Mon, 26 Apr 2021 14:30:17 +0530 Subject: [PATCH 01/46] fix: auto repeat schedule not rendered in the dashboard --- frappe/automation/doctype/auto_repeat/auto_repeat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/automation/doctype/auto_repeat/auto_repeat.js b/frappe/automation/doctype/auto_repeat/auto_repeat.js index 7028ac486d..896a10dfe0 100644 --- a/frappe/automation/doctype/auto_repeat/auto_repeat.js +++ b/frappe/automation/doctype/auto_repeat/auto_repeat.js @@ -103,7 +103,7 @@ frappe.ui.form.on('Auto Repeat', { frappe.auto_repeat.render_schedule = function(frm) { if (!frm.is_dirty() && frm.doc.status !== 'Disabled') { frm.call("get_auto_repeat_schedule").then(r => { - frm.dashboard.wrapper.empty(); + frm.dashboard.reset(); frm.dashboard.add_section( frappe.render_template("auto_repeat_schedule", { schedule_details: r.message || [] From db5038f40e1dd129b61a39a8efe9f844ab0b3f8d Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 3 May 2021 16:35:21 +0530 Subject: [PATCH 02/46] fix: Allow to duplicate standard notification --- frappe/email/doctype/notification/notification.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/email/doctype/notification/notification.py b/frappe/email/doctype/notification/notification.py index 2940a34f63..f0252f6dec 100644 --- a/frappe/email/doctype/notification/notification.py +++ b/frappe/email/doctype/notification/notification.py @@ -65,6 +65,10 @@ def get_context(context): """) def validate_standard(self): + # indicates that this is a new doc + if self._doc_before_save == None and not frappe.conf.developer_mode: + self.is_standard = False + return if self.is_standard and not frappe.conf.developer_mode: frappe.throw(_('Cannot edit Standard Notification. To edit, please disable this and duplicate it')) From 4926c8298e7642dc0d50f212e5d7ba6d44b3944c Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 3 May 2021 19:42:20 +0530 Subject: [PATCH 03/46] fix: sider fix --- frappe/email/doctype/notification/notification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/email/doctype/notification/notification.py b/frappe/email/doctype/notification/notification.py index f0252f6dec..50f5bea073 100644 --- a/frappe/email/doctype/notification/notification.py +++ b/frappe/email/doctype/notification/notification.py @@ -66,7 +66,7 @@ def get_context(context): def validate_standard(self): # indicates that this is a new doc - if self._doc_before_save == None and not frappe.conf.developer_mode: + if self._doc_before_save is None and not frappe.conf.developer_mode: self.is_standard = False return if self.is_standard and not frappe.conf.developer_mode: From b7b92845f0a4d34f91b49b486535ea78e8681b21 Mon Sep 17 00:00:00 2001 From: David Angulo Date: Mon, 3 May 2021 13:38:17 -0500 Subject: [PATCH 04/46] fix: Use docfields from options if no docfields are returned from meta --- frappe/public/js/frappe/form/grid_row.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/grid_row.js b/frappe/public/js/frappe/form/grid_row.js index f6da88df57..453b8b5f24 100644 --- a/frappe/public/js/frappe/form/grid_row.js +++ b/frappe/public/js/frappe/form/grid_row.js @@ -7,7 +7,8 @@ export default class GridRow { $.extend(this, opts); if (this.doc && this.parent_df.options) { frappe.meta.make_docfield_copy_for(this.parent_df.options, this.doc.name, this.docfields); - this.docfields = frappe.meta.get_docfields(this.parent_df.options, this.doc.name); + const docfields = frappe.meta.get_docfields(this.parent_df.options, this.doc.name); + this.docfields = docfields.length ? docfields : opts.docfields; } this.columns = {}; this.columns_list = []; From 432047ecd00789066e59513ac6f90186de30ba52 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Tue, 4 May 2021 11:26:09 +0530 Subject: [PATCH 05/46] fix: Using No Copy for is_standard field --- frappe/email/doctype/notification/notification.json | 5 +++-- frappe/email/doctype/notification/notification.py | 4 ---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/frappe/email/doctype/notification/notification.json b/frappe/email/doctype/notification/notification.json index c1c877efd4..8b6900a3c9 100644 --- a/frappe/email/doctype/notification/notification.json +++ b/frappe/email/doctype/notification/notification.json @@ -102,7 +102,8 @@ "default": "0", "fieldname": "is_standard", "fieldtype": "Check", - "label": "Is Standard" + "label": "Is Standard", + "no_copy": 1 }, { "depends_on": "is_standard", @@ -281,7 +282,7 @@ "icon": "fa fa-envelope", "index_web_pages_for_search": 1, "links": [], - "modified": "2020-11-24 14:25:43.245677", + "modified": "2021-05-04 11:17:11.882314", "modified_by": "Administrator", "module": "Email", "name": "Notification", diff --git a/frappe/email/doctype/notification/notification.py b/frappe/email/doctype/notification/notification.py index 50f5bea073..2940a34f63 100644 --- a/frappe/email/doctype/notification/notification.py +++ b/frappe/email/doctype/notification/notification.py @@ -65,10 +65,6 @@ def get_context(context): """) def validate_standard(self): - # indicates that this is a new doc - if self._doc_before_save is None and not frappe.conf.developer_mode: - self.is_standard = False - return if self.is_standard and not frappe.conf.developer_mode: frappe.throw(_('Cannot edit Standard Notification. To edit, please disable this and duplicate it')) From adbf267212c02a014efb0108e660dd5e9fb9b963 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 5 May 2021 11:11:31 +0530 Subject: [PATCH 06/46] feat(DX): Add __repr__ and __str__ for DocTypes - Show doctype and name - if docstatus != 0, show docstatus - if child doctype, show parent --- frappe/model/document.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/frappe/model/document.py b/frappe/model/document.py index 4169919091..623916597e 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1347,6 +1347,22 @@ class Document(BaseDocument): from frappe.desk.doctype.tag.tag import DocTags return DocTags(self.doctype).get_tags(self.name).split(",")[1:] + def __repr__(self): + name = self.name or "unsaved" + doctype = self.__class__.__name__ + + docstatus = f" docstatus={self.docstatus}" if self.docstatus else "" + parent = f" parent={self.parent}" if self.parent else "" + + return f"<{doctype}: {name}{docstatus}{parent}>" + + def __str__(self): + name = self.name or "unsaved" + doctype = self.__class__.__name__ + + return f"{doctype}({name})" + + def execute_action(doctype, name, action, **kwargs): """Execute an action on a document (called by background worker)""" doc = frappe.get_doc(doctype, name) From 5de39e468e8f6b5d74132f70ca66b26f00c67242 Mon Sep 17 00:00:00 2001 From: leela Date: Thu, 29 Apr 2021 06:34:00 +0530 Subject: [PATCH 07/46] refactor: Add Email Account link into Email Queue --- frappe/email/doctype/email_queue/email_queue.json | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/frappe/email/doctype/email_queue/email_queue.json b/frappe/email/doctype/email_queue/email_queue.json index 4529ea8211..f251786c90 100644 --- a/frappe/email/doctype/email_queue/email_queue.json +++ b/frappe/email/doctype/email_queue/email_queue.json @@ -24,7 +24,8 @@ "unsubscribe_method", "expose_recipients", "attachments", - "retry" + "retry", + "email_account" ], "fields": [ { @@ -139,13 +140,19 @@ "fieldtype": "Int", "label": "Retry", "read_only": 1 + }, + { + "fieldname": "email_account", + "fieldtype": "Link", + "label": "Email Account", + "options": "Email Account" } ], "icon": "fa fa-envelope", "idx": 1, "in_create": 1, "links": [], - "modified": "2020-07-17 15:58:15.369419", + "modified": "2021-04-29 06:33:25.191729", "modified_by": "Administrator", "module": "Email", "name": "Email Queue", From 59bfc12da6fe077f2c3cfd9dfc527de3aa95b43a Mon Sep 17 00:00:00 2001 From: leela Date: Thu, 29 Apr 2021 21:41:33 +0530 Subject: [PATCH 08/46] refactor: Cleaned Email Queue sendmail functionality * Sending mail works independently * You can send a mail by calling Queue_doc.send() * Used context manager to track exceptions while sending mails --- frappe/__init__.py | 6 + .../doctype/email_account/email_account.py | 61 ++-- .../email/doctype/email_queue/email_queue.py | 260 ++++++++++++++++- .../email_queue_recipient.py | 14 +- frappe/email/queue.py | 263 ++---------------- frappe/email/smtp.py | 151 +++++----- frappe/email/test_email_body.py | 13 +- frappe/email/test_smtp.py | 2 +- 8 files changed, 402 insertions(+), 368 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 2436692c81..26f8b9bfa4 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1620,6 +1620,12 @@ def enqueue(*args, **kwargs): import frappe.utils.background_jobs return frappe.utils.background_jobs.enqueue(*args, **kwargs) +def task(**task_kwargs): + def decorator_task(f): + f.enqueue = lambda **fun_kwargs: enqueue(f, **task_kwargs, **fun_kwargs) + return f + return decorator_task + def enqueue_doc(*args, **kwargs): ''' Enqueue method to be executed using a background worker diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 3aa7c10ea5..36b662bb39 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -35,9 +35,6 @@ OUTGOING_EMAIL_ACCOUNT_MISSING = _("Please setup default Email Account from Setu class SentEmailInInbox(Exception): pass -class InvalidEmailCredentials(frappe.ValidationError): - pass - def cache_email_account(cache_name): def decorator_cache_email_account(func): @functools.wraps(func) @@ -100,9 +97,8 @@ class EmailAccount(Document): self.get_incoming_server() self.no_failed = 0 - if self.enable_outgoing: - self.check_smtp() + self.validate_smtp_conn() else: if self.enable_incoming or (self.enable_outgoing and not self.no_smtp_authentication): frappe.throw(_("Password is required or select Awaiting Password")) @@ -118,6 +114,13 @@ class EmailAccount(Document): if self.append_to not in valid_doctypes: frappe.throw(_("Append To can be one of {0}").format(comma_or(valid_doctypes))) + def validate_smtp_conn(self): + if not self.smtp_server: + frappe.throw(_("SMTP Server is required")) + + server = self.get_smtp_server() + return server.session + def before_save(self): messages = [] as_list = 1 @@ -179,24 +182,6 @@ class EmailAccount(Document): except Exception: pass - def check_smtp(self): - """Checks SMTP settings.""" - if self.enable_outgoing: - if not self.smtp_server: - frappe.throw(_("{0} is required").format("SMTP Server")) - - server = SMTPServer( - login = getattr(self, "login_id", None) or self.email_id, - server=self.smtp_server, - port=cint(self.smtp_port), - use_tls=cint(self.use_tls), - use_ssl=cint(self.use_ssl_for_outgoing) - ) - if self.password and not self.no_smtp_authentication: - server.password = self.get_password() - - server.sess - def get_incoming_server(self, in_receive=False, email_sync_rule="UNSEEN"): """Returns logged in POP3/IMAP connection object.""" if frappe.cache().get_value("workers:no-internet") == True: @@ -259,7 +244,7 @@ class EmailAccount(Document): return None elif not in_receive and any(map(lambda t: t in message, auth_error_codes)): - self.throw_invalid_credentials_exception() + SMTPServer.throw_invalid_credentials_exception() else: frappe.throw(cstr(e)) @@ -279,20 +264,18 @@ class EmailAccount(Document): @property def _password(self): - raise_exception = not self.no_smtp_authentication + raise_exception = not (self.no_smtp_authentication or frappe.flags.in_test) return self.get_password(raise_exception=raise_exception) @property def default_sender(self): return email.utils.formataddr((self.name, self.get("email_id"))) - @classmethod - def throw_invalid_credentials_exception(cls): - frappe.throw( - _("Incorrect email or password. Please check your login credentials."), - exc=InvalidEmailCredentials, - title=_("Invalid Credentials") - ) + def is_exists_in_db(self): + """Some of the Email Accounts we create from configs and those doesn't exists in DB. + This is is to check the specific email account exists in DB or not. + """ + return self.find_one_by_filters(name=self.name) @classmethod def from_record(cls, record): @@ -402,6 +385,20 @@ class EmailAccount(Document): account_details[doc_field_name] = (value and value[0]) or default return account_details + def sendmail_config(self): + return { + 'server': self.smtp_server, + 'port': cint(self.smtp_port), + 'login': getattr(self, "login_id", None) or self.email_id, + 'password': self._password, + 'use_ssl': cint(self.use_ssl_for_outgoing), + 'use_tls': cint(self.use_tls) + } + + def get_smtp_server(self): + config = self.sendmail_config() + return SMTPServer(**config) + def handle_incoming_connect_error(self, description): if test_internet(): if self.get_failed_attempts_count() > 2: diff --git a/frappe/email/doctype/email_queue/email_queue.py b/frappe/email/doctype/email_queue/email_queue.py index 267fbdfe9c..076dfc5417 100644 --- a/frappe/email/doctype/email_queue/email_queue.py +++ b/frappe/email/doctype/email_queue/email_queue.py @@ -2,15 +2,26 @@ # Copyright (c) 2015, Frappe Technologies and contributors # For license information, please see license.txt -from __future__ import unicode_literals +import traceback +import json + +from rq.timeouts import JobTimeoutException +import smtplib +import quopri +from email.parser import Parser + import frappe -from frappe import _ +from frappe import _, safe_encode, task from frappe.model.document import Document -from frappe.email.queue import send_one -from frappe.utils import now_datetime - +from frappe.email.queue import get_unsubcribed_url +from frappe.email.email_body import add_attachment +from frappe.utils import cint +from email.policy import SMTPUTF8 +MAX_RETRY_COUNT = 3 class EmailQueue(Document): + DOCTYPE = 'Email Queue' + def set_recipients(self, recipients): self.set("recipients", []) for r in recipients: @@ -30,6 +41,241 @@ class EmailQueue(Document): duplicate.set_recipients(recipients) return duplicate + @classmethod + def find(cls, name): + return frappe.get_doc(cls.DOCTYPE, name) + + def update_db(self, commit=False, **kwargs): + frappe.db.set_value(self.DOCTYPE, self.name, kwargs) + if commit: + frappe.db.commit() + + def update_status(self, status, commit=False, **kwargs): + self.update_db(status = status, commit = commit, **kwargs) + if self.communication: + communication_doc = frappe.get_doc('Communication', self.communication) + communication_doc.set_delivery_status(commit=commit) + + @property + def cc(self): + return (self.show_as_cc and self.show_as_cc.split(",")) or [] + + @property + def to(self): + return [r.recipient for r in self.recipients if r.recipient not in self.cc] + + @property + def attachments_list(self): + return json.loads(self.attachments) if self.attachments else [] + + def get_email_account(self): + from frappe.email.doctype.email_account.email_account import EmailAccount + + if self.email_account: + return frappe.get_doc('Email Account', self.email_account) + + return EmailAccount.find_outgoing( + match_by_email = self.sender, match_by_doctype = self.reference_doctype) + + def is_to_be_sent(self): + return self.status in ['Not Sent','Partially Sent'] + + def can_send_now(self): + hold_queue = (cint(frappe.defaults.get_defaults().get("hold_queue"))==1) + if frappe.are_emails_muted() or not self.is_to_be_sent() or hold_queue: + return False + + return True + + def send(self, is_background_task=False): + """ Send emails to recipients. + """ + if not self.can_send_now(): + frappe.db.rollback() + return + + with SendMailContext(self, is_background_task) as ctx: + message = None + for recipient in self.recipients: + if not recipient.is_mail_to_be_sent(): + continue + + message = ctx.build_message(recipient.recipient) + if not frappe.flags.in_test: + ctx.smtp_session.sendmail(recipient.recipient, self.sender, message) + ctx.add_to_sent_list(recipient) + + if frappe.flags.in_test: + frappe.flags.sent_mail = message + return + + if ctx.email_account_doc.append_emails_to_sent_folder and ctx.sent_to: + ctx.email_account_doc.append_email_to_sent_folder(message) + + +@task(queue = 'short') +def send_mail(email_queue_name, is_background_task=False): + """This is equalent to EmqilQueue.send. + + This provides a way to make sending mail as a background job. + """ + record = EmailQueue.find(email_queue_name) + record.send(is_background_task=is_background_task) + +class SendMailContext: + def __init__(self, queue_doc: Document, is_background_task: bool = False): + self.queue_doc = queue_doc + self.is_background_task = is_background_task + self.email_account_doc = queue_doc.get_email_account() + self.smtp_server = self.email_account_doc.get_smtp_server() + self.sent_to = [rec.recipient for rec in self.queue_doc.recipients if rec.is_main_sent()] + + def __enter__(self): + self.queue_doc.update_status(status='Sending', commit=True) + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + exceptions = [ + smtplib.SMTPServerDisconnected, + smtplib.SMTPAuthenticationError, + smtplib.SMTPRecipientsRefused, + smtplib.SMTPConnectError, + smtplib.SMTPHeloError, + JobTimeoutException + ] + + self.smtp_server.quit() + self.log_exception(exc_type, exc_val, exc_tb) + + if exc_type in exceptions: + email_status = (self.sent_to and 'Partially Sent') or 'Not Sent' + self.queue_doc.update_status(status = email_status, commit = True) + elif exc_type: + if self.queue_doc.retry < MAX_RETRY_COUNT: + update_fields = {'status': 'Not Sent', 'retry': self.queue_doc.retry + 1} + else: + update_fields = {'status': (self.sent_to and 'Partially Errored') or 'Error'} + self.queue_doc.update_status(**update_fields, commit = True) + else: + email_status = self.is_mail_sent_to_all() and 'Sent' + email_status = email_status or (self.sent_to and 'Partially Sent') or 'Not Sent' + self.queue_doc.update_status(status = email_status, commit = True) + + def log_exception(self, exc_type, exc_val, exc_tb): + if exc_type: + traceback_string = "".join(traceback.format_tb(exc_tb)) + traceback_string += f"\n Queue Name: {self.queue_doc.name}" + + if self.is_background_task: + frappe.log_error(title = 'frappe.email.queue.flush', message = traceback_string) + else: + frappe.log_error(message = traceback_string) + + @property + def smtp_session(self): + if frappe.flags.in_test: + return + return self.smtp_server.session + + def add_to_sent_list(self, recipient): + # Update recipient status + recipient.update_db(status='Sent', commit=True) + self.sent_to.append(recipient.recipient) + + def is_mail_sent_to_all(self): + return sorted(self.sent_to) == sorted([rec.recipient for rec in self.queue_doc.recipients]) + + def get_message_object(self, message): + return Parser(policy=SMTPUTF8).parsestr(message) + + def message_placeholder(self, placeholder_key): + map = { + 'tracker': '', + 'unsubscribe_url': '', + 'cc': '', + 'recipient': '', + } + return map.get(placeholder_key) + + def build_message(self, recipient_email): + """Build message specific to the recipient. + """ + message = self.queue_doc.message + if not message: + return "" + + message = message.replace(self.message_placeholder('tracker'), self.get_tracker_str()) + message = message.replace(self.message_placeholder('unsubscribe_url'), + self.get_unsubscribe_str(recipient_email)) + message = message.replace(self.message_placeholder('cc'), self.get_receivers_str()) + message = message.replace(self.message_placeholder('recipient'), + self.get_receipient_str(recipient_email)) + message = self.include_attachments(message) + return message + + def get_tracker_str(self): + tracker_url_html = \ + '' + + message = '' + if frappe.conf.use_ssl and self.queue_doc.track_email_status: + message = quopri.encodestring( + tracker_url_html.format(frappe.local.site, self.queue_doc.communication).encode() + ).decode() + return message + + def get_unsubscribe_str(self, recipient_email): + unsubscribe_url = '' + if self.queue_doc.add_unsubscribe_link and self.queue_doc.reference_doctype: + doctype, doc_name = self.queue_doc.reference_doctype, self.queue_doc.reference_name + unsubscribe_url = get_unsubcribed_url(doctype, doc_name, recipient_email, + self.queue_doc.unsubscribe_method, self.queue_doc.unsubscribe_param) + + return quopri.encodestring(unsubscribe_url.encode()).decode() + + def get_receivers_str(self): + message = '' + if self.queue_doc.expose_recipients == "footer": + to_str = ', '.join(self.queue_doc.to) + cc_str = ', '.join(self.queue_doc.cc) + message = f"This email was sent to {to_str}" + message = message + f" and copied to {cc_str}" if cc_str else message + return message + + def get_receipient_str(self, recipient_email): + message = '' + if self.queue_doc.expose_recipients != "header": + message = recipient_email + return message + + def include_attachments(self, message): + message_obj = self.get_message_object(message) + attachments = self.queue_doc.attachments_list + + for attachment in attachments: + if attachment.get('fcontent'): + continue + + fid = attachment.get("fid") + if fid: + _file = frappe.get_doc("File", fid) + fcontent = _file.get_content() + attachment.update({ + 'fname': _file.file_name, + 'fcontent': fcontent, + 'parent': message_obj + }) + attachment.pop("fid", None) + add_attachment(**attachment) + + elif attachment.get("print_format_attachment") == 1: + attachment.pop("print_format_attachment", None) + print_format_file = frappe.attach_print(**attachment) + print_format_file.update({"parent": message_obj}) + add_attachment(**print_format_file) + + return safe_encode(message_obj.as_string()) + @frappe.whitelist() def retry_sending(name): doc = frappe.get_doc("Email Queue", name) @@ -42,7 +288,9 @@ def retry_sending(name): @frappe.whitelist() def send_now(name): - send_one(name, now=True) + record = EmailQueue.find(name) + if record: + record.send() def on_doctype_update(): """Add index in `tabCommunication` for `(reference_doctype, reference_name)`""" diff --git a/frappe/email/doctype/email_queue_recipient/email_queue_recipient.py b/frappe/email/doctype/email_queue_recipient/email_queue_recipient.py index 42956a1180..3f07ec58f3 100644 --- a/frappe/email/doctype/email_queue_recipient/email_queue_recipient.py +++ b/frappe/email/doctype/email_queue_recipient/email_queue_recipient.py @@ -7,4 +7,16 @@ import frappe from frappe.model.document import Document class EmailQueueRecipient(Document): - pass + DOCTYPE = 'Email Queue Recipient' + + def is_mail_to_be_sent(self): + return self.status == 'Not Sent' + + def is_main_sent(self): + return self.status == 'Sent' + + def update_db(self, commit=False, **kwargs): + frappe.db.set_value(self.DOCTYPE, self.name, kwargs) + if commit: + frappe.db.commit() + diff --git a/frappe/email/queue.py b/frappe/email/queue.py index cd984e9bf9..52c91baf1c 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -173,19 +173,19 @@ def add(recipients, sender, subject, **kwargs): if not email_queue: email_queue = get_email_queue([r], sender, subject, **kwargs) if kwargs.get('now'): - send_one(email_queue.name, now=True) + email_queue.send() else: duplicate = email_queue.get_duplicate([r]) duplicate.insert(ignore_permissions=True) if kwargs.get('now'): - send_one(duplicate.name, now=True) + duplicate.send() frappe.db.commit() else: email_queue = get_email_queue(recipients, sender, subject, **kwargs) if kwargs.get('now'): - send_one(email_queue.name, now=True) + email_queue.send() def get_email_queue(recipients, sender, subject, **kwargs): '''Make Email Queue object''' @@ -237,6 +237,9 @@ def get_email_queue(recipients, sender, subject, **kwargs): ', '.join(mail.recipients), traceback.format_exc()), 'Email Not Sent') recipients = list(set(recipients + kwargs.get('cc', []) + kwargs.get('bcc', []))) + email_account = kwargs.get('email_account') + email_account_name = email_account and email_account.is_exists_in_db() and email_account.name + e.set_recipients(recipients) e.reference_doctype = kwargs.get('reference_doctype') e.reference_name = kwargs.get('reference_name') @@ -248,8 +251,8 @@ def get_email_queue(recipients, sender, subject, **kwargs): e.send_after = kwargs.get('send_after') e.show_as_cc = ",".join(kwargs.get('cc', [])) e.show_as_bcc = ",".join(kwargs.get('bcc', [])) + e.email_account = email_account_name or None e.insert(ignore_permissions=True) - return e def get_emails_sent_this_month(): @@ -331,44 +334,25 @@ def return_unsubscribed_page(email, doctype, name): indicator_color='green') def flush(from_test=False): - """flush email queue, every time: called from scheduler""" - # additional check - - auto_commit = not from_test + """flush email queue, every time: called from scheduler + """ + from frappe.email.doctype.email_queue.email_queue import send_mail + # To avoid running jobs inside unit tests if frappe.are_emails_muted(): msgprint(_("Emails are muted")) from_test = True - smtpserver_dict = frappe._dict() + if cint(frappe.defaults.get_defaults().get("hold_queue"))==1: + return - for email in get_queue(): + for row in get_queue(): + try: + func = send_mail if from_test else send_mail.enqueue + is_background_task = not from_test + func(email_queue_name = row.name, is_background_task = is_background_task) + except Exception: + frappe.log_error() - if cint(frappe.defaults.get_defaults().get("hold_queue"))==1: - break - - if email.name: - smtpserver = smtpserver_dict.get(email.sender) - if not smtpserver: - smtpserver = SMTPServer() - smtpserver_dict[email.sender] = smtpserver - - if from_test: - send_one(email.name, smtpserver, auto_commit) - else: - send_one_args = { - 'email': email.name, - 'smtpserver': smtpserver, - 'auto_commit': auto_commit, - } - enqueue( - method = 'frappe.email.queue.send_one', - queue = 'short', - **send_one_args - ) - - # NOTE: removing commit here because we pass auto_commit - # finally: - # frappe.db.commit() def get_queue(): return frappe.db.sql('''select name, sender @@ -381,213 +365,6 @@ def get_queue(): by priority desc, creation asc limit 500''', { 'now': now_datetime() }, as_dict=True) - -def send_one(email, smtpserver=None, auto_commit=True, now=False): - '''Send Email Queue with given smtpserver''' - - email = frappe.db.sql('''select - name, status, communication, message, sender, reference_doctype, - reference_name, unsubscribe_param, unsubscribe_method, expose_recipients, - show_as_cc, add_unsubscribe_link, attachments, retry - from - `tabEmail Queue` - where - name=%s - for update''', email, as_dict=True) - - if len(email): - email = email[0] - else: - return - - recipients_list = frappe.db.sql('''select name, recipient, status from - `tabEmail Queue Recipient` where parent=%s''', email.name, as_dict=1) - - if frappe.are_emails_muted(): - frappe.msgprint(_("Emails are muted")) - return - - if cint(frappe.defaults.get_defaults().get("hold_queue"))==1 : - return - - if email.status not in ('Not Sent','Partially Sent') : - # rollback to release lock and return - frappe.db.rollback() - return - - frappe.db.sql("""update `tabEmail Queue` set status='Sending', modified=%s where name=%s""", - (now_datetime(), email.name), auto_commit=auto_commit) - - if email.communication: - frappe.get_doc('Communication', email.communication).set_delivery_status(commit=auto_commit) - - email_sent_to_any_recipient = None - - try: - message = None - - if not frappe.flags.in_test: - if not smtpserver: - smtpserver = SMTPServer() - - # to avoid always using default email account for outgoing - if getattr(frappe.local, "outgoing_email_account", None): - frappe.local.outgoing_email_account = {} - - smtpserver.setup_email_account(email.reference_doctype, sender=email.sender) - - for recipient in recipients_list: - if recipient.status != "Not Sent": - continue - - message = prepare_message(email, recipient.recipient, recipients_list) - if not frappe.flags.in_test: - smtpserver.sess.sendmail(email.sender, recipient.recipient, message) - - recipient.status = "Sent" - frappe.db.sql("""update `tabEmail Queue Recipient` set status='Sent', modified=%s where name=%s""", - (now_datetime(), recipient.name), auto_commit=auto_commit) - - email_sent_to_any_recipient = any("Sent" == s.status for s in recipients_list) - - #if all are sent set status - if email_sent_to_any_recipient: - frappe.db.sql("""update `tabEmail Queue` set status='Sent', modified=%s where name=%s""", - (now_datetime(), email.name), auto_commit=auto_commit) - else: - frappe.db.sql("""update `tabEmail Queue` set status='Error', error=%s - where name=%s""", ("No recipients to send to", email.name), auto_commit=auto_commit) - if frappe.flags.in_test: - frappe.flags.sent_mail = message - return - if email.communication: - frappe.get_doc('Communication', email.communication).set_delivery_status(commit=auto_commit) - - if smtpserver.append_emails_to_sent_folder and email_sent_to_any_recipient: - smtpserver.email_account.append_email_to_sent_folder(message) - - except (smtplib.SMTPServerDisconnected, - smtplib.SMTPConnectError, - smtplib.SMTPHeloError, - smtplib.SMTPAuthenticationError, - smtplib.SMTPRecipientsRefused, - JobTimeoutException): - - # bad connection/timeout, retry later - - if email_sent_to_any_recipient: - frappe.db.sql("""update `tabEmail Queue` set status='Partially Sent', modified=%s where name=%s""", - (now_datetime(), email.name), auto_commit=auto_commit) - else: - frappe.db.sql("""update `tabEmail Queue` set status='Not Sent', modified=%s where name=%s""", - (now_datetime(), email.name), auto_commit=auto_commit) - - if email.communication: - frappe.get_doc('Communication', email.communication).set_delivery_status(commit=auto_commit) - - # no need to attempt further - return - - except Exception as e: - frappe.db.rollback() - - if email.retry < 3: - frappe.db.sql("""update `tabEmail Queue` set status='Not Sent', modified=%s, retry=retry+1 where name=%s""", - (now_datetime(), email.name), auto_commit=auto_commit) - else: - if email_sent_to_any_recipient: - frappe.db.sql("""update `tabEmail Queue` set status='Partially Errored', error=%s where name=%s""", - (text_type(e), email.name), auto_commit=auto_commit) - else: - frappe.db.sql("""update `tabEmail Queue` set status='Error', error=%s - where name=%s""", (text_type(e), email.name), auto_commit=auto_commit) - - if email.communication: - frappe.get_doc('Communication', email.communication).set_delivery_status(commit=auto_commit) - - if now: - print(frappe.get_traceback()) - raise e - - else: - # log to Error Log - frappe.log_error('frappe.email.queue.flush') - -def prepare_message(email, recipient, recipients_list): - message = email.message - if not message: - return "" - - # Parse "Email Account" from "Email Sender" - email_account = EmailAccount.find_outgoing(match_by_email=email.sender) - if frappe.conf.use_ssl and email_account.track_email_status: - # Using SSL => Publically available domain => Email Read Reciept Possible - message = message.replace("", quopri.encodestring(''.format(frappe.local.site, email.communication).encode()).decode()) - else: - # No SSL => No Email Read Reciept - message = message.replace("", quopri.encodestring("".encode()).decode()) - - if email.add_unsubscribe_link and email.reference_doctype: # is missing the check for unsubscribe message but will not add as there will be no unsubscribe url - unsubscribe_url = get_unsubcribed_url(email.reference_doctype, email.reference_name, recipient, - email.unsubscribe_method, email.unsubscribe_params) - message = message.replace("", quopri.encodestring(unsubscribe_url.encode()).decode()) - - if email.expose_recipients == "header": - pass - else: - if email.expose_recipients == "footer": - if isinstance(email.show_as_cc, string_types): - email.show_as_cc = email.show_as_cc.split(",") - email_sent_to = [r.recipient for r in recipients_list] - email_sent_cc = ", ".join([e for e in email_sent_to if e in email.show_as_cc]) - email_sent_to = ", ".join([e for e in email_sent_to if e not in email.show_as_cc]) - - if email_sent_cc: - email_sent_message = _("This email was sent to {0} and copied to {1}").format(email_sent_to,email_sent_cc) - else: - email_sent_message = _("This email was sent to {0}").format(email_sent_to) - message = message.replace("", quopri.encodestring(email_sent_message.encode()).decode()) - - message = message.replace("", recipient) - - message = (message and message.encode('utf8')) or '' - message = safe_decode(message) - - if PY3: - from email.policy import SMTPUTF8 - message = Parser(policy=SMTPUTF8).parsestr(message) - else: - message = Parser().parsestr(message) - - if email.attachments: - # On-demand attachments - - attachments = json.loads(email.attachments) - - for attachment in attachments: - if attachment.get('fcontent'): - continue - - fid = attachment.get("fid") - if fid: - _file = frappe.get_doc("File", fid) - fcontent = _file.get_content() - attachment.update({ - 'fname': _file.file_name, - 'fcontent': fcontent, - 'parent': message - }) - attachment.pop("fid", None) - add_attachment(**attachment) - - elif attachment.get("print_format_attachment") == 1: - attachment.pop("print_format_attachment", None) - print_format_file = frappe.attach_print(**attachment) - print_format_file.update({"parent": message}) - add_attachment(**print_format_file) - - return safe_encode(message.as_string()) - def clear_outbox(days=None): """Remove low priority older than 31 days in Outbox or configured in Log Settings. Note: Used separate query to avoid deadlock diff --git a/frappe/email/smtp.py b/frappe/email/smtp.py index ca69e621cc..3acb76af23 100644 --- a/frappe/email/smtp.py +++ b/frappe/email/smtp.py @@ -9,11 +9,24 @@ import _socket, sys from frappe import _ from frappe.utils import cint, cstr, parse_addr +CONNECTION_FAILED = _('Could not connect to outgoing email server') +AUTH_ERROR_TITLE = _("Invalid Credentials") +AUTH_ERROR = _("Incorrect email or password. Please check your login credentials.") +SOCKET_ERROR_TITLE = _("Incorrect Configuration") +SOCKET_ERROR = _("Invalid Outgoing Mail Server or Port") +SEND_MAIL_FAILED = _("Unable to send emails at this time") +EMAIL_ACCOUNT_MISSING = _('Email Account not setup. Please create a new Email Account from Setup > Email > Email Account') + +class InvalidEmailCredentials(frappe.ValidationError): + pass + def send(email, append_to=None, retry=1): """Deprecated: Send the message or add it to Outbox Email""" def _send(retry): + from frappe.email.doctype.email_account.email_account import EmailAccount try: - smtpserver = SMTPServer(append_to=append_to) + email_account = EmailAccount.find_outgoing(match_by_doctype=append_to) + smtpserver = email_account.get_smtp_server() # validate is called in as_string email_body = email.as_string() @@ -34,102 +47,80 @@ def send(email, append_to=None, retry=1): _send(retry) - class SMTPServer: - def __init__(self, login=None, password=None, server=None, port=None, use_tls=None, use_ssl=None, append_to=None): - # get defaults from mail settings + def __init__(self, server, login=None, password=None, port=None, use_tls=None, use_ssl=None): + self.login = login + self.password = password + self._server = server + self._port = port + self.use_tls = use_tls + self.use_ssl = use_ssl + self._session = None - self._sess = None - self.email_account = None - self.server = None - self.append_emails_to_sent_folder = None - - if server: - self.server = server - self.port = port - self.use_tls = cint(use_tls) - self.use_ssl = cint(use_ssl) - self.login = login - self.password = password - - else: - self.setup_email_account(append_to) - - def setup_email_account(self, append_to=None, sender=None): - from frappe.email.doctype.email_account.email_account import EmailAccount - self.email_account = EmailAccount.find_outgoing(match_by_doctype=append_to, match_by_email=sender) - if self.email_account: - self.server = self.email_account.smtp_server - self.login = (getattr(self.email_account, "login_id", None) or self.email_account.email_id) - if self.email_account.no_smtp_authentication or frappe.local.flags.in_test: - self.password = None - else: - self.password = self.email_account._password - self.port = self.email_account.smtp_port - self.use_tls = self.email_account.use_tls - self.sender = self.email_account.email_id - self.use_ssl = self.email_account.use_ssl_for_outgoing - self.append_emails_to_sent_folder = self.email_account.append_emails_to_sent_folder - self.always_use_account_email_id_as_sender = cint(self.email_account.get("always_use_account_email_id_as_sender")) - self.always_use_account_name_as_sender_name = cint(self.email_account.get("always_use_account_name_as_sender_name")) + if not self.server: + frappe.msgprint(EMAIL_ACCOUNT_MISSING, raise_exception=frappe.OutgoingEmailError) @property - def sess(self): - """get session""" - if self._sess: - return self._sess + def port(self): + port = self._port or (self.use_ssl and 465) or (self.use_tls and 587) + return cint(port) - # check if email server specified - if not getattr(self, 'server'): - err_msg = _('Email Account not setup. Please create a new Email Account from Setup > Email > Email Account') - frappe.msgprint(err_msg) - raise frappe.OutgoingEmailError(err_msg) + @property + def server(self): + return cstr(self._server or "") + + def secure_session(self, conn): + """Secure the connection incase of TLS. + """ + if self.use_tls: + conn.ehlo() + conn.starttls() + conn.ehlo() + + @property + def session(self): + if self.is_session_active(): + return self._session + + SMTP = smtplib.SMTP_SSL if self.use_ssl else smtplib.SMTP try: - if self.use_ssl: - if not self.port: - self.port = 465 - - self._sess = smtplib.SMTP_SSL((self.server or ""), cint(self.port)) - else: - if self.use_tls and not self.port: - self.port = 587 - - self._sess = smtplib.SMTP(cstr(self.server or ""), - cint(self.port) or None) - - if not self._sess: - err_msg = _('Could not connect to outgoing email server') - frappe.msgprint(err_msg) - raise frappe.OutgoingEmailError(err_msg) - - if self.use_tls: - self._sess.ehlo() - self._sess.starttls() - self._sess.ehlo() + self._session = SMTP(self.server, self.port) + if not self._session: + frappe.msgprint(CONNECTION_FAILED, raise_exception=frappe.OutgoingEmailError) + self.secure_session(self._session) if self.login and self.password: - ret = self._sess.login(str(self.login or ""), str(self.password or "")) + res = self._session.login(str(self.login or ""), str(self.password or "")) # check if logged correctly - if ret[0]!=235: - frappe.msgprint(ret[1]) - raise frappe.OutgoingEmailError(ret[1]) + if res[0]!=235: + frappe.msgprint(res[1], raise_exception=frappe.OutgoingEmailError) - return self._sess + return self._session except smtplib.SMTPAuthenticationError as e: - from frappe.email.doctype.email_account.email_account import EmailAccount - EmailAccount.throw_invalid_credentials_exception() + self.throw_invalid_credentials_exception() except _socket.error as e: # Invalid mail server -- due to refusing connection - frappe.throw( - _("Invalid Outgoing Mail Server or Port"), - exc=frappe.ValidationError, - title=_("Incorrect Configuration") - ) + frappe.throw(SOCKET_ERROR, title=SOCKET_ERROR_TITLE) except smtplib.SMTPException: - frappe.msgprint(_('Unable to send emails at this time')) + frappe.msgprint(SEND_MAIL_FAILED) raise + + def is_session_active(self): + if self._session: + try: + return self._session.noop()[0] == 250 + except Exception: + return False + + def quit(self): + if self.is_session_active(): + self._session.quit() + + @classmethod + def throw_invalid_credentials_exception(cls): + frappe.throw(AUTH_ERROR, title=AUTH_ERROR_TITLE, exc=InvalidEmailCredentials) diff --git a/frappe/email/test_email_body.py b/frappe/email/test_email_body.py index 3fcabb9495..33668cddba 100644 --- a/frappe/email/test_email_body.py +++ b/frappe/email/test_email_body.py @@ -7,10 +7,10 @@ from frappe import safe_decode from frappe.email.receive import Email from frappe.email.email_body import (replace_filename_with_cid, get_email, inline_style_in_html, get_header) -from frappe.email.queue import prepare_message, get_email_queue +from frappe.email.queue import get_email_queue +from frappe.email.doctype.email_queue.email_queue import SendMailContext from six import PY3 - class TestEmailBody(unittest.TestCase): def setUp(self): email_html = ''' @@ -57,7 +57,8 @@ This is the text version of this email content='

' + uni_chr1 + 'abcd' + uni_chr2 + '

', formatted='

' + uni_chr1 + 'abcd' + uni_chr2 + '

', text_content='whatever') - result = prepare_message(email=email, recipient='test@test.com', recipients_list=[]) + mail_ctx = SendMailContext(queue_doc = email) + result = mail_ctx.build_message(recipient_email = 'test@test.com') self.assertTrue(b"

=EA=80=80abcd=DE=B4

" in result) def test_prepare_message_returns_cr_lf(self): @@ -68,8 +69,10 @@ This is the text version of this email content='

\n this is a test of newlines\n' + '

', formatted='

\n this is a test of newlines\n' + '

', text_content='whatever') - result = safe_decode(prepare_message(email=email, - recipient='test@test.com', recipients_list=[])) + + mail_ctx = SendMailContext(queue_doc = email) + result = safe_decode(mail_ctx.build_message(recipient_email='test@test.com')) + if PY3: self.assertTrue(result.count('\n') == result.count("\r")) else: diff --git a/frappe/email/test_smtp.py b/frappe/email/test_smtp.py index e170617383..58e4fdd8a6 100644 --- a/frappe/email/test_smtp.py +++ b/frappe/email/test_smtp.py @@ -75,4 +75,4 @@ def make_server(port, ssl, tls): use_tls = tls ) - server.sess + server.session From 483cd85eba617624da4fd60c8b3d2a1181ce4ea3 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Thu, 6 May 2021 16:44:18 +0530 Subject: [PATCH 09/46] fix: Revert naming for custom naming series --- frappe/model/naming.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 1a3f90da37..1cfcd56350 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -202,7 +202,12 @@ def revert_series_if_last(key, name, doc=None): if ".#" in key: prefix, hashes = key.rsplit(".", 1) if "#" not in hashes: - return + key = key.rsplit(".") + hash = list(filter(re.compile(".*#").match, key))[0] + if not hash: + return + name = name.replace(hashes, "") + prefix, hashes = key[:key.index(hash)+1] else: prefix = key From 2548bca1870a0083cd0e57aa043c9060177d5b6a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 6 May 2021 16:48:59 +0530 Subject: [PATCH 10/46] fix: hook format for user_data_fields --- frappe/utils/boilerplate.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index ba20562544..f6a2ac488c 100755 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -288,24 +288,24 @@ app_license = "{app_license}" # -------------------- # user_data_fields = [ -# { +# {{ # "doctype": "{{doctype_1}}", # "filter_by": "{{filter_by}}", # "redact_fields": ["{{field_1}}", "{{field_2}}"], # "partial": 1, -# }, -# { +# }}, +# {{ # "doctype": "{{doctype_2}}", # "filter_by": "{{filter_by}}", # "partial": 1, -# }, -# { +# }}, +# {{ # "doctype": "{{doctype_3}}", # "strict": False, -# }, -# { +# }}, +# {{ # "doctype": "{{doctype_4}}" -# } +# }} # ] # Authentication and authorization From 9aa2f366dd6f51adde6fbd9be4fbc2b5c7a85ab9 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 20 Jan 2021 13:52:05 +0530 Subject: [PATCH 11/46] fix: Don't execute dynamic properties to check if conflicts exist --- frappe/core/doctype/doctype/doctype.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 3588cc553a..6f33221155 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1176,9 +1176,15 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): def check_if_fieldname_conflicts_with_methods(doctype, fieldname): doc = frappe.get_doc({"doctype": doctype}) - method_list = [method for method in dir(doc) if isinstance(method, str) and callable(getattr(doc, method))] + available_objects = [x for x in dir(doc) if isinstance(x, str)] + property_list = [ + x for x in available_objects if isinstance(getattr(type(doc), x, None), property) + ] + method_list = [ + x for x in available_objects if x not in property_list and callable(getattr(doc, x)) + ] - if fieldname in method_list: + if fieldname in method_list + property_list: frappe.throw(_("Fieldname {0} conflicting with meta object").format(fieldname)) def clear_linked_doctype_cache(): From 255a959a3eb706ab25845aa94890708c37fe7573 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 4 Jan 2021 10:41:18 +0530 Subject: [PATCH 12/46] chore: Rename function to validate conflicting methods and properties --- frappe/core/doctype/doctype/doctype.py | 2 +- frappe/custom/doctype/custom_field/custom_field.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 6f33221155..d277a217d1 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1174,7 +1174,7 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): else: raise -def check_if_fieldname_conflicts_with_methods(doctype, fieldname): +def check_if_fieldname_conflicts_with_properties_and_methods(doctype, fieldname): doc = frappe.get_doc({"doctype": doctype}) available_objects = [x for x in dir(doc) if isinstance(x, str)] property_list = [ diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index 3126326636..ac1213c727 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -64,8 +64,8 @@ class CustomField(Document): self.translatable = 0 if not self.flags.ignore_validate: - from frappe.core.doctype.doctype.doctype import check_if_fieldname_conflicts_with_methods - check_if_fieldname_conflicts_with_methods(self.dt, self.fieldname) + from frappe.core.doctype.doctype.doctype import check_if_fieldname_conflicts_with_properties_and_methods + check_if_fieldname_conflicts_with_properties_and_methods(self.dt, self.fieldname) def on_update(self): frappe.clear_cache(doctype=self.dt) From c652c7b7f52ad8d9a5b00b84af3f1ab33c3d1769 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 6 May 2021 11:58:17 +0530 Subject: [PATCH 13/46] feat: Validate field name conflicts in DocType.validate # Conflicts: # frappe/core/doctype/doctype/doctype.py --- frappe/core/doctype/doctype/doctype.py | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index d277a217d1..13ebcd82ac 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -70,6 +70,7 @@ class DocType(Document): validate_series(self) self.validate_document_type() validate_fields(self) + self.validate_field_name_conflicts() if not self.istable: validate_permissions(self) @@ -89,6 +90,39 @@ class DocType(Document): if self.default_print_format and not self.custom: frappe.throw(_('Standard DocType cannot have default print format, use Customize Form')) + if frappe.conf.get('developer_mode'): + self.owner = 'Administrator' + self.modified_by = 'Administrator' + + def validate_field_name_conflicts(self): + """Check if field names dont conflict with controller properties and methods""" + from frappe.model.base_document import get_controller + + controller = get_controller(self.name) + available_objects = {x for x in dir(controller) if isinstance(x, str)} + property_set = { + x for x in available_objects if isinstance(getattr(controller, x, None), property) + } + method_set = { + x for x in available_objects if x not in property_set and callable(getattr(controller, x, None)) + } + + for docfield in self.get("fields") or []: + conflict_type = "" + field = docfield.fieldname + field_label = docfield.label or docfield.fieldname + + if docfield.fieldname in method_set: + conflict_type = "controller method" + if docfield.fieldname in property_set: + conflict_type = "class property" + + if conflict_type: + frappe.throw( + _("Fieldname '{0}' conflicting with a {1} of the name {2} in {3}") + .format(field_label, conflict_type, field, self.name) + ) + def after_insert(self): # clear user cache so that on the next reload this doctype is included in boot clear_user_cache(frappe.session.user) From 78e1297392706eaa209e53d3acb42faae66dfa65 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 22 Jan 2021 18:08:18 +0530 Subject: [PATCH 14/46] chore: Drop dead file --- frappe/email/doctype/newsletter/newsletter..json | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 frappe/email/doctype/newsletter/newsletter..json diff --git a/frappe/email/doctype/newsletter/newsletter..json b/frappe/email/doctype/newsletter/newsletter..json deleted file mode 100644 index e69de29bb2..0000000000 From ce4253c6318546a7249f0edd518ad1f499f8fd88 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 25 Jan 2021 13:55:10 +0530 Subject: [PATCH 15/46] fix: Ignore 'special' DocTypes to avoid breaking changes eg: Fieldname 'Delete' conflicting with a controller method of the name delete in Custom DocPerm Fieldname 'Delete' conflicting with a controller method of the name delete in DocPerm Fieldname 'Precision' conflicting with a controller method of the name precision in Custom Field Fieldname 'Precision' conflicting with a controller method of the name precision in Customize Form Field Fieldname 'Precision' conflicting with a controller method of the name precision in DocField --- frappe/core/doctype/doctype/doctype.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 13ebcd82ac..c1a8527512 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -96,6 +96,17 @@ class DocType(Document): def validate_field_name_conflicts(self): """Check if field names dont conflict with controller properties and methods""" + core_doctypes = [ + "Custom DocPerm", + "DocPerm", + "Custom Field", + "Customize Form Field", + "DocField", + ] + + if self.name in core_doctypes: + return + from frappe.model.base_document import get_controller controller = get_controller(self.name) From a3b79081d64e754d73fad46f26aa24ac42a32a2e Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Sat, 18 Jan 2020 04:11:19 +0530 Subject: [PATCH 16/46] fix: Use Document in case get_controller raises import errors --- frappe/core/doctype/doctype/doctype.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index c1a8527512..d361bb8a98 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -109,7 +109,11 @@ class DocType(Document): from frappe.model.base_document import get_controller - controller = get_controller(self.name) + try: + controller = get_controller(self.name) + except ImportError: + controller = Document + available_objects = {x for x in dir(controller) if isinstance(x, str)} property_set = { x for x in available_objects if isinstance(getattr(controller, x, None), property) From 05712abc60721f4633d6b0166648743154b56cca Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 18 Feb 2021 19:55:12 +0530 Subject: [PATCH 17/46] fix: Check for db value if cache doesn't exist in get_controller, if cached value doesn't exist for a DocType in the frappe.db.value_cache, then query the db as fallback before the final fallback of assuming module as Core and non custom --- frappe/model/base_document.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 05435482bd..9288d621c9 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -34,8 +34,11 @@ def get_controller(doctype): from frappe.model.document import Document from frappe.utils.nestedset import NestedSet - module_name, custom = frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) \ + module_name, custom = ( + frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) + or frappe.db.get_value("DocType", doctype, ("module", "custom")) or ["Core", False] + ) if custom: if frappe.db.field_exists("DocType", "is_tree"): From 843c544117e7f661cc97cc78c33349833b12df21 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 16 Mar 2021 12:23:10 +0530 Subject: [PATCH 18/46] refactor: Rename function and add docstring Co-authored-by: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> --- frappe/core/doctype/doctype/doctype.py | 4 +++- frappe/custom/doctype/custom_field/custom_field.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index d361bb8a98..2cdccda8e1 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -1223,7 +1223,9 @@ def make_module_and_roles(doc, perm_fieldname="permissions"): else: raise -def check_if_fieldname_conflicts_with_properties_and_methods(doctype, fieldname): +def check_fieldname_conflicts(doctype, fieldname): + """Checks if fieldname conflicts with methods or properties""" + doc = frappe.get_doc({"doctype": doctype}) available_objects = [x for x in dir(doc) if isinstance(x, str)] property_list = [ diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index ac1213c727..b3f61c707d 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -64,8 +64,8 @@ class CustomField(Document): self.translatable = 0 if not self.flags.ignore_validate: - from frappe.core.doctype.doctype.doctype import check_if_fieldname_conflicts_with_properties_and_methods - check_if_fieldname_conflicts_with_properties_and_methods(self.dt, self.fieldname) + from frappe.core.doctype.doctype.doctype import check_fieldname_conflicts + check_fieldname_conflicts(self.dt, self.fieldname) def on_update(self): frappe.clear_cache(doctype=self.dt) From 877f9d08df70a741aa493421a002651ef0098cbc Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 16 Mar 2021 16:28:38 +0530 Subject: [PATCH 19/46] fix: Use fallback values if doctype values unset --- frappe/model/base_document.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 9288d621c9..2724e320c4 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -34,11 +34,15 @@ def get_controller(doctype): from frappe.model.document import Document from frappe.utils.nestedset import NestedSet - module_name, custom = ( - frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) - or frappe.db.get_value("DocType", doctype, ("module", "custom")) - or ["Core", False] - ) + if frappe.flags.in_migrate or frappe.flags.in_install or frappe.flags.in_patch: + module_name, custom = ["Core", False] + else: + # this could be simplified in PY3.8 using walrus operators + result = frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) + if result: + module_name, custom = result + else: + module_name, custom = ["Core", bool(not frappe.db.exists(doctype))] if custom: if frappe.db.field_exists("DocType", "is_tree"): From 87ed7796ded4053d8c1f04924716b29bc4848a6d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 23 Apr 2021 12:43:13 +0530 Subject: [PATCH 20/46] fix: Use older logic to set module_name and custom vars --- frappe/model/base_document.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/frappe/model/base_document.py b/frappe/model/base_document.py index 2724e320c4..154a091b8a 100644 --- a/frappe/model/base_document.py +++ b/frappe/model/base_document.py @@ -34,15 +34,9 @@ def get_controller(doctype): from frappe.model.document import Document from frappe.utils.nestedset import NestedSet - if frappe.flags.in_migrate or frappe.flags.in_install or frappe.flags.in_patch: - module_name, custom = ["Core", False] - else: - # this could be simplified in PY3.8 using walrus operators - result = frappe.db.get_value("DocType", doctype, ("module", "custom"), cache=True) - if result: - module_name, custom = result - else: - module_name, custom = ["Core", bool(not frappe.db.exists(doctype))] + module_name, custom = frappe.db.get_value( + "DocType", doctype, ("module", "custom"), cache=True + ) or ["Core", False] if custom: if frappe.db.field_exists("DocType", "is_tree"): From a280547c22455bc6fabffb410eb1f4017977877a Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Thu, 6 May 2021 17:05:55 +0530 Subject: [PATCH 21/46] perf: Performance enhancement on creation of custom fields from setup wizard (#13139) --- frappe/custom/doctype/custom_field/custom_field.py | 13 +++++++++++-- frappe/desk/page/setup_wizard/setup_wizard.py | 1 + 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/frappe/custom/doctype/custom_field/custom_field.py b/frappe/custom/doctype/custom_field/custom_field.py index 3126326636..fb49aa5da0 100644 --- a/frappe/custom/doctype/custom_field/custom_field.py +++ b/frappe/custom/doctype/custom_field/custom_field.py @@ -68,14 +68,15 @@ class CustomField(Document): check_if_fieldname_conflicts_with_methods(self.dt, self.fieldname) def on_update(self): - frappe.clear_cache(doctype=self.dt) + if not frappe.flags.in_setup_wizard: + frappe.clear_cache(doctype=self.dt) if not self.flags.ignore_validate: # validate field from frappe.core.doctype.doctype.doctype import validate_fields_for_doctype validate_fields_for_doctype(self.dt) # update the schema - if not frappe.db.get_value('DocType', self.dt, 'issingle'): + if not frappe.db.get_value('DocType', self.dt, 'issingle') and not frappe.flags.in_setup_wizard: frappe.db.updatedb(self.dt) def on_trash(self): @@ -144,6 +145,10 @@ def create_custom_fields(custom_fields, ignore_validate = False, update=True): '''Add / update multiple custom fields :param custom_fields: example `{'Sales Invoice': [dict(fieldname='test')]}`''' + + if not ignore_validate and frappe.flags.in_setup_wizard: + ignore_validate = True + for doctype, fields in custom_fields.items(): if isinstance(fields, dict): # only one field @@ -163,6 +168,10 @@ def create_custom_fields(custom_fields, ignore_validate = False, update=True): custom_field.update(df) custom_field.save() + frappe.clear_cache(doctype=doctype) + frappe.db.updatedb(doctype) + + @frappe.whitelist() def add_custom_field(doctype, df): diff --git a/frappe/desk/page/setup_wizard/setup_wizard.py b/frappe/desk/page/setup_wizard/setup_wizard.py index c38cf47626..1ac5279508 100755 --- a/frappe/desk/page/setup_wizard/setup_wizard.py +++ b/frappe/desk/page/setup_wizard/setup_wizard.py @@ -124,6 +124,7 @@ def handle_setup_exception(args): frappe.db.rollback() if args: traceback = frappe.get_traceback() + print(traceback) for hook in frappe.get_hooks("setup_wizard_exception"): frappe.get_attr(hook)(traceback, args) From bc08459ca77194bc5853a21b311aebfd125d62e9 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Thu, 6 May 2021 17:22:59 +0530 Subject: [PATCH 22/46] test: Test case for revert series --- frappe/tests/test_naming.py | 88 +++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 33 deletions(-) diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index b47fb809ca..fbfb1e677d 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -16,50 +16,50 @@ class TestNaming(unittest.TestCase): todo_doctype.autoname = 'hash' todo_doctype.save() - def test_append_number_if_name_exists(self): - ''' - Append number to name based on existing values - if Bottle exists - Bottle -> Bottle-1 - if Bottle-1 exists - Bottle -> Bottle-2 - ''' + # def test_append_number_if_name_exists(self): + # ''' + # Append number to name based on existing values + # if Bottle exists + # Bottle -> Bottle-1 + # if Bottle-1 exists + # Bottle -> Bottle-2 + # ''' - note = frappe.new_doc('Note') - note.title = 'Test' - note.insert() + # note = frappe.new_doc('Note') + # note.title = 'Test' + # note.insert() - title2 = append_number_if_name_exists('Note', 'Test') - self.assertEqual(title2, 'Test-1') + # title2 = append_number_if_name_exists('Note', 'Test') + # self.assertEqual(title2, 'Test-1') - title2 = append_number_if_name_exists('Note', 'Test', 'title', '_') - self.assertEqual(title2, 'Test_1') + # title2 = append_number_if_name_exists('Note', 'Test', 'title', '_') + # self.assertEqual(title2, 'Test_1') - def test_format_autoname(self): - ''' - Test if braced params are replaced in format autoname - ''' - doctype = 'ToDo' + # def test_format_autoname(self): + # ''' + # Test if braced params are replaced in format autoname + # ''' + # doctype = 'ToDo' - todo_doctype = frappe.get_doc('DocType', doctype) - todo_doctype.autoname = 'format:TODO-{MM}-{status}-{##}' - todo_doctype.save() + # todo_doctype = frappe.get_doc('DocType', doctype) + # todo_doctype.autoname = 'format:TODO-{MM}-{status}-{##}' + # todo_doctype.save() - description = 'Format' + # description = 'Format' - todo = frappe.new_doc(doctype) - todo.description = description - todo.insert() + # todo = frappe.new_doc(doctype) + # todo.description = description + # todo.insert() - series = getseries('', 2) + # series = getseries('', 2) - series = str(int(series)-1) + # series = str(int(series)-1) - if len(series) < 2: - series = '0' + series + # if len(series) < 2: + # series = '0' + series - self.assertEqual(todo.name, 'TODO-{month}-{status}-{series}'.format( - month=now_datetime().strftime('%m'), status=todo.status, series=series)) + # self.assertEqual(todo.name, 'TODO-{month}-{status}-{series}'.format( + # month=now_datetime().strftime('%m'), status=todo.status, series=series)) def test_revert_series(self): from datetime import datetime @@ -95,3 +95,25 @@ class TestNaming(unittest.TestCase): self.assertEqual(count.get('current'), 2) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) + + series = 'TEST1-' + key = 'TEST1-.#####.-2021-22' + name = 'TEST1-00003-2021-22' + frappe.db.sql("DELETE FROM `tabSeries` WHERE `name`=%s", series) + frappe.db.sql("""INSERT INTO `tabSeries` (name, current) values (%s, 3)""", (series,)) + revert_series_if_last(key, name) + count = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] + + self.assertEqual(count.get('current'), 2) + frappe.db.sql("""delete from `tabSeries` where name = %s""", series) + + series = '' + key = '.#####.-2021-22' + name = '00003-2021-22' + frappe.db.sql("DELETE FROM `tabSeries` WHERE `name`=%s", series) + frappe.db.sql("""INSERT INTO `tabSeries` (name, current) values (%s, 3)""", (series,)) + revert_series_if_last(key, name) + count = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] + + self.assertEqual(count.get('current'), 2) + frappe.db.sql("""delete from `tabSeries` where name = %s""", series) From 47d1f3d50abff0d253c157e760a72ad1eff3b696 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Thu, 6 May 2021 17:24:12 +0530 Subject: [PATCH 23/46] test: uncomment --- frappe/tests/test_naming.py | 66 ++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index fbfb1e677d..0c3481ba98 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -16,50 +16,50 @@ class TestNaming(unittest.TestCase): todo_doctype.autoname = 'hash' todo_doctype.save() - # def test_append_number_if_name_exists(self): - # ''' - # Append number to name based on existing values - # if Bottle exists - # Bottle -> Bottle-1 - # if Bottle-1 exists - # Bottle -> Bottle-2 - # ''' + def test_append_number_if_name_exists(self): + ''' + Append number to name based on existing values + if Bottle exists + Bottle -> Bottle-1 + if Bottle-1 exists + Bottle -> Bottle-2 + ''' - # note = frappe.new_doc('Note') - # note.title = 'Test' - # note.insert() + note = frappe.new_doc('Note') + note.title = 'Test' + note.insert() - # title2 = append_number_if_name_exists('Note', 'Test') - # self.assertEqual(title2, 'Test-1') + title2 = append_number_if_name_exists('Note', 'Test') + self.assertEqual(title2, 'Test-1') - # title2 = append_number_if_name_exists('Note', 'Test', 'title', '_') - # self.assertEqual(title2, 'Test_1') + title2 = append_number_if_name_exists('Note', 'Test', 'title', '_') + self.assertEqual(title2, 'Test_1') - # def test_format_autoname(self): - # ''' - # Test if braced params are replaced in format autoname - # ''' - # doctype = 'ToDo' + def test_format_autoname(self): + ''' + Test if braced params are replaced in format autoname + ''' + doctype = 'ToDo' - # todo_doctype = frappe.get_doc('DocType', doctype) - # todo_doctype.autoname = 'format:TODO-{MM}-{status}-{##}' - # todo_doctype.save() + todo_doctype = frappe.get_doc('DocType', doctype) + todo_doctype.autoname = 'format:TODO-{MM}-{status}-{##}' + todo_doctype.save() - # description = 'Format' + description = 'Format' - # todo = frappe.new_doc(doctype) - # todo.description = description - # todo.insert() + todo = frappe.new_doc(doctype) + todo.description = description + todo.insert() - # series = getseries('', 2) + series = getseries('', 2) - # series = str(int(series)-1) + series = str(int(series)-1) - # if len(series) < 2: - # series = '0' + series + if len(series) < 2: + series = '0' + series - # self.assertEqual(todo.name, 'TODO-{month}-{status}-{series}'.format( - # month=now_datetime().strftime('%m'), status=todo.status, series=series)) + self.assertEqual(todo.name, 'TODO-{month}-{status}-{series}'.format( + month=now_datetime().strftime('%m'), status=todo.status, series=series)) def test_revert_series(self): from datetime import datetime From f46c3b59a6385f3d470b03712e00ce0ad5cf289c Mon Sep 17 00:00:00 2001 From: shariquerik Date: Thu, 6 May 2021 17:26:54 +0530 Subject: [PATCH 24/46] fix: sider fix --- frappe/tests/test_naming.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index 0c3481ba98..f808d06db3 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -106,7 +106,7 @@ class TestNaming(unittest.TestCase): self.assertEqual(count.get('current'), 2) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) - + series = '' key = '.#####.-2021-22' name = '00003-2021-22' From abc49ef96066e2020a185ba3e9d76ba302444186 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Thu, 6 May 2021 19:35:43 +0550 Subject: [PATCH 25/46] bumped to version 13.2.1 --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 7a7881947f..12346d78c3 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -34,7 +34,7 @@ if PY2: reload(sys) sys.setdefaultencoding("utf-8") -__version__ = '13.2.0' +__version__ = '13.2.1' __title__ = "Frappe Framework" From c48a8e933f4017e41a175ba5a9a6e1b2207f758f Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 6 May 2021 18:39:17 +0530 Subject: [PATCH 26/46] fix: remove six.moves.input and use builtin.input reason: py2 support no longer required and six makes mocking/testing complicated. --- frappe/utils/boilerplate.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/utils/boilerplate.py b/frappe/utils/boilerplate.py index ba20562544..b935bfbf4f 100755 --- a/frappe/utils/boilerplate.py +++ b/frappe/utils/boilerplate.py @@ -3,8 +3,6 @@ from __future__ import unicode_literals, print_function -from six.moves import input - import frappe, os, re, git from frappe.utils import touch_file, cstr From 4210299ab424aa77c61269fb41f1b29d95cdee94 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 6 May 2021 19:12:19 +0530 Subject: [PATCH 27/46] test: add test for creating new app --- frappe/tests/test_boilerplate.py | 67 ++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 frappe/tests/test_boilerplate.py diff --git a/frappe/tests/test_boilerplate.py b/frappe/tests/test_boilerplate.py new file mode 100644 index 0000000000..14741dc29e --- /dev/null +++ b/frappe/tests/test_boilerplate.py @@ -0,0 +1,67 @@ +import os +import unittest +from unittest.mock import patch + +import frappe +from frappe.utils.boilerplate import make_boilerplate + + +class TestBoilerPlate(unittest.TestCase): + def test_create_app(self): + title = "Test App" + description = "Test app for unit testing" + publisher = "Test Publisher" + email = "example@example.org" + icon = "" # empty -> default + color = "" + app_license = "MIT" + + user_input = [ + title, + description, + publisher, + email, + icon, + color, + app_license, + ] + + bench_path = frappe.utils.get_bench_path() + apps_dir = os.path.join(bench_path, "apps") + app_name = "test_app" + + with patch("builtins.input", side_effect=user_input): + make_boilerplate(apps_dir, app_name) + + root_paths = [ + app_name, + "requirements.txt", + "README.md", + "setup.py", + "license.txt", + ".git", + ] + paths_inside_app = [ + "__init__.py", + "hooks.py", + "patches.txt", + "templates", + "www", + "config", + "modules.txt", + "public", + app_name, + ] + + new_app_dir = os.path.join(bench_path, apps_dir, app_name) + + all_paths = list() + + for path in root_paths: + all_paths.append(os.path.join(new_app_dir, path)) + + for path in paths_inside_app: + all_paths.append(os.path.join(new_app_dir, app_name, path)) + + for path in all_paths: + self.assertTrue(os.path.exists(path), msg=f"{path} should exist in new app") From a077466b1e217985a6eab0d1ba68fb58ac5e102e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 6 May 2021 21:16:22 +0530 Subject: [PATCH 28/46] chore: replace assertEquals with alias assertEqual `assertEquals` has been deprecated, while not fully removed it will still keep giving warnings in tests / CI. ref: https://docs.python.org/3/library/unittest.html#deprecated-aliases --- .../doctype/auto_repeat/test_auto_repeat.py | 2 +- .../doctype/activity_log/test_activity_log.py | 4 +- .../user_permission/test_user_permission.py | 20 +++---- .../customize_form/test_customize_form.py | 38 +++++++------- .../document_follow/test_document_follow.py | 4 +- frappe/tests/test_commands.py | 52 +++++++++---------- frappe/tests/test_website.py | 20 +++---- 7 files changed, 70 insertions(+), 70 deletions(-) diff --git a/frappe/automation/doctype/auto_repeat/test_auto_repeat.py b/frappe/automation/doctype/auto_repeat/test_auto_repeat.py index f41f31f3bb..6ceb4dba72 100644 --- a/frappe/automation/doctype/auto_repeat/test_auto_repeat.py +++ b/frappe/automation/doctype/auto_repeat/test_auto_repeat.py @@ -173,7 +173,7 @@ class TestAutoRepeat(unittest.TestCase): fields=['docstatus'], limit=1 ) - self.assertEquals(docnames[0].docstatus, 1) + self.assertEqual(docnames[0].docstatus, 1) def make_auto_repeat(**args): diff --git a/frappe/core/doctype/activity_log/test_activity_log.py b/frappe/core/doctype/activity_log/test_activity_log.py index bd0ea08cc7..05ece76c7f 100644 --- a/frappe/core/doctype/activity_log/test_activity_log.py +++ b/frappe/core/doctype/activity_log/test_activity_log.py @@ -65,12 +65,12 @@ class TestActivityLog(unittest.TestCase): frappe.local.login_manager = LoginManager() auth_log = self.get_auth_log() - self.assertEquals(auth_log.status, 'Success') + self.assertEqual(auth_log.status, 'Success') # test user logout log frappe.local.login_manager.logout() auth_log = self.get_auth_log(operation='Logout') - self.assertEquals(auth_log.status, 'Success') + self.assertEqual(auth_log.status, 'Success') # test invalid login frappe.form_dict.update({ 'pwd': 'password' }) diff --git a/frappe/core/doctype/user_permission/test_user_permission.py b/frappe/core/doctype/user_permission/test_user_permission.py index 2e9b832acc..47651fee72 100644 --- a/frappe/core/doctype/user_permission/test_user_permission.py +++ b/frappe/core/doctype/user_permission/test_user_permission.py @@ -46,7 +46,7 @@ class TestUserPermission(unittest.TestCase): frappe.set_user('test_user_perm1@example.com') doc = frappe.new_doc("Blog Post") - self.assertEquals(doc.blog_category, 'general') + self.assertEqual(doc.blog_category, 'general') frappe.set_user('Administrator') def test_apply_to_all(self): @@ -54,7 +54,7 @@ class TestUserPermission(unittest.TestCase): user = create_user('test_bulk_creation_update@example.com') param = get_params(user, 'User', user.name) is_created = add_user_permissions(param) - self.assertEquals(is_created, 1) + self.assertEqual(is_created, 1) def test_for_apply_to_all_on_update_from_apply_all(self): user = create_user('test_bulk_creation_update@example.com') @@ -63,11 +63,11 @@ class TestUserPermission(unittest.TestCase): # Initially create User Permission document with apply_to_all checked is_created = add_user_permissions(param) - self.assertEquals(is_created, 1) + self.assertEqual(is_created, 1) is_created = add_user_permissions(param) # User Permission should not be changed - self.assertEquals(is_created, 0) + self.assertEqual(is_created, 0) def test_for_applicable_on_update_from_apply_to_all(self): ''' Update User Permission from all to some applicable Doctypes''' @@ -77,7 +77,7 @@ class TestUserPermission(unittest.TestCase): # Initially create User Permission document with apply_to_all checked is_created = add_user_permissions(get_params(user, 'User', user.name)) - self.assertEquals(is_created, 1) + self.assertEqual(is_created, 1) is_created = add_user_permissions(param) frappe.db.commit() @@ -92,7 +92,7 @@ class TestUserPermission(unittest.TestCase): # Check that User Permissions for applicable is created self.assertIsNotNone(is_created_applicable_first) self.assertIsNotNone(is_created_applicable_second) - self.assertEquals(is_created, 1) + self.assertEqual(is_created, 1) def test_for_apply_to_all_on_update_from_applicable(self): ''' Update User Permission from some to all applicable Doctypes''' @@ -102,7 +102,7 @@ class TestUserPermission(unittest.TestCase): # create User permissions that with applicable is_created = add_user_permissions(get_params(user, 'User', user.name, applicable = ["Chat Room", "Chat Message"])) - self.assertEquals(is_created, 1) + self.assertEqual(is_created, 1) is_created = add_user_permissions(param) is_created_apply_to_all = frappe.db.exists("User Permission", get_exists_param(user)) @@ -115,7 +115,7 @@ class TestUserPermission(unittest.TestCase): # Check that all User Permission with applicable is removed self.assertIsNone(removed_applicable_first) self.assertIsNone(removed_applicable_second) - self.assertEquals(is_created, 1) + self.assertEqual(is_created, 1) def test_user_perm_for_nested_doctype(self): """Test if descendants' visibility is controlled for a nested DocType.""" @@ -183,7 +183,7 @@ class TestUserPermission(unittest.TestCase): # User perm is created on ToDo but for doctype Assignment Rule only # it should not have impact on Doc A - self.assertEquals(new_doc.doc, "ToDo") + self.assertEqual(new_doc.doc, "ToDo") frappe.set_user('Administrator') remove_applicable(["Assignment Rule"], "new_doc_test@example.com", "DocType", "ToDo") @@ -228,7 +228,7 @@ class TestUserPermission(unittest.TestCase): # User perm is created on ToDo but for doctype Assignment Rule only # it should not have impact on Doc A - self.assertEquals(new_doc.doc, "ToDo") + self.assertEqual(new_doc.doc, "ToDo") frappe.set_user('Administrator') clear_session_defaults() diff --git a/frappe/custom/doctype/customize_form/test_customize_form.py b/frappe/custom/doctype/customize_form/test_customize_form.py index f5e0371c1f..75555a8205 100644 --- a/frappe/custom/doctype/customize_form/test_customize_form.py +++ b/frappe/custom/doctype/customize_form/test_customize_form.py @@ -47,64 +47,64 @@ class TestCustomizeForm(unittest.TestCase): self.assertEqual(len(d.get("fields")), 0) d = self.get_customize_form("Event") - self.assertEquals(d.doc_type, "Event") - self.assertEquals(len(d.get("fields")), 36) + self.assertEqual(d.doc_type, "Event") + self.assertEqual(len(d.get("fields")), 36) d = self.get_customize_form("Event") - self.assertEquals(d.doc_type, "Event") + self.assertEqual(d.doc_type, "Event") self.assertEqual(len(d.get("fields")), len(frappe.get_doc("DocType", d.doc_type).fields) + 1) - self.assertEquals(d.get("fields")[-1].fieldname, "test_custom_field") - self.assertEquals(d.get("fields", {"fieldname": "event_type"})[0].in_list_view, 1) + self.assertEqual(d.get("fields")[-1].fieldname, "test_custom_field") + self.assertEqual(d.get("fields", {"fieldname": "event_type"})[0].in_list_view, 1) return d def test_save_customization_property(self): d = self.get_customize_form("Event") - self.assertEquals(frappe.db.get_value("Property Setter", + self.assertEqual(frappe.db.get_value("Property Setter", {"doc_type": "Event", "property": "allow_copy"}, "value"), None) d.allow_copy = 1 d.run_method("save_customization") - self.assertEquals(frappe.db.get_value("Property Setter", + self.assertEqual(frappe.db.get_value("Property Setter", {"doc_type": "Event", "property": "allow_copy"}, "value"), '1') d.allow_copy = 0 d.run_method("save_customization") - self.assertEquals(frappe.db.get_value("Property Setter", + self.assertEqual(frappe.db.get_value("Property Setter", {"doc_type": "Event", "property": "allow_copy"}, "value"), None) def test_save_customization_field_property(self): d = self.get_customize_form("Event") - self.assertEquals(frappe.db.get_value("Property Setter", + self.assertEqual(frappe.db.get_value("Property Setter", {"doc_type": "Event", "property": "reqd", "field_name": "repeat_this_event"}, "value"), None) repeat_this_event_field = d.get("fields", {"fieldname": "repeat_this_event"})[0] repeat_this_event_field.reqd = 1 d.run_method("save_customization") - self.assertEquals(frappe.db.get_value("Property Setter", + self.assertEqual(frappe.db.get_value("Property Setter", {"doc_type": "Event", "property": "reqd", "field_name": "repeat_this_event"}, "value"), '1') repeat_this_event_field = d.get("fields", {"fieldname": "repeat_this_event"})[0] repeat_this_event_field.reqd = 0 d.run_method("save_customization") - self.assertEquals(frappe.db.get_value("Property Setter", + self.assertEqual(frappe.db.get_value("Property Setter", {"doc_type": "Event", "property": "reqd", "field_name": "repeat_this_event"}, "value"), None) def test_save_customization_custom_field_property(self): d = self.get_customize_form("Event") - self.assertEquals(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0) + self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0) custom_field = d.get("fields", {"fieldname": "test_custom_field"})[0] custom_field.reqd = 1 d.run_method("save_customization") - self.assertEquals(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 1) + self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 1) custom_field = d.get("fields", {"is_custom_field": True})[0] custom_field.reqd = 0 d.run_method("save_customization") - self.assertEquals(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0) + self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_custom_field", "reqd"), 0) def test_save_customization_new_field(self): d = self.get_customize_form("Event") @@ -115,14 +115,14 @@ class TestCustomizeForm(unittest.TestCase): "is_custom_field": 1 }) d.run_method("save_customization") - self.assertEquals(frappe.db.get_value("Custom Field", + self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_add_custom_field_via_customize_form", "fieldtype"), "Data") - self.assertEquals(frappe.db.get_value("Custom Field", + self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_add_custom_field_via_customize_form", 'insert_after'), last_fieldname) frappe.delete_doc("Custom Field", "Event-test_add_custom_field_via_customize_form") - self.assertEquals(frappe.db.get_value("Custom Field", + self.assertEqual(frappe.db.get_value("Custom Field", "Event-test_add_custom_field_via_customize_form"), None) @@ -142,7 +142,7 @@ class TestCustomizeForm(unittest.TestCase): d.doc_type = "Event" d.run_method('reset_to_defaults') - self.assertEquals(d.get("fields", {"fieldname": "repeat_this_event"})[0].in_list_view, 0) + self.assertEqual(d.get("fields", {"fieldname": "repeat_this_event"})[0].in_list_view, 0) frappe.local.test_objects["Property Setter"] = [] make_test_records_for_doctype("Property Setter") @@ -156,7 +156,7 @@ class TestCustomizeForm(unittest.TestCase): d = self.get_customize_form("Event") # don't allow for standard fields - self.assertEquals(d.get("fields", {"fieldname": "subject"})[0].allow_on_submit or 0, 0) + self.assertEqual(d.get("fields", {"fieldname": "subject"})[0].allow_on_submit or 0, 0) # allow for custom field self.assertEqual(d.get("fields", {"fieldname": "test_custom_field"})[0].allow_on_submit, 1) diff --git a/frappe/email/doctype/document_follow/test_document_follow.py b/frappe/email/doctype/document_follow/test_document_follow.py index 1ac2d19305..38aa870232 100644 --- a/frappe/email/doctype/document_follow/test_document_follow.py +++ b/frappe/email/doctype/document_follow/test_document_follow.py @@ -17,14 +17,14 @@ class TestDocumentFollow(unittest.TestCase): document_follow.unfollow_document("Event", event_doc.name, user.name) doc = document_follow.follow_document("Event", event_doc.name, user.name) - self.assertEquals(doc.user, user.name) + self.assertEqual(doc.user, user.name) document_follow.send_hourly_updates() email_queue_entry_name = frappe.get_all("Email Queue", limit=1)[0].name email_queue_entry_doc = frappe.get_doc("Email Queue", email_queue_entry_name) - self.assertEquals((email_queue_entry_doc.recipients[0].recipient), user.name) + self.assertEqual((email_queue_entry_doc.recipients[0].recipient), user.name) self.assertIn(event_doc.doctype, email_queue_entry_doc.message) self.assertIn(event_doc.name, email_queue_entry_doc.message) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 9ed8ecb054..b6cd0b575c 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -115,12 +115,12 @@ class TestCommands(BaseTestCommands): def test_execute(self): # test 1: execute a command expecting a numeric output self.execute("bench --site {site} execute frappe.db.get_database_size") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertIsInstance(float(self.stdout), float) # test 2: execute a command expecting an errored output as local won't exist self.execute("bench --site {site} execute frappe.local.site") - self.assertEquals(self.returncode, 1) + self.assertEqual(self.returncode, 1) self.assertIsNotNone(self.stderr) # test 3: execute a command with kwargs @@ -128,8 +128,8 @@ class TestCommands(BaseTestCommands): # terminal command has been escaped to avoid .format string replacement # The returned value has quotes which have been trimmed for the test self.execute("""bench --site {site} execute frappe.bold --kwargs '{{"text": "DocType"}}'""") - self.assertEquals(self.returncode, 0) - self.assertEquals(self.stdout[1:-1], frappe.bold(text="DocType")) + self.assertEqual(self.returncode, 0) + self.assertEqual(self.stdout[1:-1], frappe.bold(text="DocType")) def test_backup(self): backup = { @@ -155,7 +155,7 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} backup") after_backup = fetch_latest_backups() - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertIn("successfully completed", self.stdout) self.assertNotEqual(before_backup["database"], after_backup["database"]) @@ -164,7 +164,7 @@ class TestCommands(BaseTestCommands): self.execute("bench --site {site} backup --with-files") after_backup = fetch_latest_backups() - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertIn("successfully completed", self.stdout) self.assertIn("with files", self.stdout) self.assertNotEqual(before_backup, after_backup) @@ -175,7 +175,7 @@ class TestCommands(BaseTestCommands): backup_path = os.path.join(home, "backups") self.execute("bench --site {site} backup --backup-path {backup_path}", {"backup_path": backup_path}) - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertTrue(os.path.exists(backup_path)) self.assertGreaterEqual(len(os.listdir(backup_path)), 2) @@ -200,19 +200,19 @@ class TestCommands(BaseTestCommands): kwargs, ) - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) for path in kwargs.values(): self.assertTrue(os.path.exists(path)) # test 5: take a backup with --compress self.execute("bench --site {site} backup --with-files --compress") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) compressed_files = glob.glob(site_backup_path + "/*.tgz") self.assertGreater(len(compressed_files), 0) # test 6: take a backup with --verbose self.execute("bench --site {site} backup --verbose") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) # test 7: take a backup with frappe.conf.backup.includes self.execute( @@ -220,7 +220,7 @@ class TestCommands(BaseTestCommands): {"includes": json.dumps(backup["includes"])}, ) self.execute("bench --site {site} backup --verbose") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) @@ -230,7 +230,7 @@ class TestCommands(BaseTestCommands): {"excludes": json.dumps(backup["excludes"])}, ) self.execute("bench --site {site} backup --verbose") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) @@ -240,7 +240,7 @@ class TestCommands(BaseTestCommands): "bench --site {site} backup --include '{include}'", {"include": ",".join(backup["includes"]["includes"])}, ) - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertTrue(exists_in_backup(backup["includes"]["includes"], database)) @@ -249,13 +249,13 @@ class TestCommands(BaseTestCommands): "bench --site {site} backup --exclude '{exclude}'", {"exclude": ",".join(backup["excludes"]["excludes"])}, ) - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) database = fetch_latest_backups(partial=True)["database"] self.assertFalse(exists_in_backup(backup["excludes"]["excludes"], database)) # test 11: take a backup with --ignore-backup-conf self.execute("bench --site {site} backup --ignore-backup-conf") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) database = fetch_latest_backups()["database"] self.assertTrue(exists_in_backup(backup["excludes"]["excludes"], database)) @@ -296,7 +296,7 @@ class TestCommands(BaseTestCommands): ) site_data.update({"database": json.loads(self.stdout)["database"]}) self.execute("bench --site {another_site} restore {database}", site_data) - self.assertEquals(self.returncode, 1) + self.assertEqual(self.returncode, 1) def test_partial_restore(self): _now = now() @@ -319,8 +319,8 @@ class TestCommands(BaseTestCommands): frappe.db.commit() self.execute("bench --site {site} partial-restore {path}", {"path": db_path}) - self.assertEquals(self.returncode, 0) - self.assertEquals(frappe.db.count("ToDo"), todo_count) + self.assertEqual(self.returncode, 0) + self.assertEqual(frappe.db.count("ToDo"), todo_count) def test_recorder(self): frappe.recorder.stop() @@ -343,18 +343,18 @@ class TestCommands(BaseTestCommands): # test 1: remove app from installed_apps global default self.execute("bench --site {site} remove-from-installed-apps {app}", {"app": app}) - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.execute("bench --site {site} list-apps") self.assertNotIn(app, self.stdout) def test_list_apps(self): # test 1: sanity check for command self.execute("bench --site all list-apps") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) # test 2: bare functionality for single site self.execute("bench --site {site} list-apps") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) list_apps = set([ _x.split()[0] for _x in self.stdout.split("\n") ]) @@ -367,7 +367,7 @@ class TestCommands(BaseTestCommands): # test 3: parse json format self.execute("bench --site all list-apps --format json") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertIsInstance(json.loads(self.stdout), dict) self.execute("bench --site {site} list-apps --format json") @@ -379,7 +379,7 @@ class TestCommands(BaseTestCommands): def test_show_config(self): # test 1: sanity check for command self.execute("bench --site all show-config") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) # test 2: test keys in table text self.execute( @@ -387,13 +387,13 @@ class TestCommands(BaseTestCommands): {"second_order": json.dumps({"test_key": "test_value"})}, ) self.execute("bench --site {site} show-config") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertIn("test_key.test_key", self.stdout.split()) self.assertIn("test_value", self.stdout.split()) # test 3: parse json format self.execute("bench --site all show-config --format json") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertIsInstance(json.loads(self.stdout), dict) self.execute("bench --site {site} show-config --format json") @@ -423,6 +423,6 @@ class TestCommands(BaseTestCommands): def test_frappe_site_env(self): os.putenv('FRAPPE_SITE', frappe.local.site) self.execute("bench execute frappe.ping") - self.assertEquals(self.returncode, 0) + self.assertEqual(self.returncode, 0) self.assertIn("pong", self.stdout) diff --git a/frappe/tests/test_website.py b/frappe/tests/test_website.py index c5da2bdfb7..dc3862174d 100644 --- a/frappe/tests/test_website.py +++ b/frappe/tests/test_website.py @@ -48,7 +48,7 @@ class TestWebsite(unittest.TestCase): set_request(method='POST', path='login') response = render.render() - self.assertEquals(response.status_code, 200) + self.assertEqual(response.status_code, 200) html = frappe.safe_decode(response.get_data()) @@ -76,27 +76,27 @@ class TestWebsite(unittest.TestCase): set_request(method='GET', path='/testfrom') response = render.render() - self.assertEquals(response.status_code, 301) - self.assertEquals(response.headers.get('Location'), r'://testto1') + self.assertEqual(response.status_code, 301) + self.assertEqual(response.headers.get('Location'), r'://testto1') set_request(method='GET', path='/testfromregex/test') response = render.render() - self.assertEquals(response.status_code, 301) - self.assertEquals(response.headers.get('Location'), r'://testto2') + self.assertEqual(response.status_code, 301) + self.assertEqual(response.headers.get('Location'), r'://testto2') set_request(method='GET', path='/testsub/me') response = render.render() - self.assertEquals(response.status_code, 301) - self.assertEquals(response.headers.get('Location'), r'://testto3/me') + self.assertEqual(response.status_code, 301) + self.assertEqual(response.headers.get('Location'), r'://testto3/me') set_request(method='GET', path='/test404') response = render.render() - self.assertEquals(response.status_code, 404) + self.assertEqual(response.status_code, 404) set_request(method='GET', path='/testsource') response = render.render() - self.assertEquals(response.status_code, 301) - self.assertEquals(response.headers.get('Location'), '/testtarget') + self.assertEqual(response.status_code, 301) + self.assertEqual(response.headers.get('Location'), '/testtarget') delattr(frappe.hooks, 'website_redirects') frappe.cache().delete_key('app_hooks') From 7933b9c825b528d640eee9565f81c78ea7d7a830 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Thu, 6 May 2021 21:23:04 +0530 Subject: [PATCH 29/46] chore: replace assertNotEquals with assertNotEqual --- frappe/core/doctype/report/test_report.py | 2 +- frappe/social/doctype/energy_point_log/test_energy_point_log.py | 2 +- frappe/tests/test_db.py | 2 +- frappe/website/doctype/web_form/test_web_form.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frappe/core/doctype/report/test_report.py b/frappe/core/doctype/report/test_report.py index 9c76c839f3..d09799ca69 100644 --- a/frappe/core/doctype/report/test_report.py +++ b/frappe/core/doctype/report/test_report.py @@ -106,7 +106,7 @@ class TestReport(unittest.TestCase): else: report = frappe.get_doc('Report', 'Test Report') - self.assertNotEquals(report.is_permitted(), True) + self.assertNotEqual(report.is_permitted(), True) frappe.set_user('Administrator') # test for the `_format` method if report data doesn't have sort_by parameter diff --git a/frappe/social/doctype/energy_point_log/test_energy_point_log.py b/frappe/social/doctype/energy_point_log/test_energy_point_log.py index 9ccef02eb3..222c64389b 100644 --- a/frappe/social/doctype/energy_point_log/test_energy_point_log.py +++ b/frappe/social/doctype/energy_point_log/test_energy_point_log.py @@ -78,7 +78,7 @@ class TestEnergyPointLog(unittest.TestCase): points_after_closing_todo = get_points('test@example.com') # test max_points cap - self.assertNotEquals(points_after_closing_todo, + self.assertNotEqual(points_after_closing_todo, energy_point_of_user + round(todo_point_rule.points * multiplier_value)) self.assertEqual(points_after_closing_todo, diff --git a/frappe/tests/test_db.py b/frappe/tests/test_db.py index 73afb5d304..79ab8bf421 100644 --- a/frappe/tests/test_db.py +++ b/frappe/tests/test_db.py @@ -18,7 +18,7 @@ class TestDB(unittest.TestCase): def test_get_value(self): self.assertEqual(frappe.db.get_value("User", {"name": ["=", "Administrator"]}), "Administrator") self.assertEqual(frappe.db.get_value("User", {"name": ["like", "Admin%"]}), "Administrator") - self.assertNotEquals(frappe.db.get_value("User", {"name": ["!=", "Guest"]}), "Guest") + self.assertNotEqual(frappe.db.get_value("User", {"name": ["!=", "Guest"]}), "Guest") self.assertEqual(frappe.db.get_value("User", {"name": ["<", "Adn"]}), "Administrator") self.assertEqual(frappe.db.get_value("User", {"name": ["<=", "Administrator"]}), "Administrator") diff --git a/frappe/website/doctype/web_form/test_web_form.py b/frappe/website/doctype/web_form/test_web_form.py index f4a3b5d1e3..d16a613546 100644 --- a/frappe/website/doctype/web_form/test_web_form.py +++ b/frappe/website/doctype/web_form/test_web_form.py @@ -42,7 +42,7 @@ class TestWebForm(unittest.TestCase): 'name': self.event_name } - self.assertNotEquals(frappe.db.get_value("Event", + self.assertNotEqual(frappe.db.get_value("Event", self.event_name, "description"), doc.get('description')) accept(web_form='manage-events', docname=self.event_name, data=json.dumps(doc)) From 6c85c366305f7d784e3409814676a9ebaaf903c9 Mon Sep 17 00:00:00 2001 From: leela Date: Thu, 6 May 2021 07:14:29 +0530 Subject: [PATCH 30/46] refactor: Remove deprecated inspect.getargspec Replace deprecated inspect.getargspec with inspect.getfullargspec. * inspect.getargspec does not support kw only args aswell as type annotations. * replace with inspect.getfullargspec, that supports kw only args. --- frappe/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 4356bdf9ef..10bf3eaddf 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -1173,9 +1173,8 @@ def get_newargs(fn, kwargs): fnargs = fn.fnargs else: fnargs = inspect.getfullargspec(fn).args - varargs = inspect.getfullargspec(fn).varargs + fnargs.extend(inspect.getfullargspec(fn).kwonlyargs) varkw = inspect.getfullargspec(fn).varkw - defaults = inspect.getfullargspec(fn).defaults newargs = {} for a in kwargs: From 72ca7e975335114d986caa2e7106075c69d44f67 Mon Sep 17 00:00:00 2001 From: leela Date: Thu, 6 May 2021 07:32:59 +0530 Subject: [PATCH 31/46] refactor: remove six dependency --- frappe/__init__.py | 23 +++++++++---------- frappe/core/doctype/data_import/importer.py | 6 ++--- .../user_permission/user_permission.py | 2 +- frappe/database/database.py | 2 +- frappe/database/postgres/database.py | 8 +++---- .../notification_log/notification_log.py | 2 +- frappe/model/db_query.py | 2 +- frappe/model/meta.py | 2 +- frappe/utils/data.py | 2 +- frappe/utils/jinja_globals.py | 2 +- 10 files changed, 25 insertions(+), 26 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 10bf3eaddf..54dc9edc8e 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -11,7 +11,6 @@ be used to build database driven apps. Read the documentation: https://frappeframework.com/docs """ -from six import iteritems, binary_type, text_type, string_types from werkzeug.local import Local, release_local import os, sys, importlib, inspect, json, warnings import typing @@ -91,14 +90,14 @@ def _(msg, lang=None, context=None): def as_unicode(text, encoding='utf-8'): '''Convert to unicode if required''' - if isinstance(text, text_type): + if isinstance(text, str): return text elif text==None: return '' - elif isinstance(text, binary_type): - return text_type(text, encoding) + elif isinstance(text, bytes): + return str(text, encoding) else: - return text_type(text) + return str(text) def get_lang_dict(fortype, name=None): """Returns the translated language dict for the given type and name. @@ -591,7 +590,7 @@ def is_whitelisted(method): # strictly sanitize form_dict # escapes html characters like <> except for predefined tags like a, b, ul etc. for key, value in form_dict.items(): - if isinstance(value, string_types): + if isinstance(value, str): form_dict[key] = sanitize_html(value) def read_only(): @@ -715,7 +714,7 @@ def has_website_permission(doc=None, ptype='read', user=None, verbose=False, doc user = session.user if doc: - if isinstance(doc, string_types): + if isinstance(doc, str): doc = get_doc(doctype, doc) doctype = doc.doctype @@ -784,7 +783,7 @@ def set_value(doctype, docname, fieldname, value=None): return frappe.client.set_value(doctype, docname, fieldname, value) def get_cached_doc(*args, **kwargs): - if args and len(args) > 1 and isinstance(args[1], text_type): + if args and len(args) > 1 and isinstance(args[1], str): key = get_document_cache_key(args[0], args[1]) # local cache doc = local.document_cache.get(key) @@ -815,7 +814,7 @@ def clear_document_cache(doctype, name): def get_cached_value(doctype, name, fieldname, as_dict=False): doc = get_cached_doc(doctype, name) - if isinstance(fieldname, string_types): + if isinstance(fieldname, str): if as_dict: throw('Cannot make dict for single fieldname') return doc.get(fieldname) @@ -1021,7 +1020,7 @@ def get_doc_hooks(): if not hasattr(local, 'doc_events_hooks'): hooks = get_hooks('doc_events', {}) out = {} - for key, value in iteritems(hooks): + for key, value in hooks.items(): if isinstance(key, tuple): for doctype in key: append_hook(out, doctype, value) @@ -1138,7 +1137,7 @@ def get_file_json(path): def read_file(path, raise_not_found=False): """Open a file and return its content as Unicode.""" - if isinstance(path, text_type): + if isinstance(path, str): path = path.encode("utf-8") if os.path.exists(path): @@ -1161,7 +1160,7 @@ def get_attr(method_string): def call(fn, *args, **kwargs): """Call a function and match arguments.""" - if isinstance(fn, string_types): + if isinstance(fn, str): fn = get_attr(fn) newargs = get_newargs(fn, kwargs) diff --git a/frappe/core/doctype/data_import/importer.py b/frappe/core/doctype/data_import/importer.py index 388d9389f2..720fe1dda7 100644 --- a/frappe/core/doctype/data_import/importer.py +++ b/frappe/core/doctype/data_import/importer.py @@ -233,7 +233,7 @@ class Importer: return updated_doc else: # throw if no changes - frappe.throw("No changes to update") + frappe.throw(_("No changes to update")) def get_eta(self, current, total, processing_time): self.last_eta = getattr(self, "last_eta", 0) @@ -319,7 +319,7 @@ class ImportFile: self.warnings = [] self.file_doc = self.file_path = self.google_sheets_url = None - if isinstance(file, frappe.string_types): + if isinstance(file, str): if frappe.db.exists("File", {"file_url": file}): self.file_doc = frappe.get_doc("File", {"file_url": file}) elif "docs.google.com/spreadsheets" in file: @@ -626,7 +626,7 @@ class Row: return elif df.fieldtype in ["Date", "Datetime"]: value = self.get_date(value, col) - if isinstance(value, frappe.string_types): + if isinstance(value, str): # value was not parsed as datetime object self.warnings.append( { diff --git a/frappe/core/doctype/user_permission/user_permission.py b/frappe/core/doctype/user_permission/user_permission.py index fbc788f6bf..fec5019ca9 100644 --- a/frappe/core/doctype/user_permission/user_permission.py +++ b/frappe/core/doctype/user_permission/user_permission.py @@ -191,7 +191,7 @@ def clear_user_permissions(user, for_doctype): def add_user_permissions(data): ''' Add and update the user permissions ''' frappe.only_for('System Manager') - if isinstance(data, frappe.string_types): + if isinstance(data, str): data = json.loads(data) data = frappe._dict(data) diff --git a/frappe/database/database.py b/frappe/database/database.py index 58e5c8a46e..c9c1ec3909 100644 --- a/frappe/database/database.py +++ b/frappe/database/database.py @@ -858,7 +858,7 @@ class Database(object): if not datetime: return '0001-01-01 00:00:00.000000' - if isinstance(datetime, frappe.string_types): + if isinstance(datetime, str): if ':' not in datetime: datetime = datetime + ' 00:00:00.000000' else: diff --git a/frappe/database/postgres/database.py b/frappe/database/postgres/database.py index f40dfad901..6ac2767a71 100644 --- a/frappe/database/postgres/database.py +++ b/frappe/database/postgres/database.py @@ -11,9 +11,9 @@ from frappe.database.postgres.schema import PostgresTable # cast decimals as floats DEC2FLOAT = psycopg2.extensions.new_type( - psycopg2.extensions.DECIMAL.values, - 'DEC2FLOAT', - lambda value, curs: float(value) if value is not None else None) + psycopg2.extensions.DECIMAL.values, + 'DEC2FLOAT', + lambda value, curs: float(value) if value is not None else None) psycopg2.extensions.register_type(DEC2FLOAT) @@ -111,7 +111,7 @@ class PostgresDatabase(Database): if not date: return '0001-01-01' - if not isinstance(date, frappe.string_types): + if not isinstance(date, str): date = date.strftime('%Y-%m-%d') return date diff --git a/frappe/desk/doctype/notification_log/notification_log.py b/frappe/desk/doctype/notification_log/notification_log.py index 20551559fd..25af92f532 100644 --- a/frappe/desk/doctype/notification_log/notification_log.py +++ b/frappe/desk/doctype/notification_log/notification_log.py @@ -46,7 +46,7 @@ def enqueue_create_notification(users, doc): doc = frappe._dict(doc) - if isinstance(users, frappe.string_types): + if isinstance(users, str): users = [user.strip() for user in users.split(',') if user.strip()] users = list(set(users)) diff --git a/frappe/model/db_query.py b/frappe/model/db_query.py index 1c863a1577..e0c3406c46 100644 --- a/frappe/model/db_query.py +++ b/frappe/model/db_query.py @@ -465,7 +465,7 @@ class DatabaseQuery(object): elif f.operator.lower() in ('in', 'not in'): values = f.value or '' - if isinstance(values, frappe.string_types): + if isinstance(values, str): values = values.split(",") fallback = "''" diff --git a/frappe/model/meta.py b/frappe/model/meta.py index 7f58c28397..66e8b08d92 100644 --- a/frappe/model/meta.py +++ b/frappe/model/meta.py @@ -118,7 +118,7 @@ class Meta(Document): # non standard list object, skip continue - if (isinstance(value, (frappe.text_type, int, float, datetime, list, tuple)) + if (isinstance(value, (str, int, float, datetime, list, tuple)) or (not no_nulls and value is None)): out[key] = value diff --git a/frappe/utils/data.py b/frappe/utils/data.py index 3ffa8dc874..1e09be7ad0 100644 --- a/frappe/utils/data.py +++ b/frappe/utils/data.py @@ -871,7 +871,7 @@ def in_words(integer, in_million=True): return ret.replace('-', ' ') def is_html(text): - if not isinstance(text, frappe.string_types): + if not isinstance(text, str): return False return re.search('<[^>]+>', text) diff --git a/frappe/utils/jinja_globals.py b/frappe/utils/jinja_globals.py index e63926a109..d8c5cb79f1 100644 --- a/frappe/utils/jinja_globals.py +++ b/frappe/utils/jinja_globals.py @@ -10,7 +10,7 @@ def resolve_class(classes): if classes is None: return "" - if isinstance(classes, frappe.string_types): + if isinstance(classes, str): return classes if isinstance(classes, (list, tuple)): From 5fabd51a03faca5652e78b7d51968d6c1b8d094c Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 7 May 2021 09:30:31 +0530 Subject: [PATCH 32/46] fix: Summary Item width --- frappe/public/scss/desk/report.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frappe/public/scss/desk/report.scss b/frappe/public/scss/desk/report.scss index 0aa2806649..ea09bd8046 100644 --- a/frappe/public/scss/desk/report.scss +++ b/frappe/public/scss/desk/report.scss @@ -161,7 +161,8 @@ .summary-item { // SIZE & SPACING margin: 0px 30px; - width: 180px; + min-width: 180px; + max-width: 300px; height: 62px; // LAYOUT From 4fb545fb8cbc57216632332faa76eb6d7bd5db3e Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Fri, 7 May 2021 09:30:59 +0530 Subject: [PATCH 33/46] fix: Quill Table border color --- frappe/public/scss/common/quill.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/scss/common/quill.scss b/frappe/public/scss/common/quill.scss index 24e8293cf3..67cf369db8 100644 --- a/frappe/public/scss/common/quill.scss +++ b/frappe/public/scss/common/quill.scss @@ -164,7 +164,7 @@ } .ql-editor td { - border: 1px solid var(--border-color); + border: 1px solid var(---dark-border-color); } .ql-editor blockquote { From 835ed181b5d57ed26b163403bdd8eb3872a8212a Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 7 May 2021 10:34:17 +0530 Subject: [PATCH 34/46] test: delete created test_app in tearDownClass --- frappe/tests/test_boilerplate.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frappe/tests/test_boilerplate.py b/frappe/tests/test_boilerplate.py index 14741dc29e..7dd704aad1 100644 --- a/frappe/tests/test_boilerplate.py +++ b/frappe/tests/test_boilerplate.py @@ -1,4 +1,5 @@ import os +import shutil import unittest from unittest.mock import patch @@ -7,6 +8,14 @@ from frappe.utils.boilerplate import make_boilerplate class TestBoilerPlate(unittest.TestCase): + @classmethod + def tearDownClass(cls): + + bench_path = frappe.utils.get_bench_path() + test_app_dir = os.path.join(bench_path, "apps", "test_app") + if os.path.exists(test_app_dir): + shutil.rmtree(test_app_dir) + def test_create_app(self): title = "Test App" description = "Test app for unit testing" From e9a3569bf78792cc9b03fe2027a7c8f3ae396311 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Fri, 7 May 2021 10:34:46 +0530 Subject: [PATCH 35/46] test: check if created python files are parsable --- frappe/tests/test_boilerplate.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frappe/tests/test_boilerplate.py b/frappe/tests/test_boilerplate.py index 7dd704aad1..4ae78c94de 100644 --- a/frappe/tests/test_boilerplate.py +++ b/frappe/tests/test_boilerplate.py @@ -1,3 +1,5 @@ +import ast +import glob import os import shutil import unittest @@ -74,3 +76,13 @@ class TestBoilerPlate(unittest.TestCase): for path in all_paths: self.assertTrue(os.path.exists(path), msg=f"{path} should exist in new app") + + # check if python files are parsable + python_files = glob.glob(new_app_dir + "**/*.py", recursive=True) + + for python_file in python_files: + with open(python_file) as p: + try: + ast.parse(p.read()) + except Exception as e: + self.fail(f"Can't parse python file in new app: {python_file}\n" + str(e)) From a2bb92f6a43fd6aeba9beef5a8101d45ccbb95f3 Mon Sep 17 00:00:00 2001 From: Suraj Shetty <13928957+surajshetty3416@users.noreply.github.com> Date: Fri, 7 May 2021 11:40:42 +0530 Subject: [PATCH 36/46] fix: Typo --- frappe/public/scss/common/quill.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/scss/common/quill.scss b/frappe/public/scss/common/quill.scss index 67cf369db8..f96a66020d 100644 --- a/frappe/public/scss/common/quill.scss +++ b/frappe/public/scss/common/quill.scss @@ -164,7 +164,7 @@ } .ql-editor td { - border: 1px solid var(---dark-border-color); + border: 1px solid var(--dark-border-color); } .ql-editor blockquote { From 926d13e69ef88309f409446d2dc1c2e3f65ab0f3 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 7 May 2021 18:02:25 +0530 Subject: [PATCH 37/46] fix: Skip field-method conflicts validation on new docs --- frappe/core/doctype/doctype/doctype.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index 2cdccda8e1..e0b9d15114 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -70,7 +70,6 @@ class DocType(Document): validate_series(self) self.validate_document_type() validate_fields(self) - self.validate_field_name_conflicts() if not self.istable: validate_permissions(self) @@ -84,6 +83,7 @@ class DocType(Document): if not self.is_new(): self.before_update = frappe.get_doc('DocType', self.name) self.setup_fields_to_fetch() + self.validate_field_name_conflicts() check_email_append_to(self) From e7552413b4212e1daf47660b72838d1713c6a9bd Mon Sep 17 00:00:00 2001 From: shariquerik Date: Fri, 7 May 2021 18:10:59 +0530 Subject: [PATCH 38/46] refactor: minor fix --- frappe/model/naming.py | 1 + frappe/tests/test_naming.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index 1cfcd56350..dbe77562d0 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -203,6 +203,7 @@ def revert_series_if_last(key, name, doc=None): prefix, hashes = key.rsplit(".", 1) if "#" not in hashes: key = key.rsplit(".") + # get the hash part from the key hash = list(filter(re.compile(".*#").match, key))[0] if not hash: return diff --git a/frappe/tests/test_naming.py b/frappe/tests/test_naming.py index f808d06db3..66d48e3612 100644 --- a/frappe/tests/test_naming.py +++ b/frappe/tests/test_naming.py @@ -70,9 +70,9 @@ class TestNaming(unittest.TestCase): name = 'TEST-{}-00001'.format(year) frappe.db.sql("""INSERT INTO `tabSeries` (name, current) values (%s, 1)""", (series,)) revert_series_if_last(key, name) - count = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] + current_index = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] - self.assertEqual(count.get('current'), 0) + self.assertEqual(current_index.get('current'), 0) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) series = 'TEST-{}-'.format(year) @@ -80,9 +80,9 @@ class TestNaming(unittest.TestCase): name = 'TEST-{}-00002'.format(year) frappe.db.sql("""INSERT INTO `tabSeries` (name, current) values (%s, 2)""", (series,)) revert_series_if_last(key, name) - count = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] + current_index = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] - self.assertEqual(count.get('current'), 1) + self.assertEqual(current_index.get('current'), 1) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) series = 'TEST-' @@ -91,9 +91,9 @@ class TestNaming(unittest.TestCase): frappe.db.sql("DELETE FROM `tabSeries` WHERE `name`=%s", series) frappe.db.sql("""INSERT INTO `tabSeries` (name, current) values (%s, 3)""", (series,)) revert_series_if_last(key, name) - count = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] + current_index = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] - self.assertEqual(count.get('current'), 2) + self.assertEqual(current_index.get('current'), 2) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) series = 'TEST1-' @@ -102,9 +102,9 @@ class TestNaming(unittest.TestCase): frappe.db.sql("DELETE FROM `tabSeries` WHERE `name`=%s", series) frappe.db.sql("""INSERT INTO `tabSeries` (name, current) values (%s, 3)""", (series,)) revert_series_if_last(key, name) - count = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] + current_index = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] - self.assertEqual(count.get('current'), 2) + self.assertEqual(current_index.get('current'), 2) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) series = '' @@ -113,7 +113,7 @@ class TestNaming(unittest.TestCase): frappe.db.sql("DELETE FROM `tabSeries` WHERE `name`=%s", series) frappe.db.sql("""INSERT INTO `tabSeries` (name, current) values (%s, 3)""", (series,)) revert_series_if_last(key, name) - count = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] + current_index = frappe.db.sql("""SELECT current from `tabSeries` where name = %s""", series, as_dict=True)[0] - self.assertEqual(count.get('current'), 2) + self.assertEqual(current_index.get('current'), 2) frappe.db.sql("""delete from `tabSeries` where name = %s""", series) From c84feb16e9e456cdd040f20a6ac154d3a1d760c0 Mon Sep 17 00:00:00 2001 From: gavin Date: Sat, 8 May 2021 12:39:27 +0530 Subject: [PATCH 39/46] fix: Avoid possible whitespace bug * Handles semgrep warning * Changed "" to None as a precaution against future whitespace bugs via human error Co-authored-by: Ankush Menat --- frappe/core/doctype/doctype/doctype.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index e0b9d15114..6eef5a4023 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -123,7 +123,7 @@ class DocType(Document): } for docfield in self.get("fields") or []: - conflict_type = "" + conflict_type = None field = docfield.fieldname field_label = docfield.label or docfield.fieldname From bf33a80042cc76e2cbc36991330d7dbbd7a33db4 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 3 May 2021 17:14:45 +0530 Subject: [PATCH 40/46] fix(setup): do not show messsage when exception is handled (cherry picked from commit ac9fe71733af5fe944cbca62dd688d2e49254497) --- frappe/translate.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frappe/translate.py b/frappe/translate.py index 4baf4bdd89..5c5480d0a2 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -540,8 +540,12 @@ def extract_messages_from_code(code): try: code = frappe.as_unicode(render_include(code)) - except (TemplateError, ImportError, InvalidIncludePath, IOError): - # Exception will occur when it encounters John Resig's microtemplating code + + # Exception will occur when it encounters John Resig's microtemplating code + except (TemplateError, ImportError, InvalidIncludePath, IOError) as e: + if isinstance(e, InvalidIncludePath): + frappe.clear_last_message() + pass messages = [] From 0bbf7c89bfdb8919c93b9da48ba45ee36bf81177 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Sat, 8 May 2021 17:38:38 +0550 Subject: [PATCH 41/46] bumped to version 13.2.2 --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 12346d78c3..1a495d8ce1 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -34,7 +34,7 @@ if PY2: reload(sys) sys.setdefaultencoding("utf-8") -__version__ = '13.2.1' +__version__ = '13.2.2' __title__ = "Frappe Framework" From 8995e8e9db83b40e9df19433b77ec7ee853eaf18 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Sun, 9 May 2021 06:24:20 +0530 Subject: [PATCH 42/46] refactor: remove unnecessary code --- frappe/model/naming.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index dbe77562d0..ade4eaad95 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -202,13 +202,13 @@ def revert_series_if_last(key, name, doc=None): if ".#" in key: prefix, hashes = key.rsplit(".", 1) if "#" not in hashes: - key = key.rsplit(".") + key = key.split(".") # get the hash part from the key hash = list(filter(re.compile(".*#").match, key))[0] if not hash: return name = name.replace(hashes, "") - prefix, hashes = key[:key.index(hash)+1] + prefix = prefix.replace(".{}".format(hash), "") else: prefix = key From 36d6d224df87e03e6247afc4ec873fb8058ba000 Mon Sep 17 00:00:00 2001 From: Suraj Shetty Date: Mon, 10 May 2021 12:18:37 +0530 Subject: [PATCH 43/46] fix: Do not skip text in save while using shortcut --- frappe/public/js/frappe/desk.js | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/frappe/public/js/frappe/desk.js b/frappe/public/js/frappe/desk.js index 6bcd20c494..d6cb7f5507 100644 --- a/frappe/public/js/frappe/desk.js +++ b/frappe/public/js/frappe/desk.js @@ -474,14 +474,19 @@ frappe.Application = Class.extend({ $('').appendTo("head"); }, trigger_primary_action: function() { - if(window.cur_dialog && cur_dialog.display) { - // trigger primary - cur_dialog.get_primary_btn().trigger("click"); - } else if(cur_frm && cur_frm.page.btn_primary.is(':visible')) { - cur_frm.page.btn_primary.trigger('click'); - } else if(frappe.container.page.save_action) { - frappe.container.page.save_action(); - } + // to trigger change event on active input before triggering primary action + $(document.activeElement).blur(); + // wait for possible JS validations triggered after blur (it might change primary button) + setTimeout(() => { + if (window.cur_dialog && cur_dialog.display) { + // trigger primary + cur_dialog.get_primary_btn().trigger("click"); + } else if (cur_frm && cur_frm.page.btn_primary.is(':visible')) { + cur_frm.page.btn_primary.trigger('click'); + } else if (frappe.container.page.save_action) { + frappe.container.page.save_action(); + } + }, 100); }, set_rtl: function() { From e0efefd9e5756fb3e17fe2891964c9a6d0831607 Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 10 May 2021 12:54:04 +0530 Subject: [PATCH 44/46] refactor: Used re.search also added examples --- frappe/model/naming.py | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index ade4eaad95..c5b2775ec5 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -199,16 +199,39 @@ def getseries(key, digits): def revert_series_if_last(key, name, doc=None): - if ".#" in key: + """ + Reverts the series for particular naming series: + * key is naming series - SINV-.YYYY-.#### + * name is actual name - SINV-2021-0001 + + 1. This function split the key into two parts prefix (SINV-YYYY) & hashes (####). + 2. Use prefix to get the current index of that naming series from Series table + 3. Then revert the current index. + + *For custom naming series:* + 1. hash can exist anywhere, if it exist in hashes then it take normal flow. + 2. If hash doesn't exit in hashes, we get the hash from prefix, then update name and prefix accordingly. + + *Example:* + 1. key = SINV-.YYYY.- + * If key doesn't have hash it will add hash at the end + * prefix will be SINV-YYYY based on this will get current index from Series table. + 2. key = SINV-.####.-2021 + * now prefix = SINV-#### and hashes = 2021 (hash doesn't exist) + * will search hash in key then accordingly get prefix = SINV- + 3. key = ####.-2021 + * prefix = #### and hashes = 2021 (hash doesn't exist) + * will search hash in key then accordingly get prefix = "" + """ + if ".#" in key: prefix, hashes = key.rsplit(".", 1) if "#" not in hashes: - key = key.split(".") # get the hash part from the key - hash = list(filter(re.compile(".*#").match, key))[0] + hash = re.search("#+", key) if not hash: return name = name.replace(hashes, "") - prefix = prefix.replace(".{}".format(hash), "") + prefix = prefix.replace(hash.group(), "") else: prefix = key From 9919ddff2a937555881f43b83b22cd517555a94c Mon Sep 17 00:00:00 2001 From: shariquerik Date: Mon, 10 May 2021 12:56:15 +0530 Subject: [PATCH 45/46] fix: sider fix --- frappe/model/naming.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/model/naming.py b/frappe/model/naming.py index c5b2775ec5..359b8e2367 100644 --- a/frappe/model/naming.py +++ b/frappe/model/naming.py @@ -214,7 +214,7 @@ def revert_series_if_last(key, name, doc=None): *Example:* 1. key = SINV-.YYYY.- - * If key doesn't have hash it will add hash at the end + * If key doesn't have hash it will add hash at the end * prefix will be SINV-YYYY based on this will get current index from Series table. 2. key = SINV-.####.-2021 * now prefix = SINV-#### and hashes = 2021 (hash doesn't exist) From 67ceab88ff827c19e2880f03271e3ba8c254f50f Mon Sep 17 00:00:00 2001 From: Raffael Meyer <14891507+barredterra@users.noreply.github.com> Date: Tue, 11 May 2021 08:58:15 +0200 Subject: [PATCH 46/46] fix: translations (#12942) * fix: get_messages_from_include_files * feat: include labels of navbar items * refactor: strip -> lstrip Co-authored-by: gavin --- frappe/translate.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/frappe/translate.py b/frappe/translate.py index aeca758a9d..1d8b1234c7 100644 --- a/frappe/translate.py +++ b/frappe/translate.py @@ -98,6 +98,7 @@ def get_dict(fortype, name=None): translation_assets = cache.hget("translation_assets", frappe.local.lang, shared=True) or {} if not asset_key in translation_assets: + messages = [] if fortype=="doctype": messages = get_messages_from_doctype(name) elif fortype=="page": @@ -109,14 +110,12 @@ def get_dict(fortype, name=None): elif fortype=="jsfile": messages = get_messages_from_file(name) elif fortype=="boot": - messages = [] apps = frappe.get_all_apps(True) for app in apps: messages.extend(get_server_messages(app)) - messages = deduplicate_messages(messages) - messages += frappe.db.sql("""select 'navbar', item_label from `tabNavbar Item` where item_label is not null""") - messages = get_messages_from_include_files() + messages += get_messages_from_navbar() + messages += get_messages_from_include_files() messages += frappe.db.sql("select 'Print Format:', name from `tabPrint Format`") messages += frappe.db.sql("select 'DocType:', name from tabDocType") messages += frappe.db.sql("select 'Role:', name from tabRole") @@ -124,6 +123,7 @@ def get_dict(fortype, name=None): messages += frappe.db.sql("select '', format from `tabWorkspace Shortcut` where format is not null") messages += frappe.db.sql("select '', title from `tabOnboarding Step`") + messages = deduplicate_messages(messages) message_dict = make_dict_from_messages(messages, load_user_translation=False) message_dict.update(get_dict_from_hooks(fortype, name)) # remove untranslated @@ -320,10 +320,22 @@ def get_messages_for_app(app, deduplicate=True): # server_messages messages.extend(get_server_messages(app)) + + # messages from navbar settings + messages.extend(get_messages_from_navbar()) + if deduplicate: messages = deduplicate_messages(messages) + return messages + +def get_messages_from_navbar(): + """Return all labels from Navbar Items, as specified in Navbar Settings.""" + labels = frappe.get_all('Navbar Item', filters={'item_label': ('is', 'set')}, pluck='item_label') + return [('Navbar:', label, 'Label of a Navbar Item') for label in labels] + + def get_messages_from_doctype(name): """Extract all translatable messages for a doctype. Includes labels, Python code, Javascript code, html templates""" @@ -490,8 +502,14 @@ def get_server_messages(app): def get_messages_from_include_files(app_name=None): """Returns messages from js files included at time of boot like desk.min.js for desk and web""" messages = [] - for file in (frappe.get_hooks("app_include_js", app_name=app_name) or []) + (frappe.get_hooks("web_include_js", app_name=app_name) or []): - messages.extend(get_messages_from_file(os.path.join(frappe.local.sites_path, file))) + app_include_js = frappe.get_hooks("app_include_js", app_name=app_name) or [] + web_include_js = frappe.get_hooks("web_include_js", app_name=app_name) or [] + include_js = app_include_js + web_include_js + + for js_path in include_js: + relative_path = os.path.join(frappe.local.sites_path, js_path.lstrip('/')) + messages_from_file = get_messages_from_file(relative_path) + messages.extend(messages_from_file) return messages