From 525f1656ad82a71be118ca6d6821f78e85c639ee Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 1 Mar 2022 14:41:40 +0530 Subject: [PATCH 01/16] fix: Double signature in composed Email Re-do of https://github.com/frappe/frappe/pull/12520 Undone by https://github.com/frappe/frappe/pull/12878 Changes done to reflect current state of version-13 Co-authored-by: Suraj Shetty --- frappe/public/js/frappe/views/communication.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index 1d219a7044..4cdc75e8cd 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -750,7 +750,7 @@ frappe.views.CommunicationComposer = class { signature = signature.replace(/\n/g, "
"); } - return "
" + signature; + return "
" + signature; } get_earlier_reply() { From da2dbfaae987f0646113d3b3c644a7991ff09bda Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 1 Mar 2022 22:58:41 +0530 Subject: [PATCH 02/16] refactor!: Deprecate ignore_permissions & flags in communication.make API --- frappe/core/doctype/communication/email.py | 116 ++++++++++++++---- .../doctype/notification/notification.py | 7 +- 2 files changed, 99 insertions(+), 24 deletions(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 46ef7bf5d2..6a71697d37 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -22,12 +22,30 @@ OUTGOING_EMAIL_ACCOUNT_MISSING = _(""" @frappe.whitelist() -def make(doctype=None, name=None, content=None, subject=None, sent_or_received = "Sent", - sender=None, sender_full_name=None, recipients=None, communication_medium="Email", send_email=False, - print_html=None, print_format=None, attachments='[]', send_me_a_copy=False, cc=None, bcc=None, - flags=None, read_receipt=None, print_letterhead=True, email_template=None, communication_type=None, - ignore_permissions=False) -> Dict[str, str]: - """Make a new communication. +def make( + doctype=None, + name=None, + content=None, + subject=None, + sent_or_received="Sent", + sender=None, + sender_full_name=None, + recipients=None, + communication_medium="Email", + send_email=False, + print_html=None, + print_format=None, + attachments="[]", + send_me_a_copy=False, + cc=None, + bcc=None, + read_receipt=None, + print_letterhead=True, + email_template=None, + communication_type=None, + **kwargs, +) -> Dict[str, str]: + """Make a new communication. Checks for email permissions for specified Document. :param doctype: Reference DocType. :param name: Reference Document name. @@ -44,17 +62,69 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = :param send_me_a_copy: Send a copy to the sender (default **False**). :param email_template: Template which is used to compose mail . """ - is_error_report = (doctype=="User" and name==frappe.session.user and subject=="Error Report") - send_me_a_copy = cint(send_me_a_copy) + if kwargs: + from frappe.utils.commands import warn + warn( + f"Options {kwargs} used in frappe.core.doctype.communication.email.make " + "are deprecated or unsupported", + category=DeprecationWarning + ) + + if doctype and name and not frappe.has_permission(doctype=doctype, ptype="email", doc=name): + raise frappe.PermissionError( + f"You are not allowed to send emails related to: {doctype} {name}" + ) + + return _make( + doctype=doctype, + name=name, + content=content, + subject=subject, + sent_or_received=sent_or_received, + sender=sender, + sender_full_name=sender_full_name, + recipients=recipients, + communication_medium=communication_medium, + send_email=send_email, + print_html=print_html, + print_format=print_format, + attachments=attachments, + send_me_a_copy=cint(send_me_a_copy), + cc=cc, + bcc=bcc, + read_receipt=read_receipt, + print_letterhead=print_letterhead, + email_template=email_template, + communication_type=communication_type, + ) - if not ignore_permissions: - if doctype and name and not is_error_report and not frappe.has_permission(doctype, "email", name) and not (flags or {}).get('ignore_doctype_permissions'): - raise frappe.PermissionError("You are not allowed to send emails related to: {doctype} {name}".format( - doctype=doctype, name=name)) - if not sender: - sender = get_formatted_email(frappe.session.user) +def _make( + doctype=None, + name=None, + content=None, + subject=None, + sent_or_received="Sent", + sender=None, + sender_full_name=None, + recipients=None, + communication_medium="Email", + send_email=False, + print_html=None, + print_format=None, + attachments="[]", + send_me_a_copy=False, + cc=None, + bcc=None, + read_receipt=None, + print_letterhead=True, + email_template=None, + communication_type=None, +) -> Dict[str, str]: + """Internal method to make a new communication that ignores Permission checks. + """ + sender = sender or get_formatted_email(frappe.session.user) recipients = list_to_str(recipients) if isinstance(recipients, list) else recipients cc = list_to_str(cc) if isinstance(cc, list) else cc bcc = list_to_str(bcc) if isinstance(bcc, list) else bcc @@ -87,17 +157,21 @@ def make(doctype=None, name=None, content=None, subject=None, sent_or_received = if cint(send_email): if not comm.get_outgoing_email_account(): - frappe.throw(msg=OUTGOING_EMAIL_ACCOUNT_MISSING, exc=frappe.OutgoingEmailError) + frappe.throw( + msg=OUTGOING_EMAIL_ACCOUNT_MISSING, exc=frappe.OutgoingEmailError + ) - comm.send_email(print_html=print_html, print_format=print_format, - send_me_a_copy=send_me_a_copy, print_letterhead=print_letterhead) + comm.send_email( + print_html=print_html, + print_format=print_format, + send_me_a_copy=send_me_a_copy, + print_letterhead=print_letterhead, + ) emails_not_sent_to = comm.exclude_emails_list(include_sender=send_me_a_copy) - return { - "name": comm.name, - "emails_not_sent_to": ", ".join(emails_not_sent_to) - } + return {"name": comm.name, "emails_not_sent_to": ", ".join(emails_not_sent_to)} + def validate_email(doc: "Communication") -> None: """Validate Email Addresses of Recipients and CC""" diff --git a/frappe/email/doctype/notification/notification.py b/frappe/email/doctype/notification/notification.py index 2b62530847..bad32fb68f 100644 --- a/frappe/email/doctype/notification/notification.py +++ b/frappe/email/doctype/notification/notification.py @@ -186,7 +186,7 @@ def get_context(context): def send_an_email(self, doc, context): from email.utils import formataddr - from frappe.core.doctype.communication.email import make as make_communication + from frappe.core.doctype.communication.email import _make as make_communication subject = self.subject if "{" in subject: subject = frappe.render_template(self.subject, context) @@ -216,7 +216,8 @@ def get_context(context): # Add mail notification to communication list # No need to add if it is already a communication. if doc.doctype != 'Communication': - make_communication(doctype=doc.doctype, + make_communication( + doctype=doc.doctype, name=doc.name, content=message, subject=subject, @@ -228,7 +229,7 @@ def get_context(context): cc=cc, bcc=bcc, communication_type='Automated Message', - ignore_permissions=True) + ) def send_a_slack_msg(self, doc, context): send_slack_message( From 0ef99c3886020b8bb013ff20ce160bb0b69f5fb6 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 2 Mar 2022 18:51:30 +0530 Subject: [PATCH 03/16] fix: Add signature to Communication.content if not already added This fix adds a signature forcibly if found under the sender's User.email_signature or default outgoing email account's signature field. The previous method of adding a comment into the Email didn't work since Quill would discard comments before setting them. Adding signatures in get_formatted_html didn't seem apt since it's used in QueueBuilder to re-construct the Email before processing the Email Queue. This meant that the email content that was added in the Communication record would not be final. Now, we treat the signature as part of the Communication content. --- .../doctype/communication/communication.py | 38 +++++++++++++++++++ frappe/email/email_body.py | 8 +--- .../public/js/frappe/views/communication.js | 2 +- frappe/templates/emails/standard.html | 1 - requirements.txt | 1 + 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index f89f0d8765..759557ae4c 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -18,6 +18,7 @@ from urllib.parse import unquote from frappe.utils.user import is_system_user from frappe.contacts.doctype.contact.contact import get_contact_name from frappe.automation.doctype.assignment_rule.assignment_rule import apply as apply_assignment_rule +from parse import compile exclude_from_linked_with = True @@ -114,6 +115,43 @@ class Communication(Document, CommunicationEmailMixin): frappe.publish_realtime('new_message', self.as_dict(), user=self.reference_name, after_commit=True) + def set_signature_in_email_content(self): + """Set sender's User.email_signature or default outgoing's EmailAccount.signature to the email + """ + if not self.content: + return + + quill_parser = compile('
{}
') + email_body = quill_parser.parse(self.content) + + if not email_body: + return + + email_body = email_body[0] + + user_email_signature = frappe.db.get_value( + "User", + self.sender, + "email_signature", + ) if self.sender else None + + signature = user_email_signature or frappe.db.get_value( + "Email Account", + {"default_outgoing": 1, "add_signature": 1}, + "signature", + ) + + if not signature: + return + + _signature = quill_parser.parse(signature)[0] if "ql-editor" in signature else None + + if (_signature or signature) not in self.content: + self.content = f'{self.content}


{signature}' + + def before_save(self): + self.set_signature_in_email_content() + def on_update(self): # add to _comment property of the doctype, so it shows up in # comments count for the list view diff --git a/frappe/email/email_body.py b/frappe/email/email_body.py index c25e996bd3..0f45e42aac 100755 --- a/frappe/email/email_body.py +++ b/frappe/email/email_body.py @@ -259,17 +259,12 @@ def get_formatted_html(subject, message, footer=None, print_html=None, email_account = email_account or EmailAccount.find_outgoing(match_by_email=sender) - signature = None - if "" not in message: - signature = get_signature(email_account) - rendered_email = frappe.get_template("templates/emails/standard.html").render({ "brand_logo": get_brand_logo(email_account) if with_container or header else None, "with_container": with_container, "site_url": get_url(), "header": get_header(header), "content": message, - "signature": signature, "footer": get_footer(email_account, footer), "title": subject, "print_html": print_html, @@ -281,8 +276,7 @@ def get_formatted_html(subject, message, footer=None, print_html=None, if unsubscribe_link: html = html.replace("", unsubscribe_link.html) - html = inline_style_in_html(html) - return html + return inline_style_in_html(html) @frappe.whitelist() def get_email_html(template, args, subject, header=None, with_container=False): diff --git a/frappe/public/js/frappe/views/communication.js b/frappe/public/js/frappe/views/communication.js index 4cdc75e8cd..1d219a7044 100755 --- a/frappe/public/js/frappe/views/communication.js +++ b/frappe/public/js/frappe/views/communication.js @@ -750,7 +750,7 @@ frappe.views.CommunicationComposer = class { signature = signature.replace(/\n/g, "
"); } - return "
" + signature; + return "
" + signature; } get_earlier_reply() { diff --git a/frappe/templates/emails/standard.html b/frappe/templates/emails/standard.html index 4a47c9cf90..2a2093e1e9 100644 --- a/frappe/templates/emails/standard.html +++ b/frappe/templates/emails/standard.html @@ -37,7 +37,6 @@

{{ content }}

-

{{ signature }}

diff --git a/requirements.txt b/requirements.txt index ba4a1a598b..c77ab1d424 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,6 +29,7 @@ maxminddb-geolite2==2018.703 num2words~=0.5.10 oauthlib~=3.1.0 openpyxl~=3.0.7 +parse~=1.19.0 passlib~=1.7.4 paytmchecksum~=1.7.0 pdfkit~=0.6.1 From 89a7dac300deb85d3f6aa080c5460f79650a5b6b Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 3 Mar 2022 19:38:34 +0530 Subject: [PATCH 04/16] fix: Add signature only if not Communication.flags.skip_add_signature This is added to handle purposely removed signature or custom signatures added via the Email Composer or via the email.make API --- frappe/core/doctype/communication/communication.py | 3 ++- frappe/core/doctype/communication/email.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/frappe/core/doctype/communication/communication.py b/frappe/core/doctype/communication/communication.py index 759557ae4c..475762f39d 100644 --- a/frappe/core/doctype/communication/communication.py +++ b/frappe/core/doctype/communication/communication.py @@ -150,7 +150,8 @@ class Communication(Document, CommunicationEmailMixin): self.content = f'{self.content}


{signature}' def before_save(self): - self.set_signature_in_email_content() + if not self.flags.skip_add_signature: + self.set_signature_in_email_content() def on_update(self): # add to _comment property of the doctype, so it shows up in diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 6a71697d37..195518f40d 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -147,7 +147,9 @@ def _make( "read_receipt":read_receipt, "has_attachment": 1 if attachments else 0, "communication_type": communication_type, - }).insert(ignore_permissions=True) + }) + comm.flags.skip_add_signature = True + comm.insert(ignore_permissions=True) # if not committed, delayed task doesn't find the communication if attachments: From 6cdab154f511261d9ab70055004b64fbfd77b7ee Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 3 Mar 2022 19:58:08 +0100 Subject: [PATCH 05/16] feat: translate column names in excel file from query report --- frappe/desk/query_report.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/desk/query_report.py b/frappe/desk/query_report.py index a0b0aec255..b344763916 100644 --- a/frappe/desk/query_report.py +++ b/frappe/desk/query_report.py @@ -406,7 +406,7 @@ def build_xlsx_data(columns, data, visible_idx, include_indentation, ignore_visi for column in data.columns: if column.get("hidden"): continue - result[0].append(column.get("label")) + result[0].append(_(column.get("label"))) column_width = cint(column.get('width', 0)) # to convert into scale accepted by openpyxl column_width /= 10 From 8126a18f0517a69445207786a889a9869f160c7f Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Thu, 3 Mar 2022 19:58:20 +0100 Subject: [PATCH 06/16] feat: translate column names in CSV file from query report --- frappe/public/js/frappe/views/reports/query_report.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/views/reports/query_report.js b/frappe/public/js/frappe/views/reports/query_report.js index 7c12809fcd..4c4e02bf41 100644 --- a/frappe/public/js/frappe/views/reports/query_report.js +++ b/frappe/public/js/frappe/views/reports/query_report.js @@ -1343,7 +1343,7 @@ frappe.views.QueryReport = class QueryReport extends frappe.views.BaseList { if (file_format === 'CSV') { const column_row = this.columns.reduce((acc, col) => { if (!col.hidden) { - acc.push(col.label); + acc.push(__(col.label)); } return acc; }, []); From 12ae7b9239352961badbe0425adb0fd76fdab2d0 Mon Sep 17 00:00:00 2001 From: Jannat Patel <31363128+pateljannat@users.noreply.github.com> Date: Mon, 7 Mar 2022 10:38:25 +0530 Subject: [PATCH 07/16] fix: User Account Auto Deletion in Hours (#16135) * fix: data-deletion-in-hours * test: process_auto_request function * fix: import and comment * fix: patch for auto account deletion hours Co-authored-by: gavin --- frappe/hooks.py | 6 +++--- frappe/patches.txt | 1 + .../update_auto_account_deletion_duration.py | 5 +++++ .../personal_data_deletion_request.py | 6 +++--- .../test_personal_data_deletion_request.py | 16 ++++++++++++++-- .../website_settings/website_settings.json | 8 ++++---- .../request_to_delete_data.js | 2 +- 7 files changed, 31 insertions(+), 13 deletions(-) create mode 100644 frappe/patches/v14_0/update_auto_account_deletion_duration.py diff --git a/frappe/hooks.py b/frappe/hooks.py index 4895c97200..be1b0134c1 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -221,7 +221,8 @@ scheduler_events = { "frappe.deferred_insert.save_to_db", "frappe.desk.form.document_follow.send_hourly_updates", "frappe.integrations.doctype.google_calendar.google_calendar.sync", - "frappe.email.doctype.newsletter.newsletter.send_scheduled_email" + "frappe.email.doctype.newsletter.newsletter.send_scheduled_email", + "frappe.website.doctype.personal_data_deletion_request.personal_data_deletion_request.process_data_deletion_request" ], "daily": [ "frappe.email.queue.set_expiry_for_email_queue", @@ -240,8 +241,7 @@ scheduler_events = { "frappe.automation.doctype.auto_repeat.auto_repeat.set_auto_repeat_as_completed", "frappe.email.doctype.unhandled_email.unhandled_email.remove_old_unhandled_emails", "frappe.core.doctype.prepared_report.prepared_report.delete_expired_prepared_reports", - "frappe.core.doctype.log_settings.log_settings.run_log_clean_up", - "frappe.website.doctype.personal_data_deletion_request.personal_data_deletion_request.process_data_deletion_request" + "frappe.core.doctype.log_settings.log_settings.run_log_clean_up" ], "daily_long": [ "frappe.integrations.doctype.dropbox_settings.dropbox_settings.take_backups_daily", diff --git a/frappe/patches.txt b/frappe/patches.txt index c889d9a4da..a666480c90 100644 --- a/frappe/patches.txt +++ b/frappe/patches.txt @@ -197,3 +197,4 @@ frappe.patches.v14_0.copy_mail_data #08.03.21 frappe.patches.v14_0.update_github_endpoints #08-11-2021 frappe.patches.v14_0.remove_db_aggregation frappe.patches.v14_0.update_color_names_in_kanban_board_column +frappe.patches.v14_0.update_auto_account_deletion_duration diff --git a/frappe/patches/v14_0/update_auto_account_deletion_duration.py b/frappe/patches/v14_0/update_auto_account_deletion_duration.py new file mode 100644 index 0000000000..74957066e6 --- /dev/null +++ b/frappe/patches/v14_0/update_auto_account_deletion_duration.py @@ -0,0 +1,5 @@ +import frappe + +def execute(): + days = frappe.db.get_single_value("Website Settings", "auto_account_deletion") + frappe.db.set_value("Website Settings", None, "auto_account_deletion", days * 24) diff --git a/frappe/website/doctype/personal_data_deletion_request/personal_data_deletion_request.py b/frappe/website/doctype/personal_data_deletion_request/personal_data_deletion_request.py index 3699cdfbbd..e2f583fd48 100644 --- a/frappe/website/doctype/personal_data_deletion_request/personal_data_deletion_request.py +++ b/frappe/website/doctype/personal_data_deletion_request/personal_data_deletion_request.py @@ -7,7 +7,7 @@ import re import frappe from frappe import _ from frappe.model.document import Document -from frappe.utils import get_fullname, date_diff, get_datetime +from frappe.utils import get_fullname, time_diff_in_hours, get_datetime from frappe.utils.user import get_system_managers from frappe.utils.verified_command import get_signed_params, verify_request import json @@ -353,8 +353,8 @@ def process_data_deletion_request(): for request in requests: doc = frappe.get_doc("Personal Data Deletion Request", request) - if date_diff(get_datetime(), doc.creation) >= auto_account_deletion: - doc.add_comment("Comment", _("The User record for this request has been auto-deleted due to inactivity.")) + if time_diff_in_hours(get_datetime(), doc.creation) >= auto_account_deletion: + doc.add_comment("Comment", _("The User record for this request has been auto-deleted due to inactivity by system admins.")) doc.trigger_data_deletion() def remove_unverified_record(): diff --git a/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py b/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py index 27dcfe5858..675a891130 100644 --- a/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py +++ b/frappe/website/doctype/personal_data_deletion_request/test_personal_data_deletion_request.py @@ -4,10 +4,10 @@ import frappe import unittest from frappe.website.doctype.personal_data_deletion_request.personal_data_deletion_request import ( - remove_unverified_record, + remove_unverified_record, process_data_deletion_request ) from frappe.website.doctype.personal_data_download_request.test_personal_data_download_request import ( - create_user_if_not_exists, + create_user_if_not_exists ) from datetime import datetime, timedelta @@ -58,3 +58,15 @@ class TestPersonalDataDeletionRequest(unittest.TestCase): self.assertFalse( frappe.db.exists("Personal Data Deletion Request", self.delete_request.name) ) + + def test_process_auto_request(self): + frappe.db.set_value("Website Settings", None, "auto_account_deletion", "1") + date_time_obj = datetime.strptime( + self.delete_request.creation, "%Y-%m-%d %H:%M:%S.%f" + ) + timedelta(hours=-2) + self.delete_request.db_set("creation", date_time_obj) + self.delete_request.db_set("status", "Pending Approval") + + process_data_deletion_request() + self.delete_request.reload() + self.assertEqual(self.delete_request.status, "Deleted") diff --git a/frappe/website/doctype/website_settings/website_settings.json b/frappe/website/doctype/website_settings/website_settings.json index 2a6b3dc1fb..3b199a4b58 100644 --- a/frappe/website/doctype/website_settings/website_settings.json +++ b/frappe/website/doctype/website_settings/website_settings.json @@ -404,10 +404,10 @@ "label": "Show Account Deletion Link in My Account Page" }, { - "default": "3", + "default": "72", "fieldname": "auto_account_deletion", "fieldtype": "Int", - "label": "Auto Account Deletion within (Days)" + "label": "Auto Account Deletion within (Hours)" }, { "fieldname": "footer_powered", @@ -421,7 +421,7 @@ "issingle": 1, "links": [], "max_attachments": 10, - "modified": "2022-02-28 23:05:42.493192", + "modified": "2022-02-24 15:37:22.360138", "modified_by": "Administrator", "module": "Website", "name": "Website Settings", @@ -446,4 +446,4 @@ "sort_order": "ASC", "states": [], "track_changes": 1 -} \ No newline at end of file +} diff --git a/frappe/website/web_form/request_to_delete_data/request_to_delete_data.js b/frappe/website/web_form/request_to_delete_data/request_to_delete_data.js index 1b9e9ad79b..731fe29cef 100644 --- a/frappe/website/web_form/request_to_delete_data/request_to_delete_data.js +++ b/frappe/website/web_form/request_to_delete_data/request_to_delete_data.js @@ -5,7 +5,7 @@ frappe.ready(function() { callback: (data) => { if (data.message) { const intro_wrapper = $('#introduction .ql-editor.read-mode'); - const sla_description = __("Note: Your request for account deletion will be fulfilled within {0} days.", [data.message]); + const sla_description = __("Note: Your request for account deletion will be fulfilled within {0} hours.", [data.message]); const sla_description_wrapper = `
${sla_description}`; intro_wrapper.html(intro_wrapper.html() + sla_description_wrapper); } From def10e1e139bd2689bce0e0900d751fb136e8b94 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 7 Mar 2022 12:41:37 +0530 Subject: [PATCH 08/16] chore: remove unused variable `doc_before_save` --- frappe/model/document.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/frappe/model/document.py b/frappe/model/document.py index dc0fd2caf0..3c38ff3442 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -1003,8 +1003,6 @@ class Document(BaseDocument): - `on_cancel` for **Cancel** - `update_after_submit` for **Update after Submit**""" - doc_before_save = self.get_doc_before_save() - if self._action=="save": self.run_method("on_update") elif self._action=="submit": From f21b3434882091ad877017e0c582537e92c3bb6e Mon Sep 17 00:00:00 2001 From: Mohammad Hasnain Mohsin Rajan Date: Mon, 7 Mar 2022 13:01:31 +0530 Subject: [PATCH 09/16] perf: role instead of users in workflow actions (#15789) Create a single workflow action document for pending transitions. The Roles that are allowed to make the transition are stored in a Table MultiSelect field. Workflow action doctype is filtered on the basis of applied roles on the visiting user. The user column is retained for backward compatibility. BREAKING CHANGE: The user field will no longer be used, A new field that stores applicable roles will be used instead. --- .../doctype/workflow/test_workflow.py | 51 +++++- .../workflow_action/workflow_action.json | 44 +++-- .../workflow_action/workflow_action.py | 167 ++++++++++++++---- .../__init__.py | 0 .../workflow_action_permitted_role.json | 33 ++++ .../workflow_action_permitted_role.py | 8 + 6 files changed, 255 insertions(+), 48 deletions(-) create mode 100644 frappe/workflow/doctype/workflow_action_permitted_role/__init__.py create mode 100644 frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.json create mode 100644 frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.py diff --git a/frappe/workflow/doctype/workflow/test_workflow.py b/frappe/workflow/doctype/workflow/test_workflow.py index d2d85e696b..14ecdfb5a1 100644 --- a/frappe/workflow/doctype/workflow/test_workflow.py +++ b/frappe/workflow/doctype/workflow/test_workflow.py @@ -5,6 +5,7 @@ import unittest from frappe.utils import random_string from frappe.model.workflow import apply_workflow, WorkflowTransitionError, WorkflowPermissionError, get_common_transition_actions from frappe.test_runner import make_test_records +from frappe.query_builder import DocType class TestWorkflow(unittest.TestCase): @@ -15,9 +16,31 @@ class TestWorkflow(unittest.TestCase): def setUp(self): self.workflow = create_todo_workflow() frappe.set_user('Administrator') + if self._testMethodName == "test_if_workflow_actions_were_processed_using_user": + if not frappe.db.has_column('Workflow Action', 'user'): + # mariadb would raise this statement would create an implicit commit + # if we do not commit before alter statement + # nosemgrep + frappe.db.commit() + frappe.db.multisql({ + 'mariadb': 'ALTER TABLE `tabWorkflow Action` ADD COLUMN user varchar(140)', + 'postgres': 'ALTER TABLE "tabWorkflow Action" ADD COLUMN "user" varchar(140)' + }) + frappe.cache().delete_value('table_columns') def tearDown(self): frappe.delete_doc('Workflow', 'Test ToDo') + if self._testMethodName == "test_if_workflow_actions_were_processed_using_user": + if frappe.db.has_column('Workflow Action', 'user'): + # mariadb would raise this statement would create an implicit commit + # if we do not commit before alter statement + # nosemgrep + frappe.db.commit() + frappe.db.multisql({ + 'mariadb': 'ALTER TABLE `tabWorkflow Action` DROP COLUMN user', + 'postgres': 'ALTER TABLE "tabWorkflow Action" DROP COLUMN "user"' + }) + frappe.cache().delete_value('table_columns') def test_default_condition(self): '''test default condition is set''' @@ -75,7 +98,7 @@ class TestWorkflow(unittest.TestCase): actions = get_common_transition_actions([todo1, todo2], 'ToDo') self.assertListEqual(actions, ['Review']) - def test_if_workflow_actions_were_processed(self): + def test_if_workflow_actions_were_processed_using_role(self): frappe.db.delete("Workflow Action") user = frappe.get_doc('User', 'test2@example.com') user.add_roles('Test Approver', 'System Manager') @@ -93,6 +116,32 @@ class TestWorkflow(unittest.TestCase): self.assertEqual(workflow_actions[0].status, 'Completed') frappe.set_user('Administrator') + def test_if_workflow_actions_were_processed_using_user(self): + frappe.db.delete("Workflow Action") + + user = frappe.get_doc('User', 'test2@example.com') + user.add_roles('Test Approver', 'System Manager') + frappe.set_user('test2@example.com') + + doc = self.test_default_condition() + workflow_actions = frappe.get_all('Workflow Action', fields=['*']) + self.assertEqual(len(workflow_actions), 1) + + # test if status of workflow actions are updated on approval + WorkflowAction = DocType("Workflow Action") + WorkflowActionPermittedRole = DocType("Workflow Action Permitted Role") + frappe.qb.update(WorkflowAction).set(WorkflowAction.user, 'test2@example.com').run() + frappe.qb.update(WorkflowActionPermittedRole).set(WorkflowActionPermittedRole.role, '').run() + + self.test_approve(doc) + + user.remove_roles('Test Approver', 'System Manager') + workflow_actions = frappe.get_all('Workflow Action', fields=['status']) + self.assertEqual(len(workflow_actions), 1) + self.assertEqual(workflow_actions[0].status, 'Completed') + frappe.set_user('Administrator') + + def test_update_docstatus(self): todo = create_new_todo() apply_workflow(todo, 'Approve') diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.json b/frappe/workflow/doctype/workflow_action/workflow_action.json index f1290d001f..aeb60feceb 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.json +++ b/frappe/workflow/doctype/workflow_action/workflow_action.json @@ -8,9 +8,12 @@ "status", "reference_name", "reference_doctype", - "user", "workflow_state", - "completed_by" + "column_break_5", + "completed_by_role", + "completed_by", + "permitted_roles", + "user" ], "fields": [ { @@ -24,16 +27,14 @@ "fieldname": "reference_name", "fieldtype": "Dynamic Link", "label": "Reference Name", - "options": "reference_doctype", - "search_index": 1 + "options": "reference_doctype" }, { "fieldname": "reference_doctype", "fieldtype": "Link", "in_list_view": 1, "label": "Reference Document Type", - "options": "DocType", - "search_index": 1 + "options": "DocType" }, { "fieldname": "user", @@ -47,18 +48,38 @@ "fieldname": "workflow_state", "fieldtype": "Data", "hidden": 1, - "label": "Workflow State", - "search_index": 1 + "label": "Workflow State" }, { + "depends_on": "eval: doc.completed_by", "fieldname": "completed_by", "fieldtype": "Link", - "label": "Completed By", - "options": "User" + "label": "Completed By User", + "options": "User", + "read_only": 1 + }, + { + "fieldname": "column_break_5", + "fieldtype": "Column Break" + }, + { + "depends_on": "eval: doc.completed_by_role", + "fieldname": "completed_by_role", + "fieldtype": "Link", + "label": "Completed By Role", + "options": "Role", + "read_only": 1 + }, + { + "fieldname": "permitted_roles", + "fieldtype": "Table MultiSelect", + "label": "Permitted Roles", + "options": "Workflow Action Permitted Role", + "read_only": 1 } ], "links": [], - "modified": "2021-07-01 09:07:52.848618", + "modified": "2022-02-23 21:06:45.122258", "modified_by": "Administrator", "module": "Workflow", "name": "Workflow Action", @@ -72,6 +93,7 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "title_field": "reference_name", "track_changes": 1 } \ No newline at end of file diff --git a/frappe/workflow/doctype/workflow_action/workflow_action.py b/frappe/workflow/doctype/workflow_action/workflow_action.py index c8561fe922..0ab8924a6b 100644 --- a/frappe/workflow/doctype/workflow_action/workflow_action.py +++ b/frappe/workflow/doctype/workflow_action/workflow_action.py @@ -13,24 +13,46 @@ from frappe.model.workflow import apply_workflow, get_workflow_name, has_approva from frappe.desk.notifications import clear_doctype_notifications from frappe.utils.user import get_users_with_role from frappe.utils.data import get_link_to_form +from frappe.query_builder import DocType class WorkflowAction(Document): pass def on_doctype_update(): - frappe.db.add_index("Workflow Action", ["status", "user"]) + # The search order in any use case is no ["reference_name", "reference_doctype", "status"] + # The index scan would happen from left to right + # so even if status is not in the where clause the index will be used + frappe.db.add_index("Workflow Action", ["reference_name", "reference_doctype", "status"]) def get_permission_query_conditions(user): if not user: user = frappe.session.user if user == "Administrator": return "" - return "(`tabWorkflow Action`.`user`='{user}')".format(user=user) + roles = frappe.get_roles(user) + + WorkflowAction = DocType("Workflow Action") + WorkflowActionPermittedRole = DocType("Workflow Action Permitted Role") + + permitted_workflow_actions = (frappe.qb.from_(WorkflowAction) + .join(WorkflowActionPermittedRole) + .on(WorkflowAction.name == WorkflowActionPermittedRole.parent) + .select(WorkflowAction.name) + .where(WorkflowActionPermittedRole.role.isin(roles)) + ).get_sql() + + return f"""(`tabWorkflow Action`.`name` in ({permitted_workflow_actions}) + or `tabWorkflow Action`.`user`='{user}') + and `tabWorkflow Action`.`status`='Open'""" def has_permission(doc, user): - if user not in ['Administrator', doc.user]: - return False + + user_roles = set(frappe.get_roles(user)) + + permitted_roles = {permitted_role.role for permitted_role in doc.permitted_roles} + + return user == "Administrator" or user_roles.intersection(permitted_roles) def process_workflow_actions(doc, state): workflow = get_workflow_name(doc.get('doctype')) @@ -42,19 +64,18 @@ def process_workflow_actions(doc, state): if is_workflow_action_already_created(doc): return - clear_old_workflow_actions(doc) - update_completed_workflow_actions(doc) + update_completed_workflow_actions(doc, workflow=workflow, workflow_state=get_doc_workflow_state(doc)) clear_doctype_notifications('Workflow Action') next_possible_transitions = get_next_possible_transitions(workflow, get_doc_workflow_state(doc), doc) if not next_possible_transitions: return - user_data_map = get_users_next_action_data(next_possible_transitions, doc) + user_data_map, roles = get_users_next_action_data(next_possible_transitions, doc) if not user_data_map: return - create_workflow_actions_for_users(user_data_map.keys(), doc) + create_workflow_actions_for_roles(roles, doc) if send_email_alert(workflow): enqueue(send_workflow_action_email, queue='short', users_data=list(user_data_map.values()), doc=doc) @@ -132,20 +153,85 @@ def return_link_expired_page(doc, doc_workflow_state): frappe.bold(frappe.get_value('User', doc.get("modified_by"), 'full_name')) ), indicator_color='blue') -def clear_old_workflow_actions(doc, user=None): + +def update_completed_workflow_actions(doc, user=None, workflow=None, workflow_state=None): + allowed_roles = get_allowed_roles(user, workflow, workflow_state) + # There is no transaction leading upto this state + # so no older actions to complete + if not allowed_roles: + return + if workflow_action := get_workflow_action_by_role(doc, allowed_roles): + update_completed_workflow_actions_using_role(doc, user, allowed_roles, workflow_action) + else: + # backwards compatibility + # for workflow actions saved using user + clear_old_workflow_actions_using_user(doc, user) + update_completed_workflow_actions_using_user(doc, user) + +def get_allowed_roles(user, workflow, workflow_state): user = user if user else frappe.session.user - frappe.db.delete("Workflow Action", { - "reference_doctype": doc.get("doctype"), - "reference_name": doc.get("name"), - "user": ("!=", user), - "status": "Open" - }) -def update_completed_workflow_actions(doc, user=None): + allowed_roles = frappe.get_all('Workflow Transition', + fields='allowed', + filters=[ + ['parent', '=', workflow], + ['next_state', '=', workflow_state] + ], + pluck = 'allowed') + + user_roles = set(frappe.get_roles(user)) + return set(allowed_roles).intersection(user_roles) + +def get_workflow_action_by_role(doc, allowed_roles): + WorkflowAction = DocType("Workflow Action") + WorkflowActionPermittedRole = DocType("Workflow Action Permitted Role") + return (frappe.qb.from_(WorkflowAction).join(WorkflowActionPermittedRole) + .on(WorkflowAction.name == WorkflowActionPermittedRole.parent) + .select(WorkflowAction.name, WorkflowActionPermittedRole.role) + .where((WorkflowAction.reference_name == doc.get('name')) + & (WorkflowAction.reference_doctype == doc.get('doctype')) + & (WorkflowAction.status == 'Open') + & (WorkflowActionPermittedRole.role.isin(list(allowed_roles)))) + .orderby(WorkflowActionPermittedRole.role).limit(1)).run(as_dict=True) + +def update_completed_workflow_actions_using_role(doc, user=None, allowed_roles = set(), workflow_action=None): user = user if user else frappe.session.user - frappe.db.sql("""UPDATE `tabWorkflow Action` SET `status`='Completed', `completed_by`=%s - WHERE `reference_doctype`=%s AND `reference_name`=%s AND `user`=%s AND `status`='Open'""", - (user, doc.get('doctype'), doc.get('name'), user)) + WorkflowAction = DocType("Workflow Action") + + if not workflow_action: + return + + (frappe.qb.update(WorkflowAction) + .set(WorkflowAction.status, 'Completed') + .set(WorkflowAction.completed_by, user) + .set(WorkflowAction.completed_by_role, workflow_action[0].role) + .where(WorkflowAction.name == workflow_action[0].name) + ).run() + +def clear_old_workflow_actions_using_user(doc, user=None): + user = user if user else frappe.session.user + + if frappe.db.has_column('Workflow Action', 'user'): + frappe.db.delete("Workflow Action", { + "reference_name": doc.get("name"), + "reference_doctype": doc.get("doctype"), + "status": "Open", + "user": ("!=", user) + }) + +def update_completed_workflow_actions_using_user(doc, user=None): + user = user or frappe.session.user + + if frappe.db.has_column('Workflow Action', 'user'): + WorkflowAction = DocType("Workflow Action") + (frappe.qb.update(WorkflowAction) + .set(WorkflowAction.status, 'Completed') + .set(WorkflowAction.completed_by, user) + .where((WorkflowAction.reference_name == doc.get('name')) + & (WorkflowAction.reference_doctype == doc.get('doctype')) + & (WorkflowAction.status == 'Open') + & (WorkflowAction.user == user)) + ).run() def get_next_possible_transitions(workflow_name, state, doc=None): transitions = frappe.get_all('Workflow Transition', @@ -167,8 +253,10 @@ def get_next_possible_transitions(workflow_name, state, doc=None): return transitions_to_return def get_users_next_action_data(transitions, doc): + roles = set() user_data_map = {} for transition in transitions: + roles.add(transition.allowed) users = get_users_with_role(transition.allowed) filtered_users = filter_allowed_users(users, doc, transition) for user in filtered_users: @@ -182,19 +270,24 @@ def get_users_next_action_data(transitions, doc): 'action_name': transition.action, 'action_link': get_workflow_action_url(transition.action, doc, user) })) - return user_data_map + return user_data_map, roles -def create_workflow_actions_for_users(users, doc): - for user in users: - frappe.get_doc({ - 'doctype': 'Workflow Action', - 'reference_doctype': doc.get('doctype'), - 'reference_name': doc.get('name'), - 'workflow_state': get_doc_workflow_state(doc), - 'status': 'Open', - 'user': user - }).insert(ignore_permissions=True) +def create_workflow_actions_for_roles(roles, doc): + workflow_action = frappe.get_doc({ + 'doctype': 'Workflow Action', + 'reference_doctype': doc.get('doctype'), + 'reference_name': doc.get('name'), + 'workflow_state': get_doc_workflow_state(doc), + 'status': 'Open', + }) + + for role in roles: + workflow_action.append('permitted_roles', { + 'role': role + }) + + workflow_action.insert(ignore_permissions=True) def send_workflow_action_email(users_data, doc): common_args = get_common_email_args(doc) @@ -249,18 +342,20 @@ def get_confirm_workflow_action_url(doc, action, user): def is_workflow_action_already_created(doc): return frappe.db.exists({ 'doctype': 'Workflow Action', - 'reference_doctype': doc.get('doctype'), 'reference_name': doc.get('name'), - 'workflow_state': get_doc_workflow_state(doc) + 'reference_doctype': doc.get('doctype'), + 'workflow_state': get_doc_workflow_state(doc), }) def clear_workflow_actions(doctype, name): if not (doctype and name): return - frappe.db.delete("Workflow Action", { - "reference_doctype": doctype, - "reference_name": name - }) + frappe.db.delete("Workflow Action", filters = { + "reference_name": name, + "reference_doctype": doctype, + } + ) + def get_doc_workflow_state(doc): workflow_name = get_workflow_name(doc.get('doctype')) workflow_state_field = get_workflow_state_field(workflow_name) diff --git a/frappe/workflow/doctype/workflow_action_permitted_role/__init__.py b/frappe/workflow/doctype/workflow_action_permitted_role/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.json b/frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.json new file mode 100644 index 0000000000..19b2dcba19 --- /dev/null +++ b/frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.json @@ -0,0 +1,33 @@ +{ + "actions": [], + "allow_rename": 1, + "autoname": "hash", + "creation": "2022-02-21 20:28:05.662187", + "doctype": "DocType", + "editable_grid": 1, + "engine": "InnoDB", + "field_order": [ + "role" + ], + "fields": [ + { + "fieldname": "role", + "fieldtype": "Link", + "label": "Role", + "options": "Role" + } + ], + "index_web_pages_for_search": 1, + "istable": 1, + "links": [], + "modified": "2022-02-21 20:28:05.662187", + "modified_by": "Administrator", + "module": "Workflow", + "name": "Workflow Action Permitted Role", + "naming_rule": "Random", + "owner": "Administrator", + "permissions": [], + "sort_field": "modified", + "sort_order": "DESC", + "states": [] +} \ No newline at end of file diff --git a/frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.py b/frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.py new file mode 100644 index 0000000000..0370f6a4c8 --- /dev/null +++ b/frappe/workflow/doctype/workflow_action_permitted_role/workflow_action_permitted_role.py @@ -0,0 +1,8 @@ +# Copyright (c) 2022, Frappe Technologies and contributors +# For license information, please see license.txt + +# import frappe +from frappe.model.document import Document + +class WorkflowActionPermittedRole(Document): + pass From 6edb1f09e455a72512f23fafaa2860854203d31d Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 7 Mar 2022 15:02:20 +0530 Subject: [PATCH 10/16] refactor: Fetch Linked Documents in single request ;) This action was broken into some heavy UI stuff that seems to be handled from the current UI from what I can tell and a request triggered from the Desk for every suspected linked document (talk about self DOS xD) Removed long dead with_doctype option in get_linked_docs API. How do i know it's been long dead? I've not come across code that won't very obviously break when hit with any non None value. --- frappe/desk/form/linked_with.py | 22 ++-- frappe/public/js/frappe/form/linked_with.js | 107 ++------------------ 2 files changed, 17 insertions(+), 112 deletions(-) diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py index 572d3f2a94..76900565c9 100644 --- a/frappe/desk/form/linked_with.py +++ b/frappe/desk/form/linked_with.py @@ -3,7 +3,7 @@ import json from collections import defaultdict import itertools -from typing import List +from typing import Dict, List import frappe import frappe.desk.form.load @@ -367,7 +367,7 @@ def get_exempted_doctypes(): @frappe.whitelist() -def get_linked_docs(doctype, name, linkinfo=None, for_doctype=None): +def get_linked_docs(doctype: str, name: str, linkinfo: Dict = None) -> Dict[str, List]: if isinstance(linkinfo, str): # additional fields are added in linkinfo linkinfo = json.loads(linkinfo) @@ -377,23 +377,12 @@ def get_linked_docs(doctype, name, linkinfo=None, for_doctype=None): if not linkinfo: return results - if for_doctype: - links = frappe.get_doc(doctype, name).get_link_filters(for_doctype) - - if links: - linkinfo = links - - if for_doctype in linkinfo: - # only get linked with for this particular doctype - linkinfo = { for_doctype: linkinfo.get(for_doctype) } - else: - return results - for dt, link in linkinfo.items(): filters = [] link["doctype"] = dt link_meta_bundle = frappe.desk.form.load.get_meta_bundle(dt) linkmeta = link_meta_bundle[0] + if not linkmeta.get("issingle"): fields = [d.fieldname for d in linkmeta.get("fields", { "in_list_view": 1, @@ -456,6 +445,11 @@ def get_linked_docs(doctype, name, linkinfo=None, for_doctype=None): return results +@frappe.whitelist() +def get(doctype, docname): + linked_doctypes = get_linked_doctypes(doctype=doctype) + return get_linked_docs(doctype=doctype, name=docname, linkinfo=linked_doctypes) + @frappe.whitelist() def get_linked_doctypes(doctype, without_ignore_user_permissions_enabled=False): """add list of doctypes this doctype is 'linked' with. diff --git a/frappe/public/js/frappe/form/linked_with.js b/frappe/public/js/frappe/form/linked_with.js index 20db7bdb7c..c47a6e0c86 100644 --- a/frappe/public/js/frappe/form/linked_with.js +++ b/frappe/public/js/frappe/form/linked_with.js @@ -1,9 +1,8 @@ -// Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors -// MIT License. See license.txt +// Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors +// MIT License. See LICENSE frappe.ui.form.LinkedWith = class LinkedWith { - constructor(opts) { $.extend(this, opts); } @@ -21,29 +20,23 @@ frappe.ui.form.LinkedWith = class LinkedWith { } make_dialog() { - this.dialog = new frappe.ui.Dialog({ title: __("Linked With") }); this.dialog.on_page_show = () => { - // execute ajax calls sequentially - // 1. get linked doctypes - // 2. load all doctypes - // 3. load linked docs - this.get_linked_doctypes() - .then(() => this.load_doctypes()) - .then(() => this.links_not_permitted_or_missing()) - .then(() => this.get_linked_docs()) - .then(() => this.make_html()); + frappe.xcall( + "frappe.desk.form.linked_with.get", + {"doctype": cur_frm.doctype, "docname": cur_frm.docname}, + ).then(r => { + this.frm.__linked_docs = r; + }).then(() => this.make_html()); }; } make_html() { - const linked_docs = this.frm.__linked_docs; - let html = ''; - + const linked_docs = this.frm.__linked_docs; const linked_doctypes = Object.keys(linked_docs); if (linked_doctypes.length === 0) { @@ -63,88 +56,6 @@ frappe.ui.form.LinkedWith = class LinkedWith { $(this.dialog.body).html(html); } - load_doctypes() { - const already_loaded = Object.keys(locals.DocType); - let doctypes_to_load = []; - - if (this.frm.__linked_doctypes) { - doctypes_to_load = - Object.keys(this.frm.__linked_doctypes) - .filter(doctype => !already_loaded.includes(doctype)); - } - - // load all doctypes asynchronously using with_doctype - const promises = doctypes_to_load.map(dt => { - return frappe.model.with_doctype(dt, () => { - if(frappe.listview_settings[dt]) { - // add additional fields to __linked_doctypes - this.frm.__linked_doctypes[dt].add_fields = - frappe.listview_settings[dt].add_fields; - } - }); - }); - - return Promise.all(promises); - } - - links_not_permitted_or_missing() { - let links = null; - - if (this.frm.__linked_doctypes) { - links = - Object.keys(this.frm.__linked_doctypes) - .filter(frappe.model.can_get_report); - } - - let flag; - if(!links) { - $(this.dialog.body).html(`${this.frm.__linked_doctypes - ? __("Not enough permission to see links") - : __("Not Linked to any record")}`); - flag = true; - } - flag = false; - - // reject Promise if not_permitted or missing - return new Promise( - (resolve, reject) => flag ? reject() : resolve() - ); - } - - get_linked_doctypes() { - return new Promise((resolve) => { - if (this.frm.__linked_doctypes) { - resolve(); - } - - frappe.call({ - method: "frappe.desk.form.linked_with.get_linked_doctypes", - args: { - doctype: this.frm.doctype - }, - callback: (r) => { - this.frm.__linked_doctypes = r.message; - resolve(); - } - }); - }); - } - - get_linked_docs() { - return frappe.call({ - method: "frappe.desk.form.linked_with.get_linked_docs", - args: { - doctype: this.frm.doctype, - name: this.frm.docname, - linkinfo: this.frm.__linked_doctypes, - for_doctype: this.for_doctype - }, - callback: (r) => { - this.frm.__linked_docs = r.message || {}; - } - }); - } - make_doc_head(heading) { return `

From 38fbe76ebff363ae1c49df7fb7b307eed0dda013 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 7 Mar 2022 15:44:30 +0530 Subject: [PATCH 11/16] fix: Eliminate broken & impermissible links from get_linked_docs --- frappe/desk/form/linked_with.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py index 76900565c9..162707239a 100644 --- a/frappe/desk/form/linked_with.py +++ b/frappe/desk/form/linked_with.py @@ -380,9 +380,15 @@ def get_linked_docs(doctype: str, name: str, linkinfo: Dict = None) -> Dict[str, for dt, link in linkinfo.items(): filters = [] link["doctype"] = dt - link_meta_bundle = frappe.desk.form.load.get_meta_bundle(dt) + try: + link_meta_bundle = frappe.desk.form.load.get_meta_bundle(dt) + except Exception: + continue linkmeta = link_meta_bundle[0] + if not linkmeta.has_permission(): + continue + if not linkmeta.get("issingle"): fields = [d.fieldname for d in linkmeta.get("fields", { "in_list_view": 1, From 39fc90cb5bc53e58840605f64a31ea44b963a3d5 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 7 Mar 2022 16:35:53 +0530 Subject: [PATCH 12/16] fix(ux): Pop from message_log if DoesNotExistError --- frappe/desk/form/linked_with.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/frappe/desk/form/linked_with.py b/frappe/desk/form/linked_with.py index 162707239a..010d65c95b 100644 --- a/frappe/desk/form/linked_with.py +++ b/frappe/desk/form/linked_with.py @@ -1,9 +1,10 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE + import json from collections import defaultdict import itertools -from typing import Dict, List +from typing import Dict, List, Optional import frappe import frappe.desk.form.load @@ -367,7 +368,7 @@ def get_exempted_doctypes(): @frappe.whitelist() -def get_linked_docs(doctype: str, name: str, linkinfo: Dict = None) -> Dict[str, List]: +def get_linked_docs(doctype: str, name: str, linkinfo: Optional[Dict] = None) -> Dict[str, List]: if isinstance(linkinfo, str): # additional fields are added in linkinfo linkinfo = json.loads(linkinfo) @@ -382,7 +383,10 @@ def get_linked_docs(doctype: str, name: str, linkinfo: Dict = None) -> Dict[str, link["doctype"] = dt try: link_meta_bundle = frappe.desk.form.load.get_meta_bundle(dt) - except Exception: + except Exception as e: + if isinstance(e, frappe.DoesNotExistError): + if frappe.local.message_log: + frappe.local.message_log.pop() continue linkmeta = link_meta_bundle[0] @@ -451,11 +455,13 @@ def get_linked_docs(doctype: str, name: str, linkinfo: Dict = None) -> Dict[str, return results + @frappe.whitelist() def get(doctype, docname): linked_doctypes = get_linked_doctypes(doctype=doctype) return get_linked_docs(doctype=doctype, name=docname, linkinfo=linked_doctypes) + @frappe.whitelist() def get_linked_doctypes(doctype, without_ignore_user_permissions_enabled=False): """add list of doctypes this doctype is 'linked' with. @@ -470,6 +476,7 @@ def get_linked_doctypes(doctype, without_ignore_user_permissions_enabled=False): else: return frappe.cache().hget("linked_doctypes", doctype, lambda: _get_linked_doctypes(doctype)) + def _get_linked_doctypes(doctype, without_ignore_user_permissions_enabled=False): ret = {} # find fields where this doctype is linked @@ -499,6 +506,7 @@ def _get_linked_doctypes(doctype, without_ignore_user_permissions_enabled=False) return ret + def get_linked_fields(doctype, without_ignore_user_permissions_enabled=False): filters = [['fieldtype','=', 'Link'], ['options', '=', doctype]] @@ -529,6 +537,7 @@ def get_linked_fields(doctype, without_ignore_user_permissions_enabled=False): return ret + def get_dynamic_linked_fields(doctype, without_ignore_user_permissions_enabled=False): ret = {} From 3858eff51e328f33e6eb782c609d347d86120840 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 7 Mar 2022 18:17:31 +0530 Subject: [PATCH 13/16] fix: Don't add signature only when called from API --- frappe/core/doctype/communication/email.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frappe/core/doctype/communication/email.py b/frappe/core/doctype/communication/email.py index 195518f40d..b51749ccb7 100755 --- a/frappe/core/doctype/communication/email.py +++ b/frappe/core/doctype/communication/email.py @@ -96,6 +96,7 @@ def make( print_letterhead=print_letterhead, email_template=email_template, communication_type=communication_type, + add_signature=False, ) @@ -120,6 +121,7 @@ def _make( print_letterhead=True, email_template=None, communication_type=None, + add_signature=True, ) -> Dict[str, str]: """Internal method to make a new communication that ignores Permission checks. """ @@ -148,7 +150,7 @@ def _make( "has_attachment": 1 if attachments else 0, "communication_type": communication_type, }) - comm.flags.skip_add_signature = True + comm.flags.skip_add_signature = not add_signature comm.insert(ignore_permissions=True) # if not committed, delayed task doesn't find the communication From ce77e97db65a7ae2192e75a8da6c64191d76c0ee Mon Sep 17 00:00:00 2001 From: Himanshu Date: Tue, 8 Mar 2022 11:16:59 +0000 Subject: [PATCH 14/16] feat: enable tls for ngrok (#16216) * styled site module: sorted imports, space fixes --- frappe/commands/site.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) mode change 100755 => 100644 frappe/commands/site.py diff --git a/frappe/commands/site.py b/frappe/commands/site.py old mode 100755 new mode 100644 index c5d2257d75..b54f369e34 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -1,7 +1,7 @@ # imports - standard imports import os -import sys import shutil +import sys # imports - third party imports import click @@ -65,11 +65,11 @@ def restore(context, sql_file_path, encryption_key=None, db_root_username=None, "Restore site database from an sql file" from frappe.installer import ( _new_site, - extract_sql_from_archive, extract_files, + extract_sql_from_archive, is_downgrade, is_partial, - validate_database_sql + validate_database_sql, ) from frappe.utils.backups import Backup if not os.path.exists(sql_file_path): @@ -207,7 +207,7 @@ def restore(context, sql_file_path, encryption_key=None, db_root_username=None, @click.option('--encryption-key', help='Backup encryption key') @pass_context def partial_restore(context, sql_file_path, verbose, encryption_key=None): - from frappe.installer import partial_restore, extract_sql_from_archive + from frappe.installer import extract_sql_from_archive, partial_restore from frappe.utils.backups import Backup if not os.path.exists(sql_file_path): @@ -545,7 +545,7 @@ def _use(site, sites_path='.'): def use(site, sites_path='.'): if os.path.exists(os.path.join(sites_path, site)): - with open(os.path.join(sites_path, "currentsite.txt"), "w") as sitefile: + with open(os.path.join(sites_path, "currentsite.txt"), "w") as sitefile: sitefile.write(site) print("Current Site set to {}".format(site)) else: @@ -751,6 +751,7 @@ def set_admin_password(context, admin_password=None, logout_all_sessions=False): def set_user_password(site, user, password, logout_all_sessions=False): import getpass + from frappe.utils.password import update_password try: @@ -881,15 +882,16 @@ def stop_recording(context): raise SiteNotSpecifiedError @click.command('ngrok') +@click.option('--bind-tls', is_flag=True, default=False, help='Returns a reference to the https tunnel.') @pass_context -def start_ngrok(context): +def start_ngrok(context, bind_tls): from pyngrok import ngrok site = get_site(context) frappe.init(site=site) port = frappe.conf.http_port or frappe.conf.webserver_port - tunnel = ngrok.connect(addr=str(port), host_header=site) + tunnel = ngrok.connect(addr=str(port), host_header=site, bind_tls=bind_tls) print(f'Public URL: {tunnel.public_url}') print('Inspect logs at http://localhost:4040') From a6c257261b591b0b7882202e061732231d93a369 Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Wed, 9 Mar 2022 12:16:14 +0530 Subject: [PATCH 15/16] fix(ux): remove attachment limits --- frappe/core/doctype/user/user.json | 3 +-- frappe/email/doctype/newsletter/newsletter.json | 3 +-- frappe/website/doctype/blog_post/blog_post.json | 3 +-- frappe/website/doctype/web_page/web_page.json | 3 +-- frappe/website/doctype/website_settings/website_settings.json | 3 +-- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/frappe/core/doctype/user/user.json b/frappe/core/doctype/user/user.json index a47f539466..9e9529cd5e 100644 --- a/frappe/core/doctype/user/user.json +++ b/frappe/core/doctype/user/user.json @@ -668,8 +668,7 @@ "link_fieldname": "user" } ], - "max_attachments": 5, - "modified": "2022-01-03 11:53:25.250822", + "modified": "2022-03-09 01:47:56.745069", "modified_by": "Administrator", "module": "Core", "name": "User", diff --git a/frappe/email/doctype/newsletter/newsletter.json b/frappe/email/doctype/newsletter/newsletter.json index baabd4991e..b42f4755cb 100644 --- a/frappe/email/doctype/newsletter/newsletter.json +++ b/frappe/email/doctype/newsletter/newsletter.json @@ -236,8 +236,7 @@ "index_web_pages_for_search": 1, "is_published_field": "published", "links": [], - "max_attachments": 3, - "modified": "2021-12-06 20:09:37.963141", + "modified": "2022-03-09 01:48:16.741603", "modified_by": "Administrator", "module": "Email", "name": "Newsletter", diff --git a/frappe/website/doctype/blog_post/blog_post.json b/frappe/website/doctype/blog_post/blog_post.json index b05293f28b..5e3cc78d70 100644 --- a/frappe/website/doctype/blog_post/blog_post.json +++ b/frappe/website/doctype/blog_post/blog_post.json @@ -213,8 +213,7 @@ "index_web_pages_for_search": 1, "is_published_field": "published", "links": [], - "max_attachments": 5, - "modified": "2021-11-23 10:42:01.759723", + "modified": "2022-03-09 01:48:25.227295", "modified_by": "Administrator", "module": "Website", "name": "Blog Post", diff --git a/frappe/website/doctype/web_page/web_page.json b/frappe/website/doctype/web_page/web_page.json index b1fdd02af7..e7bd705272 100644 --- a/frappe/website/doctype/web_page/web_page.json +++ b/frappe/website/doctype/web_page/web_page.json @@ -338,8 +338,7 @@ "index_web_pages_for_search": 1, "is_published_field": "published", "links": [], - "max_attachments": 20, - "modified": "2022-01-03 13:01:48.182645", + "modified": "2022-03-09 01:45:28.548671", "modified_by": "Administrator", "module": "Website", "name": "Web Page", diff --git a/frappe/website/doctype/website_settings/website_settings.json b/frappe/website/doctype/website_settings/website_settings.json index 3b199a4b58..b628437315 100644 --- a/frappe/website/doctype/website_settings/website_settings.json +++ b/frappe/website/doctype/website_settings/website_settings.json @@ -420,8 +420,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "max_attachments": 10, - "modified": "2022-02-24 15:37:22.360138", + "modified": "2022-03-09 01:47:31.094462", "modified_by": "Administrator", "module": "Website", "name": "Website Settings", From e8ec8410e628806c0a54939d78b992d3582fc48f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 9 Mar 2022 12:57:27 +0530 Subject: [PATCH 16/16] fix(rename_doc): Allow updating only document title or name --- frappe/model/rename_doc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/model/rename_doc.py b/frappe/model/rename_doc.py index faa3859c91..b4a53e3131 100644 --- a/frappe/model/rename_doc.py +++ b/frappe/model/rename_doc.py @@ -43,8 +43,8 @@ def update_document_title( title_field = doc.meta.get_title_field() - title_updated = (title_field != "name") and (updated_title != doc.get(title_field)) - name_updated = updated_name != doc.name + title_updated = updated_title and (title_field != "name") and (updated_title != doc.get(title_field)) + name_updated = updated_name and (updated_name != doc.name) if name_updated: docname = rename_doc(doctype=doctype, old=docname, new=updated_name, merge=merge)